Fix MatchResult.fail() call signature in redundant_scatter_nd.py#2431
Merged
justinchuby merged 4 commits intomainfrom Jul 3, 2025
Merged
Fix MatchResult.fail() call signature in redundant_scatter_nd.py#2431justinchuby merged 4 commits intomainfrom
justinchuby merged 4 commits intomainfrom
Conversation
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix onnxscript/rewriter/redundant_scatter_nd.py
Fix MatchResult.fail() call signature in redundant_scatter_nd.py
Jul 1, 2025
Collaborator
|
Just create the MatchResult object in check() and call result.fail() inside the method. Remove the fail() function. |
…methods Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Contributor
Author
Done! I've removed the |
❌ 7 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
gramalingam
approved these changes
Jul 2, 2025
Collaborator
|
@copilot please run |
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Contributor
Author
gramalingam
approved these changes
Jul 3, 2025
justinchuby
approved these changes
Jul 3, 2025
titaiwangms
approved these changes
Jul 3, 2025
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.
The
failhelper function inonnxscript/rewriter/redundant_scatter_nd.pywas incorrectly passing multiple arguments toMatchResult.fail(), causing a TypeError when pattern matching failed.Problem
The error occurred when the rewriter tried to report match failures with multiple failure sources:
This resulted in:
The issue was that
MatchResult.fail()only accepts 2 parameters afterself:reason: str- the failure reasonfailure_source: Union[ir.Node, ir.Value, list[...]] | None- a single item or list of failure sourcesBut the helper function was passing all arguments directly:
MatchResult().fail(*args).Solution
Modified the
failhelper function to properly handle multiple failure sources by collecting them into a list when callingMatchResult.fail():This change:
matmul_add_to_gemm.pyTesting
Verified that all existing call patterns in the file work correctly:
fail("message")- reason onlyfail("message", node)- reason + single sourcefail("message", node1, node2)- reason + multiple sourcesFixes #2430.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.