Remove strange helpers for python -> tensor conversions, and at -> et tensor conversions #121
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
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D48618092 |
|
This pull request was exported from Phabricator. Differential Revision: D48618092 |
JacobSzwejbka
force-pushed
the
export-D48618092
branch
from
93bf6ce to
fb8a1ca
Compare
Summary: Pull Request resolved: pytorch/executorch#120 .pyi file is needed for pyre to recognize the pybindings. Additionally you have to name the .pyi exactly the name of the pybinding lib which is problematic for us because we have 4 libs and 2 arent public. To make matters worse the types arg in the buck rule doesnt handle targets in the same manner as srcs. Put all this together and you get the weird genrule logic in the build files. A couple of pyre-ignores remain in the test due to the program/operator meta apis that arent in the .pyi because Im going to move them out of pybindings extension Differential Revision: https://internalfb.com/D48634638 fbshipit-source-id: 3b3bb3d624939bfb2b2738a7a5f91c277fc6e0f4
… tensor conversions (#121) Summary: Pull Request resolved: pytorch/executorch#121 Keepalive was difficult to manage and Im not even sure how sound it was for things like strides and dim order. Now the run_method loop is a little grosser, but I think the intent is much clearer. We cast from python types to at types, and then possible have to do a hard conversion from at::tensor to ETensor which can be pretty gross since ETensor doesnt manage any memory. This highlights the need for like an ETensor wrapper that manages a bunch of state like sizes and TensorImpl Reviewed By: dbort Differential Revision: D48618092 fbshipit-source-id: cea1feae3f801be1c6857d148a48335f9408dca0
|
This pull request was exported from Phabricator. Differential Revision: D48618092 |
JacobSzwejbka
force-pushed
the
export-D48618092
branch
from
fb8a1ca to
da24bf3
Compare
|
This pull request has been merged in 1c92891. |
Gasoonjia
pushed a commit
that referenced
this pull request
Jul 30, 2024
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: Keepalive was difficult to manage and Im not even sure how sound it was for things like strides and dim order. Now the run_method loop is a little grosser, but I think the intent is much clearer. We cast from python types to at types, and then possible have to do a hard conversion from at::tensor to ETensor which can be pretty gross since ETensor doesnt manage any memory. This highlights the need for like an ETensor wrapper that manages a bunch of state like sizes and TensorImpl
Differential Revision: D48618092