-
Notifications
You must be signed in to change notification settings - Fork 608
Refactor XNNPACK tester to extract delegate-independent tester classes #11596
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11596
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 26 PendingAs of commit b3f192c with merge base 7503bb3 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
2b2a61d
to
2d696bd
Compare
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
2d696bd
to
ee651da
Compare
This pull request was exported from Phabricator. Differential Revision: D76547310 |
ee651da
to
beef6d8
Compare
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
This pull request was exported from Phabricator. Differential Revision: D76547310 |
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
beef6d8
to
c6bc99b
Compare
This pull request was exported from Phabricator. Differential Revision: D76547310 |
c6bc99b
to
cc8ac82
Compare
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
This pull request was exported from Phabricator. Differential Revision: D76547310 |
cc8ac82
to
552d35c
Compare
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
552d35c
to
39c729a
Compare
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
This pull request was exported from Phabricator. Differential Revision: D76547310 |
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
39c729a
to
c17e2c6
Compare
Note to reviewers: let me know if you have strong feelings on string vs enum keys for stages. I originally switched it to a enum to try to get away from the local stage registration in XNNPACK. But I could put it back to use string keys everywhere. CC @mcr229 |
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
c17e2c6
to
09048fa
Compare
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
09048fa
to
6ab0c84
Compare
I'm proposing moving this code to https://github.com/pytorch/executorch/tree/main/exir/backend under |
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very nice refactor Gregory. Thanks for doing it
StageType.PARTITION, | ||
StageType.TO_EDGE_TRANSFORM_AND_LOWER, | ||
], | ||
# TODO Make this Stage optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you just do this by making TO_EDGE either go to TO_EXECUTORCH or PARTITION. Also I know its a bit of a headache now, but I wash oping to change the name of partition to TO_BACKEND. If its not too much of a hassle to do. otherwise it's a very minor nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing majority of this is copied from the XNNPACK Tester, so looks fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you just do this by making TO_EDGE either go to TO_EXECUTORCH or PARTITION. Also I know its a bit of a headache now, but I wash oping to change the name of partition to TO_BACKEND. If its not too much of a hassle to do. otherwise it's a very minor nit
Makes sense. I'll put out a follow-up PR to rename partition to to_backend. The other TODO was moved from the previous location, but seems easy to fix, so I'll do it as well.
Sounds good. I don't have a particularly strong opinion on this. I'm happy to discuss more. If we move it, I'll put out another PR to do this since I have a pretty large stack on top of this. |
Summary: Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness. I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum. It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor. Pull Request resolved: pytorch#11596 Test Plan: CI Rollback Plan: Differential Revision: D76547310 Pulled By: GregoryComer
6ab0c84
to
b3f192c
Compare
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary
Refactor the XNNPACK tester to split out reusable base components from XNNPACK-specific parts. I've relocated the base classes to backends/test/harness.
I've kept the tester structure pretty much unchanged, except for replacing stage names with an enum.
It looks like Arm tests are directly importing for XNNPACK's tester currently. Ideally, we'll want to refactor to have their own stage implementations, but I've left that as a follow-up to minimize changes for the initial refactor.
Test plan
CI