Refactor some APIs #3863
Closed
Refactor some APIs #3863
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/3863
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ No FailuresAs of commit fe62256 with merge base 91c0485 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D58101124 |
|
This pull request was exported from Phabricator. Differential Revision: D58101124 |
mcremon-meta
added a commit
that referenced
this pull request
Jun 7, 2024
…g flow, and call print_ops_info in export_to_executorch (#3863) Summary: Pull Request resolved: #3863 The current setup is unbalanced, especially because we would love to (and should) use the `print_ops_info` function better. Currently, it's not used in the calls most people would want to do on Bento for example. For one-liner compilation (e.g. calling `export_to_executorch`), the information should be printed. This diff refactors both the APIs in `__init__.py` and `utils.py`, so that the breakdown makes more sense. Arguably it should be a stack of diffs, but it mostly all goes hand in hand IMO. Main changes: - create an `export_edge_to_executorch` API, which takes in an `EdgeProgramManager`. This is useful because we want to keep the edge graph module around to pass it in `print_ops_count` - calls `print_ops_info` in `export_to_executorch` - call `export_to_executorch` in `run_and_verify`, using the exported module - introduce a `model_is_quantized()` util to call the right API when trying to make models eval. The check on the `GraphModule` type is not robust enough, since other models could be `GraphModule`s but not be quantized. If that's the case, we assert that they have been exported already, which makes the `eval()` requirement moot. Differential Revision: D58101124
mcremon-meta
force-pushed
the
export-D58101124
branch
from
08125db
to
c4a09b0
Compare
…g flow, and call print_ops_info in export_to_executorch (#3863) Summary: The current setup is unbalanced in the way we call different APIs, especially because we would love to (and should) use the `print_ops_info` function better. Currently, it's not used in the calls most people would want to do on Bento for example. For one-liner compilation (e.g. calling `export_to_executorch`), the information should be printed. `export_to_executorch` should also be called in the testing flow. This diff refactors both the APIs in `__init__.py` and `utils.py`, so that the breakdown makes more sense. Arguably it should be a stack of diffs, but it mostly all goes hand in hand IMO so I kept it as one. Main changes: - create an `export_edge_to_executorch` API, which takes in an `EdgeProgramManager`. This is useful because we want to keep the edge graph module around to pass it in `print_ops_count`, and now we can use it in `export_to_executorch` (see next point) - calls `print_ops_info` in `export_to_executorch`, now that the edge graph is exposed there - call `export_to_executorch` in `run_and_verify`, using the exported module. This required changing the checks for `eval()` mode, see next point. - introduce a `model_is_quantized()` util to call the right API when trying to make models eval. The check on the `GraphModule` type is not robust enough, since other models could be `GraphModule`s but not be quantized. If that's the case, we assert that they have been exported already, which makes the `eval()` requirement moot. Reviewed By: dulinriley, zonglinpengmeta Differential Revision: D58101124
facebook-github-bot
force-pushed
the
export-D58101124
branch
from
c4a09b0
to
fe62256
Compare
|
This pull request was exported from Phabricator. Differential Revision: D58101124 |
dulinriley
approved these changes
Jun 7, 2024
|
This pull request has been merged in d928066. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
introduce an
is_quantized()util to call the right API when trying to make models eval. The check on theGraphModuletype is not robust enough, since other models could beGraphModules but not be quantized.Differential Revision: D58101124