Extend optimize_for_ort to cover passes #2274
Conversation
titaiwangms
requested review from
gramalingam,
shubhambhokare1,
justinchuby and
xadupre
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2274 +/- ##
==========================================
- Coverage 73.76% 73.75% -0.02%
==========================================
Files 239 239
Lines 30904 30907 +3
Branches 3494 3494
==========================================
- Hits 22797 22796 -1
- Misses 6907 6911 +4
Partials 1200 1200 ☔ View full report in Codecov by Sentry. |
|
Please also consider whether this method should be optimize in-place or not. I think we can make it in-place now that shape-inference itself is in-place. |
I think making it out-of-place is safer, in case we have passes in the future that need to be functional? |
| # https://github.com/microsoft/onnxruntime/blob/74dcf7e296639095dfa55d31336998b6f719ed76/onnxruntime/python/tools/transformers/dynamo_onnx_helper.py#L172 | ||
| common_passes.ClearMetadataAndDocStringPass(), | ||
| # https://github.com/microsoft/onnxruntime/blob/74dcf7e296639095dfa55d31336998b6f719ed76/onnxruntime/python/tools/transformers/dynamo_onnx_helper.py#L139 | ||
| common_passes.LiftConstantsToInitializersPass(lift_all_constants=False, size_limit=1), |
There was a problem hiding this comment.
We have another pass called LiftSubgraphInitializersToMainGraphPass. Do we know if it's needed in genAI? @kunal-vaishnavi
There was a problem hiding this comment.
If the pass logic is in DynamoOnnxHelper, then it is used for ONNX Runtime GenAI.
There was a problem hiding this comment.
We don't really produce graphs with subgraph initializers. I think we are ok either way
| rewrite(model, ORT_PATTERN_REWRITE_RULES) | ||
| return model, fusion_count | ||
|
|
||
| passes = [ |
There was a problem hiding this comment.
| passes = [ | |
| passes = ir.passes.Sequential( |
| ] | ||
| optimize_for_ort_passes = ir.passes.Sequential(*passes) |
There was a problem hiding this comment.
| ] | |
| optimize_for_ort_passes = ir.passes.Sequential(*passes) | |
| ) |
|
I will set up Whisper and test it before merge this PR microsoft/onnxruntime#24382 |
|
Do you intend to merge this? |
Do we still need this? I can check again and land it. |
Fix #2261
A draft for discussion. We should cover all post-processing the model shipping needs