-
Notifications
You must be signed in to change notification settings - Fork 722
EfficientSAM Model Export #6659
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/6659
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit cd7d115 with merge base 98e4dd5 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @jakmro! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Please seek CI approval before scheduling CIFlow labels |
|
Please seek CI approval before scheduling CIFlow labels |
iseeyuan
left a comment
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 for adding this example! Please add more inline comments to the source files: cite sources and highlight modifications if there are any.
| @@ -0,0 +1,316 @@ | |||
| # Copyright (c) Meta Platforms, Inc. and affiliates. | |||
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.
Could you cite the source of this file, as well as the encoder/decoder?
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.
Could you also highlight if there are any modifications to make it exportable to ExecuTorch?
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.
Yes, of course.
|
|
||
| Follow the [tutorial](https://pytorch.org/executorch/main/getting-started-setup#) to set up ExecuTorch. | ||
|
|
||
| ## 2. Exports |
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.
Are there any performance numbers of those two backend? Would be great to list them here.
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.
I have just listed some performance numbers.
…ports, add performance metrics
iseeyuan
left a comment
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.
LGTM! Thanks for updating!
|
Hi @iseeyuan, what are the next steps? Is there anything else I can do? |
|
@iseeyuan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jakmro , could you rebase your PR and make sure all CI tests pass? Referring to https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#pull-requests, if you have write permission, please go ahead and merge it. If you don't have write permission please let me know and I can merge it for you. |
|
@pytorchbot rebase -b main |
|
@iseeyuan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@iseeyuan There are still two Facebook Internal CI checks failing, but I don’t have access to the logs. Could you help me understand what’s causing the issue? Thanks! |
Unfortunately, I also don't have write permission. |
|
@pytorchbot rebase -b main |
|
@jakmro Let me look into it then. I've disabled internal CIs because they are already covered by OSS CI. I'm rebasing it and see if the internal CI failure is gone. |
|
@pytorchbot rebase |
|
@iseeyuan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jakmro The PR has been merged. The major issue is from internal tests, where it does not allow accessing external urls. I've disabled the internal tests since they are already covered by OSS CI. Next time when a new model is added, we get all signals from OSS that all contributors can see, and the efficiency should be much better. Thanks for your patience! |
|
Thanks for the update and the code review. |
Summary
This PR introduces the EfficientSAM model, along with the instructions for exporting it to Core ML and XNNPACK using ExecuTorch.
Integrating the export-ready EfficientSAM example into ExecuTorch broadens the range of available examples.
Test plan
examples/models/efficient_sam/README.mdto export the model to Core ML and XNNPACK.test_efficient_sam_export_to_executorchinexamples/models/test/test_export.py.