fix[next-dace]: Handle duplicate outer AccessNodes for same data in MoveDataflowIntoIfBody#2613
Open
edopao wants to merge 1 commit into
Open
Conversation
…oveDataflowIntoIfBody
When a relocated Tasklet is fed by two distinct AccessNode objects that both
carry the same outer array name (e.g. a stencil-like read where the same
non-transient field is accessed via two separate Map connectors), the second
incoming edge enters the `elif outer_data in fully_mapped_in_data` branch of
`_replicate_dataflow_into_branch` and hits:
assert (outer_data, branch_state) not in rename_map
The first edge already stored the mapping; the second outer node is a
different Python object so the `if (outer_node, branch_state) in node_map`
guard does not catch it, but `rename_map` was already written.
Fix: replace the assertion with a conditional so that only the first
AccessNode establishes the rename_map entry while every subsequent one still
registers itself in `node_map` (allowing the edge-creation code below to look
it up). The resulting inner SDFG is correct: multiple inner edges from the
same inner AccessNode to the Tasklet's different input connectors.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes an AssertionError in MoveDataflowIntoIfBody._replicate_dataflow_into_branch when two distinct outer AccessNode objects refer to the same data and both feed input connectors of a relocatable Tasklet. The fix makes rename_map updates idempotent (first mapping wins) while still recording each distinct outer node in node_map, and adds a regression test.
Changes:
- Replace unconditional
assert/assigntorename_mapwith a conditional update in the already-fully-mapped branch. - Add
test_if_mover_two_accessnodes_same_outer_datareproducing and verifying the fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/gt4py/next/program_processors/runners/dace/transformations/move_dataflow_into_if_body.py | Skip overwriting existing rename_map entry; always record new outer_node in node_map. |
| tests/.../test_move_dataflow_into_if_body.py | New regression test with two distinct outer AccessNode("a") objects feeding a relocatable tasklet. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
AssertionErrorin_replicate_dataflow_into_branchtriggered when two distinctAccessNodeobjects carrying the same outer array name both feed input connectors of a relocatableTasklet. The first node correctly establishesrename_map[(outer_data, branch_state)]; the second hit theassert … not in rename_mapguard even though the mapping was already valid.rename_mapwhen the entry already exists (first mapping wins) while still recording the newouter_nodeinnode_mapso edge-creation logic can find it.test_if_mover_two_accessnodes_same_outer_datathat constructs an SDFG where array"a"is fed into a Map through two separateAccessNodeobjects and verifies the transformation applies without crashing.Test plan
pytest tests/next_tests/unit_tests/program_processor_tests/runners_tests/dace_tests/transformation_tests/test_move_dataflow_into_if_body.py -k test_if_mover_two_accessnodes_same_outer_datatest_move_dataflow_into_if_body.pytest file to check for regressions🤖 Generated with Claude Code