Skip to content

Add dim order assert #7561

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 14, 2025
Merged

Add dim order assert #7561

merged 4 commits into from
Jan 14, 2025

Conversation

@neuropilot-captain
Copy link
Collaborator

@neuropilot-captain neuropilot-captain commented Jan 9, 2025

  • Add assert to AOT and runtime

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 9, 2025 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit 3c7c854 with merge base f370e78 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 9, 2025
cccclai
cccclai reviewed Jan 9, 2025
View reviewed changes
Copy link
Contributor

@cccclai cccclai left a comment

Choose a reason for hiding this comment

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

Thank you for supporting dim order!

@cccclai cccclai added partner: mediatek Issues related to the Mediatek delegate topic: not user facing labels Jan 9, 2025
@cccclai
Copy link
Contributor

cccclai commented Jan 9, 2025 •
edited by digantdesai
Loading

@digantdesai @Gasoonjia should we use this pass https://github.com/search?q=repo%3Apytorch%2Fexecutorch%20DimOrderOpsRevertPass&type=code instead of not partition?

@digantdesai - Yes, #4873 has more info. If you were lowering it before, which seems to be the case here, then you might also want to lower the dim_order variant of the same target.

@neuropilot-captain
Copy link
Collaborator Author

neuropilot-captain commented Jan 13, 2025

Fix Linterror

digantdesai
digantdesai reviewed Jan 14, 2025
View reviewed changes
digantdesai
digantdesai approved these changes Jan 14, 2025
View reviewed changes
backends/mediatek/partitioner.py
op_type = node.target.__name__

# Skip until we can handle the dimension order representation
if op_type == "aten._to_copy.default":
Copy link
Contributor

@digantdesai digantdesai Jan 14, 2025

Choose a reason for hiding this comment

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

I assume you don't partition empty - that is another one of the dim_order ops. Just FYI.

@digantdesai digantdesai merged commit cd0fcc2 into pytorch:main Jan 14, 2025
5 checks passed
@Gasoonjia
Copy link
Contributor

Gasoonjia commented Jan 14, 2025

@pytorchbot cherry-pick --onto release/0.5 -c fixnewfeature

pytorchbot pushed a commit that referenced this pull request Jan 14, 2025
@neuropilot-captain @MediaTek-ai @pytorchbot
Add dim order assert (#7561)
95cbf41 
* Add dim order assert for MTK backend

* Add dim order guard for mediatek backend AOT part

* Fix lintrunner error

* Remove unnecessary code

---------

Co-authored-by: neuropilot-captain <[email protected]>
Co-authored-by: Poyuan Jeng <[email protected]>
(cherry picked from commit cd0fcc2)
@pytorchbot
Copy link
Collaborator

pytorchbot commented Jan 14, 2025

Cherry picking #7561

The cherry pick PR is at #7660 and it is recommended to link a fixnewfeature cherry pick PR with an issue. The following tracker issues are updated:

Details for Dev Infra team Raised by workflow job

@pytorchbot pytorchbot mentioned this pull request Jan 14, 2025
Closed
SS-JIA pushed a commit that referenced this pull request Jan 15, 2025
@pytorchbot @neuropilot-captain
Add dim order assert (#7660)
187d288 
Add dim order assert (#7561)

* Add dim order assert for MTK backend

* Add dim order guard for mediatek backend AOT part

* Fix lintrunner error

* Remove unnecessary code

---------

Co-authored-by: neuropilot-captain <[email protected]>
Co-authored-by: Poyuan Jeng <[email protected]>
(cherry picked from commit cd0fcc2)

Co-authored-by: neuropilot-captain <[email protected]>
YIWENX14 pushed a commit that referenced this pull request Jan 28, 2025
@neuropilot-captain @MediaTek-ai @YIWENX14
Add dim order assert (#7561)
1127d86 
* Add dim order assert for MTK backend

* Add dim order guard for mediatek backend AOT part

* Fix lintrunner error

* Remove unnecessary code

---------

Co-authored-by: neuropilot-captain <[email protected]>
Co-authored-by: Poyuan Jeng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cccclai cccclai cccclai approved these changes

@digantdesai digantdesai digantdesai approved these changes

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. partner: mediatek Issues related to the Mediatek delegate topic: not user facing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@neuropilot-captain @cccclai @Gasoonjia @pytorchbot @digantdesai @facebook-github-bot @MediaTek-ai