MIGX EP Subgraph debug fixes and cleanup for unsupported ops #196
+69
−42
Conversation
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
Should make debugging easier more clear before we start getting into subGraph optimization, splits and partitioning. Especially important when there's cases where say a subgraph has inputs that seemingly error out and can't be evaluated correctly as the input isn't found but exists in the parent graph (As seen with the IF operator)
Should be clear which node and corresponding input is being filtered out by the MIGraphX EP. This is useful to hunt down operators who may be either malformed or incorrectly handled due to subgraphs/splits/partitioning
Put this logic behind dump_model_as_onnx call to avoid repeated code .
TedThemistokleous
added
Cleanup
Collaborator
Author
|
This will supersede - #195 |
TedThemistokleous
added
the
Upstream
ahsan-ca
reviewed
Nov 7, 2025
| return isInputNode(n, input_name); | ||
| }); | ||
| if (nit == in_nodes.end()) { | ||
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Can't Find node to name"; |
There was a problem hiding this comment.
Suggested change
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Can't Find node to name"; | |
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Cannot find node to name"; |
| "SimplifiedLayerNormalization", | ||
| "Sin", | ||
| "Sinh", | ||
| "Size", |
There was a problem hiding this comment.
Question; Is this needed for other changes in this? Wondering if it makes more sense to add this in a separate PR.
Collaborator
Author
There was a problem hiding this comment.
Its a one line change it was needed for a customer. Bundling all that work into on PR.
Comment on lines
+821
to
+826
| // if graph is split we dont know if output is used so we need this, otherwise if the graph isn't split | ||
| // then we can safely assume this output is a dangling output from a node and to discard it as part of the | ||
| // final graph output | ||
| if (is_graph_split) { | ||
| output_names.push_back(name); | ||
| } |
There was a problem hiding this comment.
Is there a change you made here? Seems identical except the format of braces.
Not sure why the comment is also showing as a change.
Collaborator
Author
There was a problem hiding this comment.
Formatting with lintrunner
|
|
||
| // Input cannot be constant folded | ||
| if (IsGraphInput(graph, input_name)) { | ||
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Can't be const folded"; |
There was a problem hiding this comment.
Suggested change
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Can't be const folded"; | |
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Cannot be const folded"; |
| } | ||
|
|
||
| if (!canEvalShapeGeneral(graph, *nit, input_nodes)) { | ||
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Can't eval shape"; |
There was a problem hiding this comment.
Suggested change
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Can't eval shape"; | |
| LOGS_DEFAULT(WARNING) << "Node:" << node->Name() << " Input:" << input_name << " Cannot eval shape"; |
Collaborator
Author
|
@apwojcik should cherry pick this to wml main since it has the debug code broken away from the dump_model_ops if you're hitting unsupported ops or issues with preprocessing operators with invalid inputs for some of the newer graphs seen on the windows/UAI side. |
TedThemistokleous
merged commit c4e1e69
into
rocm7.1_internal_testing
5 of 7 checks passed
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.
Description
Motivation and Context
Changes found for control flow, size operator and other debug fixes and cleanup
We want to be clear there's an unsupported op in the run on initial get capabilities to be clear where graph break occur
The requiring dump model ops code wasn't clear to use when we have unsupported ops to the user if there is missing support
Added some additional debug output (warnings) if an input op isn't supported due to issues with evaluation. This isn't obvious if an operator fails due to shape or const tensor inputs.