Skip to content

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

Merged
merged 1 commit into from
Jun 17, 2025

Conversation

GregoryComer
Copy link
Member

@GregoryComer GregoryComer commented Jun 12, 2025

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

Copy link

pytorch-bot bot commented Jun 12, 2025

🔗 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 Pending

As of commit b3f192c with merge base 7503bb3 (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 Jun 12, 2025
@GregoryComer GregoryComer added the release notes: none Do not include this in the release notes label Jun 12, 2025
@GregoryComer GregoryComer force-pushed the tester-refactor branch 3 times, most recently from 2b2a61d to 2d696bd Compare June 12, 2025 21:43
@facebook-github-bot
Copy link
Contributor

@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76547310

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 13, 2025
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
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76547310

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 13, 2025
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
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76547310

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 13, 2025
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
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76547310

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 13, 2025
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
GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 14, 2025
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
GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 14, 2025
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
GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 15, 2025
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
@GregoryComer GregoryComer changed the title (WIP) Refactor XNNPACK tester to extract base class Refactor XNNPACK tester to extra delegate-independent tester classes Jun 16, 2025
@GregoryComer GregoryComer marked this pull request as ready for review June 16, 2025 20:37
GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 16, 2025
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
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76547310

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 16, 2025
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
@GregoryComer
Copy link
Member Author

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

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 16, 2025
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
GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Jun 16, 2025
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
@cccclai
Copy link
Contributor

cccclai commented Jun 16, 2025

I'm proposing moving this code to https://github.com/pytorch/executorch/tree/main/exir/backend under executorch/exir/backend/test_framework, such that the strucutre of the backend is indeed a list of backends. I've brought it up to @digantdesai about this before.

@GregoryComer GregoryComer changed the title Refactor XNNPACK tester to extra delegate-independent tester classes Refactor XNNPACK tester to extract delegate-independent tester classes Jun 16, 2025
@facebook-github-bot
Copy link
Contributor

@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@mcr229 mcr229 left a 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
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Member Author

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.

@GregoryComer
Copy link
Member Author

I'm proposing moving this code to https://github.com/pytorch/executorch/tree/main/exir/backend under executorch/exir/backend/test_framework, such that the strucutre of the backend is indeed a list of backends. I've brought it up to @digantdesai about this before.

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
@facebook-github-bot
Copy link
Contributor

@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@GregoryComer GregoryComer merged commit 830631d into pytorch:main Jun 17, 2025
195 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported release notes: none Do not include this in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants