Repository navigation
perf(pt): reuse SO2 residual workspace during inference - #6058
macqueen09 wants to merge 3 commits into
Conversation
Reuse owned residual storage only when weight gradients and saved layer state are not required. Preserve caller inputs and training paths. Add generated-input regressions for alias guards, supported dtypes, partial tiles and force-loss derivatives with optimizer updates.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
njzjz-bot
left a comment
There was a problem hiding this comment.
Agent: dot
Reviewed the exact two-file delta against dbca0b1; no actionable source defect found. Reuse is restricted to internally allocated g_cur without weight gradients or retained layer state. Gated launches keep gz separate, the final launch stays out-of-place, and the residual load/output store use the same uniquely owned (focus, edge, column) tile. Caller inputs and forward-saved tensors are not mutated.
Independent CPU checks using the actual traversal and four kernel ASTs: 40 baseline/candidate cases pass across weights/keep/alpha, zero-gated and narrow/wide-BMM paths; four reversed/shuffled multi-tile fp32/fp16 comparisons pass; caller buffers remain unchanged. An intentionally unsafe gz/output alias fails the negative control. Address enumeration covers 360 supported shape/edge partitions and 71,176,320 addresses without overlap or gaps. The actual autograd-dispatch AST passes all 64 combinations: trainable/partially frozen operands, create_graph and saved-output cotangents route through unchanged training handling; second-order replay uses keep=True. Both changed files compile syntactically and the new test matrix contains 106 CUDA cases.
These are CPU semantic/address probes, not CUDA execution or Inductor functionalization validation. Torch/GPU are unavailable here. Author-reported CUDA/memcheck, optimizer equivalence and measured memory reduction remain unverified locally. The disclosed 4096-atom self-repeatability failures on both base and candidate do not establish determinism or a causal regression; no timing speedup is inferred from the near-neutral latency measurements.
At review time all seven returned build/test workflows remain action_required. No workflow activation, code modification or merge action was taken; relevant actual CUDA/compile coverage must be distinguished from skipped aggregate checks.
|
Local inference and training validation, benchmark results, and reproduction commands are documented in the PR description. Could a maintainer approve the pending runs and trigger the CUDA tests using the Test CUDA label? Please also verify that the relevant CUDA and compile tests execute rather than skip. Thanks! |
OutisLi
left a comment
There was a problem hiding this comment.
Requesting changes to the test scope and organization, not to the production optimization.
The residual-workspace reuse is a small, coherent change. The matrix operand gz remains separate, each output tile reads only its own residual elements before overwriting them, the final-layer output stays out-of-place, and weight-gradient/saved-state traversals retain their original buffers. I did not find an actionable production correctness defect within this review. Please keep that implementation focused; it needs neither a new acceleration level nor a new autotuning mechanism.
Proposed test simplification
1. Replace the 90-case Cartesian product with a representative case list.
The main parameterization expands to 5 shapes × 3 edge counts × 3 layer counts × 2 alpha settings. Thirty cases have zero edges and return before the changed traversal; another twenty have nonzero edges but no gated layer, so the aliased gated update is never exercised. These boundaries deserve coverage, but not the full cross product. Even empty-edge cases still construct and snapshot weights.
For example, one parameterization over the following seven cases retains all five original shape profiles, both alpha modes, zero/partial-edge workloads, zero/single/multiple gated layers, and narrow/wide gate-projection paths:
# (lmax, focus, width, edges, gates, apply_alpha)
CASES = [
(2, 2, 32, 0, 1, True), # Empty-edge return.
(2, 2, 32, 37, 0, True), # Final layer only.
(2, 2, 32, 37, 1, False), # Single gated layer without alpha.
(3, 2, 32, 129, 3, True), # Multi-layer recurrence and partial tiles.
(3, 1, 64, 37, 1, True), # Single focus, wider narrow-channel path.
(6, 2, 96, 37, 3, False), # BMM gate path, non-power-of-two width.
(6, 3, 128, 129, 1, True), # BMM gate path and multiple focuses.
]Keep the existing numerical, output-shape and caller-input-preservation assertions. This list is a concrete starting point, not a hard case-count target: add or adjust a case only when it covers a distinct ownership/branch/layout condition missing from the others.
2. Keep the four workspace-identity guard cases.
test_reuse_only_without_weights_or_saved_state directly tests the intended change and has a useful baseline-fails/candidate-passes assertion. Preserve all four with_weights/keep combinations, the out-of-place final layer, and the assertion that gz never aliases the output.
3. Keep the six dtype/alpha alias-versus-separate comparisons.
The three supported dtypes × two alpha settings in test_reuse_matches_separate_workspace cover a distinct contract. Preserve their numerical and input-preservation checks rather than weakening tolerances to reduce work.
4. Separate training dispatch protection from broad training equivalence.
The six frozen/trainable-weight cases each repeat first/second derivatives and two parameter updates. Much of this checks paths deliberately excluded from the changed reuse branch.
Use lightweight dispatch checks for trainable, partially frozen and fully frozen weights, especially create_graph=True, to establish that they select the protected training/saved-state path. Reuse existing model-level gradient/training regressions where they actually cover the same contract. Those existing tests do not automatically prove every frozen-weight direct-stack case, so identify any genuinely missing coverage before removing it. If a distinct numerical higher-order path still needs protection, retain the smallest representative single-step check for that path, rather than automatically crossing every weight-freezing mode with both widths and repeating optimizer updates.
This yields 17 focused core cases (7 numerical + 4 ownership guards + 6 dtype/alpha checks), plus any justified unique derivative coverage. Please explain that coverage mapping briefly in the updated PR. Do not add a new timing study, production-size benchmark, or full-model GPU test suite just to validate this test reorganization.
Verification and limits
The prior review's 11 small CPU-interpreter comparisons of the exact base/head traversal passed bitwise, preserved caller inputs, and confirmed the expected buffer identities. Ruff and whitespace checks passed. No GPU execution, test-runtime benchmark or independent reproduction of the author's GPU memory measurements was performed. The test-cost concern above is based on redundant paths and compilation/workload structure, not a claimed measured CI speedup.
Please retain the production change and make the regression suite proportionate to its contract before merging.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6058 +/- ##
==========================================
+ Coverage 77.79% 77.82% +0.02%
==========================================
Files 1171 1171
Lines 141562 141609 +47
Branches 5066 5072 +6
==========================================
+ Hits 110135 110202 +67
+ Misses 29550 29467 -83
- Partials 1877 1940 +63 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@OutisLi Addressed in 30c9fc98. This follow-up changes only
Validation on RTX 5090 / Torch 2.11.0+cu128: 26 passed; without a visible GPU 26 skipped. The retained identity guard fails at the expected alias assertion with the exact baseline traversal substituted in an isolated diagnostic process, and passes with the candidate. Full-repository Ruff check/format and whitespace checks passed. Local CUDA coverage executes the three new production statements and both outcomes of the ownership guard. This does not assert that the upstream Codecov or required workflows are already green; no CI/coverage exclusions were changed. No timing study, production-size benchmark or full-model GPU suite was added for this revision. Please re-review the focused scope when convenient. |
Review follow-up: focused regression scope (2026-10-19)
Addressing OutisLi's requested changes.
The production implementation is unchanged from
7cfd926b; this follow-up changes only the test file. No new inference level, autotuning, performance study, production-size benchmark or full-model GPU test suite was added.Existing
TestSeZMModelCompile.test_forward_backward_double_backward_matches_compilecovers ordinary model-level parameter gradients, force loss and short training;TestSeZMTrainPathParity.test_training_step_matches_the_dense_pathcovers descriptor/training binding parity. Those do not automatically establish the fully frozen direct-stack force-loss path, so one representative numerical check remains. The six new dispatch cases execute realtorch.autograd.grad(..., create_graph=...); their mocked gradient operators are not presented as numerical training-equivalence evidence.Fresh validation: 26 CUDA tests passed; with no visible GPU 26 skipped. The retained ownership guard still fails at its residual/output alias assertion when the exact baseline traversal is substituted in an isolated diagnostic process. Both retained guard/dtype functions are AST-identical to the previous head. Full-repository Ruff check/format and whitespace checks passed; production source is byte-for-byte unchanged.
Local CUDA coverage executes the three new statements (lines 3199–3201 at this revision), including both outcomes of the line-3200 guard. This is not a claim that upstream Codecov/required checks are green. No CI configuration or coverage exclusions were changed. Please verify GPU execution and the uploaded coverage in the new upstream runs.
The 106-case counts, full-model measurements and multi-step training evidence below are historical pre-review results, not newly rerun tests or performance claims for this follow-up.
Summary
Reuse the internally owned residual-gradient workspace in the SO2 mixing-stack
backward traversal when neither weight gradients nor saved per-layer state are
needed. This removes one edge-sized allocation without changing kernel
arithmetic, model parameters, precision settings or public APIs.
This is an isolated workspace change. It does not include compiler scheduling
passes, the inference-compiler flag from #6056, additional fusion, checkpoint
changes, or local benchmark infrastructure.
Why the alias is safe
The gated-layer backward GEMM computes
residual + gz @ W.T. Its matrix inputgzremains separate from the output. Each output tile reads only its matchingresidual elements before writing those same locations; distinct programs do not
read one another's residual tiles. The final-layer GEMM remains out-of-place.
Only buffers allocated inside this traversal are reused. Caller inputs and
forward-saved tensors are not mutated, so the operator's
mutates_args=()contract is unchanged. The weight-gradient and saved-state traversals keep their
original allocation paths. The guard uses actual gradient/state requirements,
not simply
model.eval(), which matters for frozen-weight and force-loss cases.The generated full-model graph was checked as well: four gated GEMM calls alias
residual/output, while both final-layer calls and every matrix-input/output pair
remain separate.
Scope
deepmd/pt_expt/kernels/triton/sezm/so2_value_path.py: local allocation changeand the alias-safety explanation.
source/tests/pt_expt/kernels/test_so2_workspace.py: generated-input CUDAregressions, with no checkpoint or machine-specific dependencies.
The current focused tests cover the contracts mapped in the review follow-up above.
They retain the allocation-identity baseline-fails/candidate-passes assertion,
unchanged dtype/alpha checks and tolerances, lightweight autograd-dispatch checks,
and one numerical frozen-weight force-loss case rather than repeated updates.
Full-model measurements
Base:
dbca0b1cfabc9be166feffcbbc93e9c833603aca.RTX 5090, driver 595.91.07; Python 3.11.12, PyTorch 2.11.0+cu128, Triton 3.6.0,
Warp 1.16.0. Same DPA4-Neo checkpoint and normal ASE/DP energy/forces/virial/stress
evaluation, including neighbor construction.
DP_COMPILE_INFER=1,DP_TRITON_INFER=2, TF32/AMP off, one host thread,PYTORCH_ALLOC_CONF=expandable_segments:True.Diamond C, lattice 3.567 Angstrom, seed 0, position noise sigma 0.02 Angstrom.
Each process: 8 warmup calls including the first, 30 synchronized wall-clock
samples, and a separate 30-call memory replay. Three independent processes for
4096 atoms; two for 1000. Timing values below are medians of process quantiles;
memory values are the maximum measured peaks. GB means 1e9 bytes.
This is a memory optimization, not a claim of significant speedup. The small
observed 1000-atom latency increase is disclosed rather than classified as a
speed improvement. The 4096-atom allocation reduction is exactly 1,656,755,200
bytes, matching one
[2, 647170, 320]FP32 workspace. First-call compilation isexcluded from steady timing but included in the full memory window. GPU clocks
were not locked, and these short research runs are not a long-duration
performance acceptance test.
Numerical validation and limitations
Full-model comparison uses all energy, force, virial and stress elements with
finite-value checks and
abs(actual-reference) <= 1e-5 + 1e-4*abs(reference).independent processes; all 10 three-frame self-repeatability rounds pass.
self-repeatability is not universally satisfied: candidate 6/15 and base
9/15 rounds pass. The failed checks are the third frame's virial. Baseline
cross-process comparisons also fail that component in both repeat runs.
No tolerance was relaxed and no output was excluded. These observations do
not establish either a fix or a regression in global numerical determinism.
An additional independent dense eager GPU reference, with compilation and
Triton/CUDA fast paths disabled, also passes for both 1000-atom candidate runs
and both base runs at the same elementwise thresholds. This was one three-frame
reference evaluation, not an exhaustive determinism test.
Historical local test evidence (before the focused-test revision)
hooks passed.
127 subtests passed. All nine
TestSeZMModelCompiletests passed. The fiveskips require DPA4 native CUDA operators absent from the installed library.
expected; the candidate passes.
the installed customized-op library was built without GPU support. This is
an environment limitation, not a passing test or a newly introduced failure.
The local source-selection harness used the new Python checkout with existing
native libraries from the measured environment. This does not claim a fresh
C++/wheel build or TensorFlow/JAX/Paddle validation. Other GPU architectures and
Torch 2.12/2.13 are not locally validated here. A CUDA-native rebuild is needed
to cover the missing native paths; this PR does not claim that every local
or required CI check is green.
Historical additional cross-checkout training equivalence
Separate BASE and candidate processes, identical generated inputs/initialization,
TF32 disabled, same GPU/environment:
2516/2516 finite arrays pass the same
atol=1e-5, rtol=1e-4elementwise comparison. 1384 arrays are bitwise identical; global maximum
absolute difference is 3.725290298461914e-9; maximum error/tolerance ratio is
0.0003452008823. This is bounded multi-step equivalence evidence, not a
claim that arbitrary long training is bitwise identical.
The normal
model.train()route is excluded from this inference stack bythe existing
not self.trainingguard inso2.py. Direct-stack force-losstests additionally cover trainable, partially frozen and frozen operands:
observed backward calls requiring weight gradients retain separate workspace,
and calls saving second-order state retain the saved-state path. No public
training flags or APIs change.
An additional tiny-model configuration combining Triton training with compilation
triggered a Torch 2.11 Inductor
replace_by_examplepattern-matcher assertionon the unmodified BASE. The final comparison therefore explicitly separates
eager Triton training from default compiled training, rather than claiming that
combined configuration passed. Candidate equivalence for that failing BASE
configuration is not established. The installed native library also lacks CUDA
operators; local CUDA toolkit 13.2 does not match this Torch cu128 build, and a
fresh matching-toolkit CUDA build remains for CI/a suitable environment.
Reproduction / reviewer requests
Run the new test file using the normal project installation. It generates all
operands and does not download a model. Also run the original
TestSeZMModelCompile,TestSeZMTritonValuePath, descriptor and training-pathregressions. The commands and standalone scripts below provide reproduction without
committing machine-specific benchmark infrastructure.
The exact checkpoint is
DPA4-Neo-OMat24-v20260627.pt, SHA2568d8b2b02b9575cb9e0af420cc1e8b1d9ffcbc04752c2203d29f48ccd0cd5f07d;it is not bundled. Exact full-model numbers require an authorized copy.
Please trigger the upstream CUDA workflow and verify that relevant compile
tests actually run rather than skip under the workflow's Torch version. Local
results do not replace required status checks, maintainer review, or the merge
queue.
Reproducible commands
Install each revision following the project's CUDA/PyTorch build instructions
in separate environments, then verify
deepmd.__file__andtorch.__version__. Do not accidentally import another editable checkout.Tests below need no checkpoint:
Memcheck does not prove absence of all global-memory races; see the tile-level
read/write argument above.
For full-model reproduction, save the following script outside the repository.
Run serially on an otherwise idle GPU, with a fresh output directory and cache
for every invocation. With BASE installed/imported first:
CUDA_VISIBLE_DEVICES=0 MODEL=/path/to/authorized/model.pt OUTPUT=/scratch/base-r0 \ TORCHINDUCTOR_CACHE_DIR=/scratch/cache-base-r0 NREP=8 \ python /scratch/public_benchmark.py # Switch to the candidate environment/checkout: CUDA_VISIBLE_DEVICES=0 MODEL=/path/to/authorized/model.pt OUTPUT=/scratch/candidate-r0 \ TORCHINDUCTOR_CACHE_DIR=/scratch/cache-candidate-r0 NREP=8 \ REFERENCE=/scratch/base-r0/outputs.npz python /scratch/public_benchmark.pyRepeat 3 processes for
NREP=8(4096 atoms), 2 forNREP=5(1000 atoms),using different paths; interleave/reverse order when repeating. Inspect every
JSON comparison and finite check. This portable script reproduces normal API
timing/memory and cross-revision checks; the reported main experiment additionally
ran five three-frame self-repeatability rounds per process and audited generated
wrappers. Those extra diagnostics are not implemented by this short script.
Do not use
torch.inference_mode(): conservative force evaluation needsdifferentiation. No private compiler hooks are needed for this change.
Standalone public_benchmark.py (normal DP / ASE APIs)
Supplementary multi-step training comparison (outside the production branch)
Save this as
training_equivalence.pyoutside the repository. Run once fromeach installed checkout's root in a fresh process/cache:
CUDA_VISIBLE_DEVICES=0 PYTHONPATH="$PWD" TRAIN_OUTPUT=/scratch/train-base TORCHINDUCTOR_CACHE_DIR=/scratch/train-cache-base python /scratch/training_equivalence.py.Use distinct candidate output/cache paths. The code below only makes GPU
selection configurable relative to the measured local script; computations are
the same. It uses existing repository test helpers and generated data; no
pretrained model is required.
Compare the two generated archives with shape/key/dtype and finite checks:
Contribution / merge checklist
benchmark caches, raw customer data, or machine-specific files committed.
Prepared against the contributing guide
and linked coding conventions
and CI documentation.
Current default-branch rules require seven check contexts (C++ tests, C++ build,
C-library build, Python tests, wheel builds, CUDA aggregate, pre-commit.ci),
at least one approval, resolved threads, and the squash merge queue.
The CUDA workflow needs maintainer activation (e.g. the
Test CUDAlabel);a passing aggregate with skipped GPU jobs is not CUDA validation. Please also
check Torch-version gates on compile tests. No claim is made that local tests
replace these server-side requirements.
Summary by CodeRabbit