fix[scripts]: emit real diffs from the DaCe determinism check - #2857
Draft
havogt wants to merge 1 commit into
Draft
fix[scripts]: emit real diffs from the DaCe determinism check#2857havogt wants to merge 1 commit into
havogt wants to merge 1 commit into
Conversation
The `diffs/` artifact only listed the relative paths whose digest differed between the two runs, and `run_determinism_check` deletes both caches before the CI job uploads it, so a failure could not be diagnosed from the artifact alone. Render the differing sources as unified diffs instead. A name is compiled once per set of parameters and the variants carry no identity that survives across runs, so pair them by smallest diff — pairing in digest order diffs unrelated variants and reports the difference between them rather than the non-determinism. Claude-Session: https://claude.ai/code/session_01Sf2j16DZhoaJJgy2gWyKUR
Contributor
Author
|
cscs-ci run dace-determinism |
Contributor
Author
|
Not sure if I like the change, but it's certainly more useful to have the diff. |
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.
Motivation
The
test_next_dace_determinismsession failed on a CI job with:The uploaded artifact could not say what differed.
write_diffsonly wrotethe program name plus the relative paths whose
(relpath, sha256)pairdiffered, and
run_determinism_checkdeletesrun1/,run2/and.dacecachein its
finally— so by the time the artifact was uploaded the two versions ofthe file were gone:
Diagnosing it needed a local reproduction of the whole selection.
Change
_scannow also returns a(relpath, sha256) -> Pathmap, andwrite_diffsuses it to render each differing source as a unified diff (capped at
MAX_DIFF_LINES).One wrinkle: a name is compiled once per set of parameters, and a variant
carries no identity that survives across runs, so run1's variants cannot be
matched to run2's by digest. The previous placeholder gave up here — which is
exactly the case that occurred, since
testeehad 34 compiled variants ofwhich 2 differed. Variants are now paired greedily by smallest diff; pairing
them in digest order diffs unrelated variants against each other and reports
the difference between the variants (here: scan direction) rather than the
non-determinism.
Result
Same failure, replayed against the new code:
The underlying bug this surfaced
Not fixed here (it is a DaCe issue), but for the record — the differing program
is
testeefromtest_scan.py::test_scan_nested_tuple_output, bothforwardparametrizations. In
dace/transformation/interstate/gpu_transform_sdfg.py:input_nodes/output_nodesare lists built in deterministic order, then runthrough
set()to dedup.Data.__hash__ishash(serialize.dumps(self))— astring hash, so iteration order follows
PYTHONHASHSEED. That order is theinsertion order of the
gpu_*clones intosdfg.arrays, hence the emittedalloc/free order. Verified: identical output for a fixed seed, different output
across seeds; replacing both
set(...)with an order-preserving dedup makesthe output seed-independent.
Testing
scripts/tests/python/test_dace_determinism.py— 46 pass, incl. four new casescovering the unified-diff output, smallest-diff variant pairing, unpaired
variants, and truncation. Validated end to end by replaying the real
two-cache reproduction.
pre-commit runclean.https://claude.ai/code/session_01Sf2j16DZhoaJJgy2gWyKUR