Update kernel matching logic: decouple from op schemas and remove kernel def hashes #12791
Conversation
This file contains 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
…lasses and fix usages.
edgchen1
commented
Sep 15, 2022
|
This pull request introduces 1 alert and fixes 1 when merging da5c9f4 into 739b567 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 6 alerts and fixes 1 when merging 716b756 into b935524 - view on LGTM.com new alerts:
fixed alerts:
|
edgchen1
commented
Sep 16, 2022
onnxruntime/core/optimizer/selectors_actions/selector_action_transformer.cc
Show resolved
Hide resolved
edgchen1
commented
Sep 16, 2022
edgchen1
commented
Sep 16, 2022
| @@ -593,7 +593,7 @@ static void CustomKernelWithCustomSchema() { | |||
| floatTensorEdgeDesc.edgeType = MLOperatorEdgeType::Tensor; | |||
| floatTensorEdgeDesc.tensorDataType = MLOperatorTensorDataType::Float; | |||
|
|
|||
There was a problem hiding this comment.
🤔 Looks like a custom op WinML test. @smk2007? Seems okay 🤷♂️ (T to T1 makes sense, given the definition above), but I'm content if the test passes.
edgchen1
commented
Sep 17, 2022
| @@ -790,8 +790,6 @@ class ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 17, ST | |||
| // !!PLEASE READ BELOW!! Following that, add new entries above this comment | |||
|
|
|||
| /* *** IMPORTANT! *** | |||
| If kernel registrations are incorrectly updated, ORT format models get broken as the kernel hashes may be invalidated. | |||
There was a problem hiding this comment.
@skottmckay should this be replaced another comment? I think it's still important to correctly register kernels
There was a problem hiding this comment.
How would they register it correctly if the kernel def builder no longer generates a hash?
snnn
approved these changes
Sep 17, 2022
|
This pull request introduces 1 alert and fixes 1 when merging 396a957 into b48f71f - view on LGTM.com new alerts:
fixed alerts:
|
|
/azp run orttraining-linux-gpu-ci-pipeline |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jeffbloo
approved these changes
Sep 20, 2022
|
/azp run Linux Eager Mode CI Pipeline |
|
Azure Pipelines successfully started running 1 pipeline(s). |
skottmckay
approved these changes
Sep 20, 2022
Merged
linnealovespie
pushed a commit
that referenced
this pull request
Sep 30, 2022
…nel def hashes (#12791) # Motivation Currently, ORT minimal builds use kernel def hashes to map from nodes to kernels to execute when loading the model. As the kernel def hashes must be known ahead of time, this works for statically registered kernels. This works well for the CPU EP. For this approach to work, the kernel def hashes must also be known at ORT format model conversion time, which means the EP with statically registered kernels must also be enabled then. This is not an issue for the always-available CPU EP. However, we do not want to require that any EP which statically registers kernels is always available too. Consequently, we explore another approach to match nodes to kernels that does not rely on kernel def hashes. An added benefit of this is the possibility of moving away from kernel def hashes completely, which would eliminate the maintenance burden of keeping the hashes stable. # Approach In a full build, ORT uses some information from the ONNX op schema to match a node to a kernel. We want to avoid including the ONNX op schema in a minimal build to reduce binary size. Essentially, we take the necessary information from the ONNX op schema and make it available in a minimal build. We decouple the ONNX op schema from the kernel matching logic. The kernel matching logic instead relies on per-op information which can either be obtained from the ONNX op schema or another source. This per-op information must be available in a minimal build when there are no ONNX op schemas. We put it in the ORT format model. Existing uses of kernel def hashes to look up kernels are replaced with the updated kernel matching logic. We no longer store kernel def hashes in the ORT format model’s session state and runtime optimization representations. We no longer keep the logic to generate and ensure stability of kernel def hashes.
siweic0
pushed a commit
to siweic0/onnxruntime-web
that referenced
this pull request
May 9, 2024
…nel def hashes (microsoft#12791) # Motivation Currently, ORT minimal builds use kernel def hashes to map from nodes to kernels to execute when loading the model. As the kernel def hashes must be known ahead of time, this works for statically registered kernels. This works well for the CPU EP. For this approach to work, the kernel def hashes must also be known at ORT format model conversion time, which means the EP with statically registered kernels must also be enabled then. This is not an issue for the always-available CPU EP. However, we do not want to require that any EP which statically registers kernels is always available too. Consequently, we explore another approach to match nodes to kernels that does not rely on kernel def hashes. An added benefit of this is the possibility of moving away from kernel def hashes completely, which would eliminate the maintenance burden of keeping the hashes stable. # Approach In a full build, ORT uses some information from the ONNX op schema to match a node to a kernel. We want to avoid including the ONNX op schema in a minimal build to reduce binary size. Essentially, we take the necessary information from the ONNX op schema and make it available in a minimal build. We decouple the ONNX op schema from the kernel matching logic. The kernel matching logic instead relies on per-op information which can either be obtained from the ONNX op schema or another source. This per-op information must be available in a minimal build when there are no ONNX op schemas. We put it in the ORT format model. Existing uses of kernel def hashes to look up kernels are replaced with the updated kernel matching logic. We no longer store kernel def hashes in the ORT format model’s session state and runtime optimization representations. We no longer keep the logic to generate and ensure stability of kernel def hashes.
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.
Motivation
Currently, ORT minimal builds use kernel def hashes to map from nodes to kernels to execute when loading the model. As the kernel def hashes must be known ahead of time, this works for statically registered kernels. This works well for the CPU EP.
For this approach to work, the kernel def hashes must also be known at ORT format model conversion time, which means the EP with statically registered kernels must also be enabled then. This is not an issue for the always-available CPU EP. However, we do not want to require that any EP which statically registers kernels is always available too.
Consequently, we explore another approach to match nodes to kernels that does not rely on kernel def hashes. An added benefit of this is the possibility of moving away from kernel def hashes completely, which would eliminate the maintenance burden of keeping the hashes stable.
Approach
In a full build, ORT uses some information from the ONNX op schema to match a node to a kernel. We want to avoid including the ONNX op schema in a minimal build to reduce binary size. Essentially, we will take the necessary information from the ONNX op schema and make it available in a minimal build.
We will decouple the ONNX op schema from the kernel matching logic. The kernel matching logic will instead rely on per-op information which can either be obtained from the ONNX op schema or another source.
This per-op information must be available in a minimal build when there are no ONNX op schemas. We can put it in the ORT format model.
Existing uses of kernel def hashes to look up kernels can be replaced with the updated kernel matching logic. We no longer need to store kernel def hashes in the ORT format model’s session state and runtime optimization representations. We no longer need to keep the logic to generate and ensure stability of kernel def hashes.