Store user model to simplify ONNXProgram.{adapt_torch_*,__call__} APIs #115281
Conversation
This file contains hidden or 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
Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
thiagocrepaldi
requested review from
BowenBao,
abock and
wschin
as code owners
pytorch-bot
bot
added
the
release notes: onnx
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/115281
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 3e8612b with merge base 441ecf0 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
…call__} APIs" Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
…call__} APIs" Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
BowenBao
reviewed
Dec 6, 2023
| self, | ||
| *model_args, | ||
| model: Optional[ | ||
| model_with_state_dict: Optional[ |
There was a problem hiding this comment.
nit: for my understanding, do we need only state_dict or the model?
There was a problem hiding this comment.
we need the model, because the lifted "constant tensors" are not part of the ExportedProgram.state_dict (yet ?!?!?!)
I will investigate further and discuss with Meta and check whether I can make that change on ExportedProgram.state_dict , simplifying the requirement to state_dict only (which would be more ideal)
BowenBao
reviewed
Dec 6, 2023
added 4 commits
December 7, 2023 03:15
…call__} APIs" Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
…call__} APIs" Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
…call__} APIs" Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
…call__} APIs" Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
added 2 commits
December 7, 2023 16:42
…call__} APIs" Currently, the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This is needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted (and used as input to the ONNX model). That approach brings an unnecessary burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict`` This PR adds ``ONNXProgram._model_torch`` to store the user model and promotes the ``model`` argument to the aforementioned APIs to optional. As a result, for the fakefied model scenario, the user can still pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict [ghstack-poisoned]
…call__} APIs" Currently (after #114407), the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This was needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted from a real model (and used as input to the ONNX model). That approach brings an unnecessary usability burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict``. This PR adds ``ONNXProgram._model_torch`` attribute to store the user model and demote ``model`` argument of the aforementioned APIs to optional, only (as opposed to required). As a result, for the fakefied model scenario, the user still need to pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict, making it easier to use. [ghstack-poisoned]
BowenBao
reviewed
Dec 8, 2023
| _fake_context: Final[Optional[ONNXFakeContext]] | ||
| _export_exception: Final[Optional[Exception]] | ||
| _model_signature: Final[Optional[torch.export.ExportGraphSignature]] | ||
| _model_torch: Final[ |
There was a problem hiding this comment.
Tracked by #115461
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
This was referenced Dec 11, 2023
Merged
dmenig
pushed a commit
to dmenig/pytorch
that referenced
this pull request
Dec 21, 2023
pytorch#115281) Currently (after pytorch#114407), the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This was needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted from a real model (and used as input to the ONNX model). That approach brings an unnecessary usability burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict``. This PR adds ``ONNXProgram._model_torch`` attribute to store the user model and demote ``model`` argument of the aforementioned APIs to optional, only (as opposed to required). As a result, for the fakefied model scenario, the user still need to pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict, making it easier to use. Pull Request resolved: pytorch#115281 Approved by: https://github.com/BowenBao ghstack dependencies: pytorch#114407
dmenig
pushed a commit
to dmenig/pytorch
that referenced
this pull request
Dec 21, 2023
…orch#114762) Fixed by pytorch#113982 Pull Request resolved: pytorch#114762 Approved by: https://github.com/BowenBao ghstack dependencies: pytorch#114407, pytorch#115281
dmenig
pushed a commit
to dmenig/pytorch
that referenced
this pull request
Dec 21, 2023
…ytorch#115353) Pull Request resolved: pytorch#115353 Approved by: https://github.com/BowenBao ghstack dependencies: pytorch#114407, pytorch#115281, pytorch#114762
thiagocrepaldi
pushed a commit
to thiagocrepaldi/pytorch
that referenced
this pull request
Jan 6, 2024
pytorch#115281) Currently (after pytorch#114407), the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This was needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted from a real model (and used as input to the ONNX model). That approach brings an unnecessary usability burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict``. This PR adds ``ONNXProgram._model_torch`` attribute to store the user model and demote ``model`` argument of the aforementioned APIs to optional, only (as opposed to required). As a result, for the fakefied model scenario, the user still need to pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict, making it easier to use. Pull Request resolved: pytorch#115281 Approved by: https://github.com/BowenBao ghstack dependencies: pytorch#114407
huydhn
pushed a commit
that referenced
this pull request
Jan 8, 2024
#115281) (#115583) Currently (after #114407), the user has must pass the original user ``model`` to APIs such as ``ONNXProgram.__call__``, ``ONNXProgram.adapt_torch_inputs_to_onnx`` and ``ONNXProgram.adapt_torch_outputs_to_onnx`` APIs. This was needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted from a real model (and used as input to the ONNX model). That approach brings an unnecessary usability burden to the user when the model is not fakefied, because the model that was already passed to ``torch.onnx.dynamo_export`` could be used to extract ``state_dict``. This PR adds ``ONNXProgram._model_torch`` attribute to store the user model and demote ``model`` argument of the aforementioned APIs to optional, only (as opposed to required). As a result, for the fakefied model scenario, the user still need to pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict, making it easier to use. Pull Request resolved: #115281 Approved by: https://github.com/BowenBao ghstack dependencies: #114407
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Stack from ghstack (oldest at bottom):
Currently (after #114407), the user has must pass the original user
modelto APIs such asONNXProgram.__call__,ONNXProgram.adapt_torch_inputs_to_onnxandONNXProgram.adapt_torch_outputs_to_onnxAPIs.This was needed because when the model is fakefied, a version of the non-fakefied model is needed so that the Initializers, buffers and constants can be extracted from a real model (and used as input to the ONNX model).
That approach brings an unnecessary usability burden to the user when the model is not fakefied, because the model that was already passed to
torch.onnx.dynamo_exportcould be used to extractstate_dict.This PR adds
ONNXProgram._model_torchattribute to store the user model and demotemodelargument of the aforementioned APIs to optional, only (as opposed to required).As a result, for the fakefied model scenario, the user still need to pass the required model, but for non fakefied models, the persisted model is implicitly used to extract the model state_dict, making it easier to use.