Skip to content

fix[next]: deterministic symbol renaming in inline_lambdas - #2701

Merged
havogt merged 2 commits into
GridTools:mainfrom
havogt:fix/deterministic-inline-lambdas
Jul 13, 2026
Merged

fix[next]: deterministic symbol renaming in inline_lambdas#2701
havogt merged 2 commits into
GridTools:mainfrom
havogt:fix/deterministic-inline-lambdas

Conversation

@havogt

@havogt havogt commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Problem

clashes is a set, and each clashing symbol is renamed through unique_symbol(), which appends
_ until the name is free. The number of underscores a symbol ends up with therefore depends on the
order the set is iterated — which is not reproducible across processes.

clashes = refs & syms
...
for sym in clashes:                     # <- set iteration order
    name_map[sym] = ir_misc.unique_symbol(sym, refs | syms | {*name_map.values()})

The order only matters when one clashing symbol's new name can collide with another's — e.g. a
and a_ both clashing. Processing a first yields a → a__, a_ → a___; processing a_ first
yields 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:

(λ(p) → (λ(a, a_) → a + a_ + p)(p, p))(a + a_)

Against unpatched main, varying PYTHONHASHSEED over 0..7:

seed=0 -> (λ(a__, a___) → a__ + a___ + (a + a_))(a + a_, a + a_)
seed=1 -> (λ(a___, a__) → a___ + a__ + (a + a_))(a + a_, a + a_)
seed=2 -> (λ(a___, a__) → ...
seed=4 -> (λ(a__, a___) → ...

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_0 vs __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 clashes is a set of str. If you would rather have the underlying
symbol 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

`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>
assert inlined == expected


def test_deterministic_symbol_renaming():

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: before this test would indeterministically fail. Not sure if we want to keep it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@havogt
havogt marked this pull request as ready for review July 10, 2026 13:44
@havogt
havogt merged commit c9da739 into GridTools:main Jul 13, 2026
24 checks passed
@havogt
havogt deleted the fix/deterministic-inline-lambdas branch July 13, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants