[PyTorch][CI] Fix test issues: connect test files that are not wired into qa - #25
Open
pggPL wants to merge 6 commits into
Open
[PyTorch][CI] Fix test issues: connect test files that are not wired into qa#25pggPL wants to merge 6 commits into
pggPL wants to merge 6 commits into
Conversation
tests/pytorch/layernorm_mlp/test_selective_activation_checkpoint.py has never been run by any script in qa/, and it fails on all 16 parametrizations on current main. The failure is in the test, not in the feature: outputs and all six parameter gradients are bit-exact between the checkpointed and the non-checkpointed path. The memory assertion used a hardcoded 6x ratio. The measured ratio is a structural constant of 5.715-5.719, independent of seq_len (128..65536) and hidden size (128..2048), so the threshold was simply unreachable. Changes: - Assert on the memory actually freed by recompute (fc1_out + act_out per layer, derived from the config) instead of a magic ratio. - Check outputs and gradients before the memory check, so a numerical regression cannot be masked by a memory/perf failure. - Drop the bare `ln_bwd_time < sln_bwd_time` assertion. The margin is as low as 13% on an idle GPU, which makes it a CI flake. Timings and memory ratios are reported via record_property instead. - Skip parametrizations that do not fit in device memory. large/65536 and huge/65536 need >32 GiB for the non-checkpointed model alone and OOM on 48 GiB cards. - Run the file in qa/L0_pytorch_unittest. Verified on RTX 5880 Ada: 12 passed, 4 skipped on memory. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
These files are not referenced by any script in qa/ and have therefore never run in CI. Confirmed with `git log -S<name> --all -- qa/`: none of them was ever added and later removed, and none of the PRs that introduced them touched qa/. - test_qk_norm.py, test_float8_current_scaling_exact.py and attention/test_cu_seqlens_cache.py get an entry in L0_pytorch_unittest. - test_nvfp4_fsdp2_hooks.py moves into tests/pytorch/nvfp4/, which L0 already runs as a whole directory. No qa/ change needed, and the file is now covered by the same rule as the other NVFP4 tests. All four are single-GPU and self-skip on unsupported hardware: test_float8_current_scaling_exact.py guards its classes with skipif(not fp8_available), test_nvfp4_fsdp2_hooks.py requires sm_100+, and the one multi-device case in test_cu_seqlens_cache.py checks device_count() first. Measured on RTX 5880 Ada: 45 passed, 5 passed, 1 passed + 1 skipped, and 16 skipped respectively - about 13 s in total. tests/pytorch/test_fused_router_perf.py is deliberately left out; it is gated behind TE_RUN_PERF_TESTS and needs a separate decision. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
The file has two tests. test_cu_seqlens_cache_isolated_across_devices_for_forward needs two CUDA devices and therefore always skips in L0, which is where the file was just wired in. That test is the actual regression guard for NVIDIA#2728 - the cu_seqlens cache key not being scoped by device - so leaving it permanently skipped defeats the purpose of connecting the file at all. L1 is the only suite that guarantees more than one GPU. It is a plain pytest run, no torchrun, matching how attention/test_cp_utils.py is invoked there. Cost is about 2 s. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
…shold Two follow-ups on the selective activation checkpointing test. record_property was added here to keep reporting the backward timings after the flaky `ln_bwd_time < sln_bwd_time` assertion was removed. It was not in the original test, and it emits a PytestWarning on every run because it is not compatible with the default xunit2 junit family. Remove it; the timings were never asserted on and nothing consumes them. The memory threshold divided by the full layer count, but the checkpointed peak still holds the transient of one layer, so recompute only saves the ffn-sized activations of the remaining layers. With `layers`, the ratio of measured to expected saving is (L-1)/L * (1 + h/2f), which happens to clear 0.95 at L=12 (1.031) but would fail at L=4 (0.844) - the threshold silently encoded the shape of the test models. With `layers - 1` the ratio is 1 + h/2f, always above 1 regardless of layer count, sequence length and hidden size, so 0.95 is what it was meant to be: slack for allocator noise. Verified on RTX 5880 Ada: 12 passed, 4 skipped, no PytestWarning. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Restore the inline gradient key list and the config[size] accesses that the previous commit had refactored for no reason. They were unrelated to the fix and only made the diff harder to read. No behaviour change: 12 passed, 4 skipped on RTX 5880 Ada, same as before. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Part of the pytorch test suite is not connected to
qa/at all. Six test files are not referencedby any script there, so they have never run in CI. This PR connects them and fixes the one that
turned out to be broken.
Confirmed with
git log -S<name> --all -- qa/: none of these files was ever added and laterremoved, and none of the PRs that introduced them touched
qa/. They were simply never wired up.maintest_qk_norm.pytest_float8_current_scaling_exact.pyattention/test_cu_seqlens_cache.pytest_nvfp4_fsdp2_hooks.pytest_fused_router_perf.pyTE_RUN_PERF_TESTS)layernorm_mlp/test_selective_activation_checkpoint.pyAll of them are single-GPU and self-skip on unsupported hardware, so they belong in L0.
test_fused_router_perf.pyis deliberately left out of this PR — it is gated behindTE_RUN_PERF_TESTSand, despite its name, contains no perf assertions at all (onlytorch.testing.assert_closeon correctness, with timings going torecord_property). Whether toungate the correctness half deserves a separate decision.
Fixes # (issue)
Type of change
Changes
1. Connect four files to L0 (commit 2)
test_qk_norm.py,test_float8_current_scaling_exact.pyandattention/test_cu_seqlens_cache.pyget an entry in
qa/L0_pytorch_unittest/test.sh.test_nvfp4_fsdp2_hooks.pymoves intotests/pytorch/nvfp4/, which L0 already runs as a wholedirectory. No
qa/change needed, and the file is now covered by the same rule as the otherNVFP4 tests. It has no local imports, so the move is inert. The real multi-GPU FSDP2 path for
NVFP4 is already covered separately by
distributed/fsdp2_tests/run_fsdp2_model.py --recipe NVFP4BlockScalingin L1; this file is a single-GPU unit test of the shape and metadataarithmetic in
fsdp_pre_all_gather/fsdp_post_all_gather.Hardware guards are already in place in all four:
test_float8_current_scaling_exact.pymarks itsclasses with
skipif(not fp8_available),test_nvfp4_fsdp2_hooks.pyrequires sm_100+, and the onemulti-device case in
test_cu_seqlens_cache.pychecksdevice_count()first.Added L0 cost is roughly 13 s.
2. Fix
test_selective_activation_checkpoint.py(commit 1)This is the file that never ran and does not pass. The failure is in the test, not in the feature.
Selective activation checkpointing is correct and does save memory:
max|out_ckpt - out_no_ckpt| = 0.0andmax|grad_ckpt - grad_no_ckpt| = 0.0for all sixparameters, in every configuration that fits in memory.
small, 5.715 formedium/large/huge— independent ofseq_len(128 → 65536) andhidden_size(128 → 2048), with norun-to-run variance.
The test asserted
ln_fwd_mem > 6 * sln_fwd_mem. That threshold was never reachable, and it nevercould have been, because the ratio is fixed by the test's own configuration rather than by anything
in TE. Both sides of the measurement are fully derivable, and the derivations reproduce the
measured peaks exactly:
For
small@ 128 this predicts 7876608 B and 1377280 B; measured 7876608 B and 1377280 B.Substituting
f = 4handlayers = 12gives120 / 21 = 5.714in units ofs*h, matching themeasurement. Reaching 6 would require a different model shape, e.g.
layers = 16gives160 / 25 = 6.4.I also checked that this is not a regression: the tensor lists in both branches of
_LayerNormMLP._forwardare unchanged since the test was added (the only difference is thefc1_wt_save/fc2_wt_savegating for FSDP2 from NVIDIA#2681, inactive for plain parameters), and thetest file itself has only been touched by the 2026 copyright update. Weights are allocated before
the
start_memsnapshot, so they never entered this measurement in the first place.Changes:
fc1_outandact_outper layer, derivedfrom the model config — instead of the ratio. Observed saving is 1.031-1.033x the derived value,
so the
0.95factor leaves ~8% headroom while still failing loudly if either recomputed tensorstops being freed (that would halve the saving). Unlike a whole-forward ratio, this does not
depend on
layersor on theffn_hidden / hiddenratio.regression would have been reported as a memory-ratio failure and the correctness comparison
would never have run — which is exactly what happens on
maintoday.assert ln_bwd_time < sln_bwd_time. The margin is as low as 13% on an idle GPU(
huge@ 128: 11.2 ms vs 12.8 ms), which makes it a flake on a loaded CI runner. Forward andbackward timings plus forward/backward memory ratios are now reported through
record_property,so they land in the JUnit XML as trend data instead of a pass/fail gate.
large@ 65536 andhuge@ 65536 needmore than 32 GiB for the non-checkpointed model alone and OOM on 48 GiB cards; the required
budget is computed up front from the same analytic formula.
Test results
RTX 5880 Ada (48 GiB), current
main+ this PR:Skips in the SAC test are the memory guard doing its job:
Sample of the recorded properties (JUnit XML):
Follow-up
Nothing in the repo prevents this from happening again — a new test file simply has to be
remembered in a shell script. A guard test asserting that every
tests/**/test_*.pyis covered byat least one job in
qa/, plustests/entries inCODEOWNERS(which currently only coverstransformer_engine/), would close the class of problem. Happy to send that separately if there isappetite for it.
Checklist: