fix[next]: deterministic symbol renaming in inline_lambdas - #2701
Merged
Conversation
`clashes` is a `set`, and each clashing symbol is renamed through `unique_symbol()`, which appends `_` until the name is free. The name a symbol ends up with therefore depends on the order the set is iterated, which is not reproducible across processes. Lowering the same program twice could produce IR differing in symbol names, and downstream SDFGs and generated code differing accordingly. Adds a regression test: the renamed symbols of the inner lambda survive the inlining, so the result is observable. On the unpatched code it fails for 4 of 8 `PYTHONHASHSEED` values; with the fix it passes for all of them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
havogt
commented
Jul 10, 2026
| assert inlined == expected | ||
|
|
||
|
|
||
| def test_deterministic_symbol_renaming(): |
Contributor
Author
There was a problem hiding this comment.
note: before this test would indeterministically fail. Not sure if we want to keep it.
Contributor
There was a problem hiding this comment.
I removed the test since in addition to it only failing indeterminably it relies on the naming of the symbols. We could use ir_misc, but it doesn't feel worth it overall.
tehrengruber
approved these changes
Jul 10, 2026
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.
Problem
clashesis aset, and each clashing symbol is renamed throughunique_symbol(), which appends_until the name is free. The number of underscores a symbol ends up with therefore depends on theorder the set is iterated — which is not reproducible across processes.
The order only matters when one clashing symbol's new name can collide with another's — e.g.
aand
a_both clashing. Processingafirst yieldsa → a__,a_ → a___; processinga_firstyields
a_ → a__,a → a___.Reproducer
The added test exercises exactly that. The renamed symbols belong to an inner lambda, so the new
names survive the inlining and are observable in the result:
Against unpatched
main, varyingPYTHONHASHSEEDover0..7:4 of the 8 seeds fail the new test on unpatched code; all 8 pass with the fix.
Why it matters downstream
Found while chasing non-deterministic DaCe codegen in icon4py. Lowering the graupel microphysics
program twice produced SDFGs whose data names and node labels differed only in underscore counts —
__arg1_______vs__arg1________,__arg2_____from_cb_fusion_0vs__arg2______from_cb_fusion_0— along with the connector names derived from them. Those names become identifiers in the generated
C++, so two cold builds of the same program emit different source.
With this fix, all name-derived differences disappear from that program's SDFG across cold builds.
(A residual, unrelated node-ordering difference remains, which I am still tracking; it is not caused
by this code.)
Scope
This does not implement the better renaming scheme requested by the existing
TODO(tehrengruber)immediately above the loop — it only makes the current scheme deterministic.Sorting is enough because
clashesis a set ofstr. If you would rather have the underlyingsymbol generation reworked as that TODO suggests, say so and I will close this.
Marked draft for that reason, and because I would like a second opinion on where the regression test
belongs. Related to the recent determinism work in #2635 / spcl/dace#2401.
🤖 Generated with Claude Code