-
Notifications
You must be signed in to change notification settings - Fork 603
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
NXP backend: Enable initial unit tests workflow #10258
Conversation
🔗 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 ( 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. |
@pytorchbot label "module: nxp" |
@pytorchbot label "release notes: nxp" |
.github/workflows/pull.yml
Outdated
|
||
# Build and install Executorch | ||
PYTHON_EXECUTABLE=python \ | ||
CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_NXP_NEUTRON=ON" \ |
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.
Curious, why do we need to enable PYBIND
?
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.
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.
a850383
to
879128d
Compare
.github/workflows/pull.yml
Outdated
@@ -513,6 +513,32 @@ jobs: | |||
# Run pytest without simulator | |||
backends/arm/test/test_arm_baremetal.sh test_pytest | |||
|
|||
unittest-nxp-neutron: |
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.
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.
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.
For my understanding: what does that mean? run on every pull request vs run only on stable?
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.
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).
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.
yeah once we get more tests/mature backend we can make it on "every pr"
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.
FYI we have to add a ciflow/trunk
label to run these jobs.
879128d
to
f6f4996
Compare
Running CI, once green I will merge. Thanks again. |
f6f4996
to
a608d0f
Compare
Rebased once again to include this fix #10838 |
Rebased to include tests fix: e7d39c2 @digantdesai Can you please include |
a608d0f
to
2b4aff1
Compare
@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:
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)
2b4aff1
to
40931af
Compare
Rebased. @JakeStevens Can you please re-run CI? |
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 |
This reverts commit b805f17.
Summary
Initial workflow definition for running unit tests for Neutron (NXP) backend.
cc @digantdesai @JakeStevens @robert-kalmar