-
Notifications
You must be signed in to change notification settings - Fork 737
Add warning for deprecated to_edge() flow with CoreML #15963
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
base: main
Are you sure you want to change the base?
Conversation
This commit adds a warning message in the CoreML partitioner's partition() method to alert users when they use the older to_edge() flow instead of the recommended to_edge_transform_and_lower() flow. The warning informs users that: - Using the old flow may result in performance regression - The recommended approach is to use to_edge_transform_and_lower() with CoreML partitioner - Provides a link to the CoreML backend documentation for more details Fixes pytorch#15960
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15963
Note: Links to docs will display an error until the docs builds have been completed. ❗ 2 Active SEVsThere are 2 currently active SEVs. If your PR is affected, please view them below: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @Lokesh9106! 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 cla@meta.com. Thanks! |
This PR needs a
|
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Great start! But I'm afraid adding a warning inside the partitioner itself isn't sufficient because the partitioner is also used in the to_edge_lower_and_transform flow. Perhaps you could take a look at the similar change for XNNPACK: #13209 We should also add some unit tests showing: 1) the warning is logged with to_edge, but 2) not logged with to_edge_lower_and_transform. |
|
Thank you for your appreciation
Can I make changes and resubmit it
Get Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: Scott Roy ***@***.***>
Sent: Tuesday, November 25, 2025 12:59:05 AM
To: pytorch/executorch ***@***.***>
Cc: BELLAM LOKESH ***@***.***>; Mention ***@***.***>
Subject: Re: [pytorch/executorch] Add warning for deprecated to_edge() flow with CoreML (PR #15963)
[https://avatars.githubusercontent.com/u/161522778?s=20&v=4]metascroy left a comment (pytorch/executorch#15963)<#15963 (comment)>
Great start! But I'm afraid adding a warning inside the partitioner itself isn't sufficient because the partitioner is also used in the to_edge_lower_and_transform flow.
Perhaps you could take a look at the similar change for XNNPACK: #13209<#13209>
We should also add some unit tests showing: 1) the warning is logged with to_edge, but 2) not logged with to_edge_lower_and_transform.
—
Reply to this email directly, view it on GitHub<#15963 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BXDRGNQCC4SIIILM7WQWYDL36NMADAVCNFSM6AAAAACNB2NXYGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTKNZSGM4DQMRRGU>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Absolutely! |
… workflow Following the pattern from PR pytorch#13209 (XNNPACK), this commit: 1. Adds `import inspect` to enable call stack inspection 2. Adds `_check_if_called_from_to_backend()` helper method that: - Returns False if called from to_edge_transform_and_lower (recommended flow) - Returns True if called from to_backend in deprecated flow 3. Wraps the deprecation warning in a conditional check This ensures the warning only appears when using the deprecated to_edge() + to_backend() workflow, not when using the recommended to_edge_transform_and_lower() flow. Unit tests will be added in a separate commit.
Adds two unit tests to verify the deprecation warning behavior: 1. `test_deprecation_warning_for_to_backend_workflow`: Verifies that the warning IS logged when using the deprecated to_edge() + to_backend() workflow 2. `test_no_warning_for_to_edge_transform_and_lower_workflow`: Verifies that the warning is NOT logged when using the recommended to_edge_transform_and_lower() workflow Also adds necessary imports: - import io - import logging - from executorch.exir import to_edge, to_edge_transform_and_lower These tests ensure the deprecation warning only appears in the deprecated flow, following the same pattern as XNNPACK (PR pytorch#13209).
This commit adds a warning message in the CoreML partitioner's partition() method to alert users when they use the older to_edge() flow instead of the recommended to_edge_transform_and_lower() flow.
The warning informs users that:
Fixes #15960