Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ONNX][TypePromo] Explicit type promotion pass #104064

Closed
wants to merge 10 commits into from

Conversation

BowenBao
Copy link
Collaborator

@BowenBao BowenBao commented Jun 22, 2023

Stack from ghstack (oldest at bottom):

This PR adds the ExplicitTypePromotionPass that does an fx graph to fx graph transformation
explicitly adding cast nodes into the graph to emulate the PyTorch type promotion behavior.

Full design doc and discussion at https://microsoft-my.sharepoint.com/:w:/p/bowbao/Edj2lF1oi0JIitT_3ntyuqkBo6ll7N6NJDmavM0lp_KkEA?e=OElyjR

@pytorch-bot pytorch-bot bot added the release notes: onnx torch.onnx related changes that should show up in the release notes label Jun 22, 2023
@pytorch-bot
Copy link

pytorch-bot bot commented Jun 22, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/104064

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure

As of commit ff924c3:

NEW FAILURE - The following job has failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

BowenBao added a commit that referenced this pull request Jun 22, 2023
ghstack-source-id: 5dafa469aa5e04e44da1e049d480324ba08e0bc2
Pull Request resolved: #104064
…on "[ONNX][TypePromo] Explicit type promotion pass"

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jun 27, 2023
ghstack-source-id: b6ecfb481d3efdfceb4e5744bfffa799d6ad4631
Pull Request resolved: #104064
@@ -556,7 +490,7 @@
and sample.kwargs.get("padding") == 1,
reason="FIXME: After https://github.com/microsoft/onnxruntime/issues/15446 is fixed",
),
xfail(
skip(
Copy link
Collaborator Author

@BowenBao BowenBao Jun 27, 2023

Choose a reason for hiding this comment

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

After type promotion this test starts to fail a few subtests with unexpected pass, which appears to be inconsistent with the tagged xfail reason.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually more likely due to the bumping onnxscript version in this PR, so unrelated to type promotion. The inconsistency concern still holds.

…multiple ops on "[ONNX][TypePromo] Explicit type promotion pass"


This PR adds the `ExplicitTypePromotionPass` that does an fx graph to fx graph transformation
explicitly adding cast nodes into the graph to emulate the PyTorch type promotion behavior.

Full design doc and discussion at https://microsoft-my.sharepoint.com/:w:/p/bowbao/Edj2lF1oi0JIitT_3ntyuqkBo6ll7N6NJDmavM0lp_KkEA?e=OElyjR


[ghstack-poisoned]
@BowenBao BowenBao marked this pull request as ready for review June 27, 2023 18:44
@BowenBao BowenBao added the topic: new features topic category label Jun 27, 2023
@BowenBao BowenBao requested review from justinchuby and removed request for jeffdaily June 27, 2023 18:49
@titaiwangms titaiwangms self-requested a review June 28, 2023 00:20
@BowenBao
Copy link
Collaborator Author

I will update new diagnostic rule id prior to merging.

torch/onnx/_internal/diagnostics/_rules.py Outdated Show resolved Hide resolved
torch/onnx/_internal/diagnostics/rules.yaml Outdated Show resolved Hide resolved
torch/onnx/_internal/diagnostics/rules.yaml Show resolved Hide resolved
torch/onnx/_internal/diagnostics/rules.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@thiagocrepaldi thiagocrepaldi left a comment

Choose a reason for hiding this comment

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

This PR has a lot of complexity and very little documentation. Hard to follow

torch/onnx/_internal/diagnostics/_rules.py Outdated Show resolved Hide resolved
torch/onnx/_internal/fx/passes/type_promotion.py Outdated Show resolved Hide resolved
torch/onnx/_internal/fx/passes/type_promotion.py Outdated Show resolved Hide resolved
torch/onnx/_internal/fx/passes/type_promotion.py Outdated Show resolved Hide resolved
…romotion pass"


This PR adds the `ExplicitTypePromotionPass` that does an fx graph to fx graph transformation
explicitly adding cast nodes into the graph to emulate the PyTorch type promotion behavior.

Full design doc and discussion at https://microsoft-my.sharepoint.com/:w:/p/bowbao/Edj2lF1oi0JIitT_3ntyuqkBo6ll7N6NJDmavM0lp_KkEA?e=OElyjR


[ghstack-poisoned]
This PR adds the `ExplicitTypePromotionPass` that does an fx graph to fx graph transformation
explicitly adding cast nodes into the graph to emulate the PyTorch type promotion behavior.

Full design doc and discussion at https://microsoft-my.sharepoint.com/:w:/p/bowbao/Edj2lF1oi0JIitT_3ntyuqkBo6ll7N6NJDmavM0lp_KkEA?e=OElyjR


[ghstack-poisoned]
@thiagocrepaldi thiagocrepaldi added the module: onnx Related to torch.onnx label Jul 5, 2023
This PR adds the `ExplicitTypePromotionPass` that does an fx graph to fx graph transformation
explicitly adding cast nodes into the graph to emulate the PyTorch type promotion behavior.

Full design doc and discussion at https://microsoft-my.sharepoint.com/:w:/p/bowbao/Edj2lF1oi0JIitT_3ntyuqkBo6ll7N6NJDmavM0lp_KkEA?e=OElyjR


[ghstack-poisoned]
@BowenBao BowenBao added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 6, 2023
…Explicit type promotion pass"


This PR adds the `ExplicitTypePromotionPass` that does an fx graph to fx graph transformation
explicitly adding cast nodes into the graph to emulate the PyTorch type promotion behavior.

Full design doc and discussion at https://microsoft-my.sharepoint.com/:w:/p/bowbao/Edj2lF1oi0JIitT_3ntyuqkBo6ll7N6NJDmavM0lp_KkEA?e=OElyjR


[ghstack-poisoned]
@BowenBao
Copy link
Collaborator Author

BowenBao commented Jul 7, 2023

@pytorchbot merge -f "unrelated cudnn failure"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@facebook-github-bot facebook-github-bot deleted the gh/BowenBao/254/head branch July 11, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: onnx Related to torch.onnx open source release notes: onnx torch.onnx related changes that should show up in the release notes topic: new features topic category
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants