-
Notifications
You must be signed in to change notification settings - Fork 633
Update CoreML model testing in CI #12808
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/12808
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 19 PendingAs of commit 4d7045f with merge base 2c84f70 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
@digantdesai @cccclai can I get a stamp? |
.github/workflows/trunk.yml
Outdated
@@ -590,12 +590,20 @@ jobs: | |||
echo "Finishing installing coreml." | |||
|
|||
# Build and test coreml model | |||
MODELS=(mv3 ic4 resnet50 edsr mobilebert w2l) | |||
for MODEL_NAME in "${MODELS[@]}"; do | |||
for MODEL_NAME in dl3 edsr emformer_join ic3 ic4 mobilebert mv2 mv3 resnet50 vit w2l; do |
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.
@metascroy have you seen this PR?
https://github.com/pytorch/executorch/pull/12371/files
What are your thoughts on consolidating? I can drop my PR but maybe you can take a look and see if there are things worth incorporating?
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.
@mergennachin I missed that PR, but I can incorporate the following changes from your PR here:
- Split CoreML/MPS into two separate jobs
- Merge the model lists from the two PRs
For the runtime checks, I'll proceed with the pybind testing I'm adding here (vs. the C++ runner checking in your PR) because with pybinding we also check model accuracy.
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.
Thank you @metascroy
Changes look great
1a9f07b
to
4d7045f
Compare
This PR updates the alpha/beta CoreML model testing in a few ways: