Skip to content

set node schema when apply NHWC transformer#13660

Merged
fs-eire merged 2 commits intomainfrom
fs-eire/opt-transformer-node-set-schema
Nov 22, 2022
Merged

set node schema when apply NHWC transformer#13660
fs-eire merged 2 commits intomainfrom
fs-eire/opt-transformer-node-set-schema

Conversation

@fs-eire
Copy link
Contributor

@fs-eire fs-eire commented Nov 15, 2022

Description

set node schema when apply NHWC transformer

Motivation and Context

The implementation in IExecutionProvider::GetCapability() checks node schema to determine the capability of the current EP. If NHWC graph transformer created a new channel last Conv node to replace the channel first Conv node, we need to assign the schema to the replaced node.

@fs-eire fs-eire requested a review from skottmckay November 15, 2022 22:10
skottmckay
skottmckay previously approved these changes Nov 15, 2022
Copy link
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:


onnx_layout_transformation::SwapNodeOpTypeAndDomain(*api_graph, *node, node->OpType(), kMSInternalNHWCDomain);
[[maybe_unused]] auto new_node_ref =
onnx_layout_transformation::SwapNodeOpTypeAndDomain(*api_graph, *node, node->OpType(), kMSInternalNHWCDomain);
Copy link
Contributor

Choose a reason for hiding this comment

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

typically we'd add

ORT_UNUSED_PARAMETER(new_node_ref);

but if all the builds pass with maybe_unused that seems fine as well. only difference would be that if the value becomes unused in the #if as well that wouldn't be noticed. probably doesn't matter though.

Copy link
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

@fs-eire fs-eire merged commit 2bebe61 into main Nov 22, 2022
@fs-eire fs-eire deleted the fs-eire/opt-transformer-node-set-schema branch November 22, 2022 20:26
simon-moo pushed a commit to simon-moo/onnxruntime that referenced this pull request Dec 26, 2022
### Description
set node schema when apply NHWC transformer

### Motivation and Context
The implementation in `IExecutionProvider::GetCapability()` checks node
schema to determine the capability of the current EP. If NHWC graph
transformer created a new channel last `Conv` node to replace the
channel first `Conv` node, we need to assign the schema to the replaced
node.
fs-eire added a commit that referenced this pull request Jan 28, 2023
### Description
as a more generic solution to #13660, always set OpSchema in
CreateNodeHelper() so that added nodes by transformers will have
OpSchema set

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
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