Repository navigation
test(determinism): add kernel replay tests and a PR coverage gate - #7148
Conversation
|
/ok to test 74d6667 |
74d6667 to
cdfe783
Compare
|
/ok to test cdfe783 |
|
/ok to test 6e91ecc |
6e91ecc to
c4f488c
Compare
|
/ok to test c4f488c |
|
@Connor-XY My concern is that these golden values may be too strict for new kernels. A new kernel can be fully deterministic and still produce slightly different numbers because it uses a different, but valid, computation order or implementation. With bit-exact golden checks, that kernel would fail even if it is correct. The author would then have to update the golden values, which means the test is mostly checking that the output has not changed from the previous implementation—not necessarily that the new implementation is correct. I think we should clarify whether the goal is to lock down one exact implementation, or to allow different correct deterministic kernels. If we want to support the latter, we may need a tolerance or a separate correctness/reference test instead of requiring an exact match to the existing golden values. |
|
Hi @guihong-nv, thanks for raising this issue up! The kernel tests do not use golden values. They replay the same kernel on the same inputs and require the outputs to match themselves, so this should work for all kernels. For functional tests, we have the full precision enabled to check bitwise determinism, and if not set to full precision, a determinism break may not be caught due to tolerance. If some functional test uses new kernel and the result changes, then its golden value should be updated. This also allows new kernels to be added. Please let me know your thoughts! |
Thanks, this makes sense. |
Add byte-exact kernel replay tests, the coverage manifest, an H100 test bucket, and PR enforcement requiring matching test updates. Keep full-precision functional enforcement and golden-data refresh in the independent PR NVIDIA#7217. Preserve the current main review workflow and its kernel test requirement. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Yan Xu <yxu1@nvidia.com>
43fe3f6 to
d5e5950
Compare
|
/ok to test d5e5950 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/34662389346 |
Kernel changes need tests that expose scheduling-dependent numerical differences and a PR check that keeps those tests maintained. Add a kernel replay suite, an explicit coverage registry, and a CI gate requiring changed kernel code to carry an updated determinism test.
Full-precision functional enforcement, its regression tests, and all 60 golden updates are isolated in #7217. The two PRs target
mainindependently and can merge in either order. This PR contains 23 files and no golden-data or functional-comparison changes.Changes
tests/unit_tests/determinism/kernels/: replay helpers compare tensor outputs and gradients byte-for-byte, including signed zeros and NaN payloads. Tests can restore RNG state and add side-stream contention; module replay and negative controls exercise the same contract.manifest.pywith tests or an explicit exemption explaining a coverage gap.tools/check_kernel_determinism_coverage.pyto the linting job on PR pushes. Changed kernel-bearing files must be registered. Changed registered kernel sources must also carry a determinism-test update; thedeterminism-exemptlabel overrides the test-update requirement for changes that do not affect numerics, but cannot bypass registration.main's refactored review workflow while retaining the kernel-test review requirement.Validation
For the split head:
main, using the unchanged test/source modules in an isolated CPU harness. Only GPU package initialization and repository conftest were bypassed.main; YAML parsing and assertions verified the gate, dedicated recipe bucket, and separation from full-precision enforcement.git diff --checkpassed.GPU replay tests have not been rerun on this split head.
Historical results reported on the combined PR: the H100 × 8 kernel bucket passed 380 tests with no failures; a four-GPU GB300 run also reported 380 passed. These results predate this split and are not a fresh validation of its head.
Known coverage gaps and follow-ups
determinism-exemptlabel and extend the bucket to GB200/GB300 runners.Related to #5785.