Skip to content
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

Support devices and delegates parameters in android-perf workflow #4484

Closed
wants to merge 1 commit into from

Conversation

guangy10
Copy link
Contributor

@guangy10 guangy10 commented Jul 31, 2024

Summary:

The workflow can support parameters models, devices, delegates.
The workflow is able to translate devices to device-pool-arn and pass to the mobile_job for scheduling
The model is packed and uploaded to artifacts/{model}_{backend}/model.zip

Note: We should only expect the scheduling is working correctly by passing models, devices and delegates. The actual job may not finish successfully because the underlying model/delegate/device are not supported yet.

Test:

The workflow scheduling is working as expected: https://github.com/pytorch/executorch/actions/runs/10187260307

Copy link

pytorch-bot bot commented Jul 31, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 73144c5 with merge base 4483bb6 (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 Jul 31, 2024
@guangy10 guangy10 force-pushed the android_perf_workflow_params branch 4 times, most recently from f488eb2 to 0f602a2 Compare July 31, 2024 20:16
@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@huydhn huydhn left a comment

Choose a reason for hiding this comment

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

LGTM! Just a note that this setup means that when there are multiple models, all of them need to be exported successfully before the benchmark job could start. It is simpler but people might want more flexibility to stop models that fail to export while continue with models that are exported successfully. We can consider that feature later if there is an ask for it.

@guangy10
Copy link
Contributor Author

guangy10 commented Aug 1, 2024

LGTM! Just a note that this setup means that when there are multiple models, all of them need to be exported successfully before the benchmark job could start. It is simpler but people might want more flexibility to stop models that fail to export while continue with models that are exported successfully. We can consider that feature later if there is an ask for it.

Yes, that's a good point. We can address it later when we can support multiple models. It's easy to do but the tricky part is that we don't want it being a surprise if they scheduled to run 10 models but only 1 gets run at the end. We will need to surface the model export failures to users in a more explicit way other than log the failure in the CI, e.g. making it part of the final report or dumped file.

@guangy10 guangy10 force-pushed the android_perf_workflow_params branch from 0f602a2 to 73144c5 Compare August 2, 2024 17:12
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@guangy10 merged this pull request in 0caaf3f.

@guangy10 guangy10 deleted the android_perf_workflow_params branch August 21, 2024 21:03
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. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants