Skip to content

Conversation

@yf225
Copy link
Contributor

@yf225 yf225 commented Mar 20, 2019

Currently, THPVariable_Wrap(…) and THPVariable_NewWithVar(…) depend on the existence of pyobj_ in the autograd metadata of a Variable to convert the Variable to a Python tensor. However, after the Variable/Tensor merge, there will be Variables that don't contain autograd metadata, and to allow the conversion from non-autograd-meta Variable to a Python tensor we need to store the pyobj_ outside of autograd metadata and in a place where it will always be available.

This PR makes it possible by moving pyobj_ into TensorImpl, so that THPVariable_Wrap(…) and THPVariable_NewWithVar(…) can always access a Variable's pyobj_ and convert the Variable to a Python tensor.

@yf225 yf225 requested review from ezyang and gchanan March 20, 2019 16:32
@yf225 yf225 mentioned this pull request Mar 20, 2019
22 tasks
@gchanan
Copy link
Contributor

gchanan commented Mar 20, 2019

can you improve the description? It says we may encounter situations -- when does this happen?

The link to the description doesn't elucidate the situation either:
Move pyobj_ to TensorImpl itself, because we always need to be able to convert to and from the Python representation.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yf225 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yf225 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants