Skip to content

NXP backend: Enable initial unit tests workflow #10258

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

Conversation

skywall
Copy link
Collaborator

@skywall skywall commented Apr 17, 2025

Summary

Initial workflow definition for running unit tests for Neutron (NXP) backend.

cc @digantdesai @JakeStevens @robert-kalmar

Copy link

pytorch-bot bot commented Apr 17, 2025

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 40931af with merge base 7d9dd46 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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 Apr 17, 2025
@skywall
Copy link
Collaborator Author

skywall commented Apr 17, 2025

@pytorchbot label "module: nxp"

@pytorch-bot pytorch-bot bot added the module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ label Apr 17, 2025
@skywall
Copy link
Collaborator Author

skywall commented Apr 17, 2025

@pytorchbot label "release notes: nxp"

@pytorch-bot pytorch-bot bot added the release notes: nxp Changes to the NXP Neutron backend delegate label Apr 17, 2025

# Build and install Executorch
PYTHON_EXECUTABLE=python \
CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_NXP_NEUTRON=ON" \
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, why do we need to enable PYBIND?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, this is my misunderstanding of how EXECUTORCH_BUILD_PYBIND should be used. I have considered it as a "mechanism to build backend-specific dependencies", because I needed to build portable_lib and also libquantized_ops_aot_lib. Removed.

@@ -513,6 +513,32 @@ jobs:
# Run pytest without simulator
backends/arm/test/test_arm_baremetal.sh test_pytest

unittest-nxp-neutron:
Copy link
Contributor

Choose a reason for hiding this comment

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

On one quick follow up, can we move this to trunk.yaml instead of here given early days for the delegate, we don't want to disrupt the regular devx.

Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding: what does that mean? run on every pull request vs run only on stable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to trunk.yaml + rebased.

@JakeStevens trunk.yaml jobs are executed only on push to main, release/* or PR's with changes in CI scripts so you're right (https://github.com/pytorch/executorch/blob/main/.github/workflows/trunk.yml#L4).

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah once we get more tests/mature backend we can make it on "every pr"

Copy link
Contributor

@digantdesai digantdesai May 14, 2025

Choose a reason for hiding this comment

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

FYI we have to add a ciflow/trunk label to run these jobs.

@skywall skywall force-pushed the feature/nxf93343/neutron-unittest-ci branch from 879128d to f6f4996 Compare May 13, 2025 13:56
@digantdesai
Copy link
Contributor

Running CI, once green I will merge. Thanks again.

@skywall skywall force-pushed the feature/nxf93343/neutron-unittest-ci branch from f6f4996 to a608d0f Compare May 14, 2025 11:20
@skywall
Copy link
Collaborator Author

skywall commented May 14, 2025

Rebased once again to include this fix #10838

@skywall
Copy link
Collaborator Author

skywall commented May 15, 2025

Rebased to include tests fix: e7d39c2 @digantdesai Can you please include ciflow/trunk label to re-run tests?

@skywall skywall force-pushed the feature/nxf93343/neutron-unittest-ci branch from a608d0f to 2b4aff1 Compare May 15, 2025 13:55
@skywall
Copy link
Collaborator Author

skywall commented May 16, 2025

@digantdesai Do you please have an idea why the CI fails? Looking at the logs of trunk / unittest-nxp-neutron / linux-job (push) I don't see any problems with job definition itself, but I'm not sure why other similar jobs pass. Maybe some caching issue after moving from pull.yaml to trunk.yaml?

@JakeStevens
Copy link
Contributor

@digantdesai Do you please have an idea why the CI fails? Looking at the logs of trunk / unittest-nxp-neutron / linux-job (push) I don't see any problems with job definition itself, but I'm not sure why other similar jobs pass. Maybe some caching issue after moving from pull.yaml to trunk.yaml?

Error is here:

Error:
2025-05-15 15:42:42,467 [ExecuTorch] ERROR: Failed to query buck for sources. Failed command:

 buck2 cquery inputs(deps('//examples/models/llama/runner:runner'))

Seems like may be unrelated, might be fixed by now. I think the lintrunner issue is also resolved. So perhaps rebase and try again? I think should be green then

Fix duplicate bpe tokenizer base symbol (pytorch#63)
@skywall skywall force-pushed the feature/nxf93343/neutron-unittest-ci branch from 2b4aff1 to 40931af Compare May 19, 2025 06:16
@skywall
Copy link
Collaborator Author

skywall commented May 19, 2025

Rebased. @JakeStevens Can you please re-run CI?

@digantdesai
Copy link
Contributor

Rerunning it, seems unrelated, sorry about that. If you want less "turbulence" I would say feel free to disable things you don't want to build through CMAKE_ARGS for ET

@digantdesai digantdesai merged commit b805f17 into pytorch:main May 22, 2025
88 of 90 checks passed
@skywall skywall deleted the feature/nxf93343/neutron-unittest-ci branch May 22, 2025 06:52
digantdesai added a commit that referenced this pull request May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ release notes: nxp Changes to the NXP Neutron backend delegate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants