Skip to content

Fix non-deterministic rewriter behavior in multi-output pattern matching#2880

Merged
justinchuby merged 2 commits intomainfrom
copilot/fix-non-deterministic-rewriter-behavior
Apr 2, 2026
Merged

Fix non-deterministic rewriter behavior in multi-output pattern matching#2880
justinchuby merged 2 commits intomainfrom
copilot/fix-non-deterministic-rewriter-behavior

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 2, 2026

GraphPattern.__init__ collects output nodes into a set[NodePattern], then converts to a list via list(output_nodes). Python's hash randomization means this list has non-deterministic order across process invocations. For multi-output patterns (e.g. SlicesSplit), the matcher fixes output_nodes[0] to the current node and searches for output_nodes[1:] — so different orderings cause the same rule to match or not match non-deterministically.

  • Replace set[NodePattern] with dict[NodePattern, None] (ordered set idiom) to preserve deterministic insertion order from the outputs sequence
  • Add regression test verifying output_nodes ordering is stable across repeated GraphPattern constructions
# Before: non-deterministic iteration order
output_nodes: set[NodePattern] = set()
...
output_nodes.add(candidate)
...
self.output_nodes: list[NodePattern] = list(output_nodes)

# After: preserves insertion order from outputs sequence
output_nodes: dict[NodePattern, None] = {}
...
output_nodes[candidate] = None
...
self.output_nodes: list[NodePattern] = list(output_nodes)

Fix #2878

… of set for output_nodes

Replace set[NodePattern] with dict[NodePattern, None] to preserve deterministic
insertion order from the outputs sequence. Python sets have non-deterministic
iteration order due to hash randomization, which caused the multi-output pattern
matching to behave differently across runs.

Fixes #2234

Agent-Logs-Url: https://github.com/microsoft/onnxscript/sessions/50f46d7a-beee-47c4-9369-3c28417380d1

Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix non-deterministic rewriter behavior in onnxscript Fix non-deterministic rewriter behavior in multi-output pattern matching Apr 2, 2026
Copilot AI requested a review from justinchuby April 2, 2026 00:37
@justinchuby justinchuby requested a review from titaiwangms April 2, 2026 03:41
@justinchuby justinchuby marked this pull request as ready for review April 2, 2026 03:41
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.04%. Comparing base (1077da7) to head (7be2601).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2880      +/-   ##
==========================================
+ Coverage   71.96%   72.04%   +0.07%     
==========================================
  Files         239      239              
  Lines       29224    29305      +81     
  Branches     2878     2880       +2     
==========================================
+ Hits        21031    21112      +81     
  Misses       7216     7216              
  Partials      977      977              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-project-automation github-project-automation Bot moved this from Todo to Done in ONNX Script Review Board Apr 2, 2026
@justinchuby justinchuby merged commit 4291ff2 into main Apr 2, 2026
31 of 34 checks passed
@justinchuby justinchuby deleted the copilot/fix-non-deterministic-rewriter-behavior branch April 2, 2026 14:12
justinchuby added a commit that referenced this pull request Apr 17, 2026
…ing (#2880)

`GraphPattern.__init__` collects output nodes into a `set[NodePattern]`,
then converts to a list via `list(output_nodes)`. Python's hash
randomization means this list has non-deterministic order across process
invocations. For multi-output patterns (e.g. `SlicesSplit`), the matcher
fixes `output_nodes[0]` to the current node and searches for
`output_nodes[1:]` — so different orderings cause the same rule to match
or not match non-deterministically.

- Replace `set[NodePattern]` with `dict[NodePattern, None]` (ordered set
idiom) to preserve deterministic insertion order from the `outputs`
sequence
- Add regression test verifying `output_nodes` ordering is stable across
repeated `GraphPattern` constructions

```python
# Before: non-deterministic iteration order
output_nodes: set[NodePattern] = set()
...
output_nodes.add(candidate)
...
self.output_nodes: list[NodePattern] = list(output_nodes)

# After: preserves insertion order from outputs sequence
output_nodes: dict[NodePattern, None] = {}
...
output_nodes[candidate] = None
...
self.output_nodes: list[NodePattern] = list(output_nodes)
```

Fix #2878

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

non-deterministic rewriter behavior in onnxscript

3 participants