Skip to content

Conversation

@yf225
Copy link
Contributor

@yf225 yf225 commented Apr 11, 2019

Currently, a TensorImpl's is_variable_ is true if and only if the TensorImpl has AutogradMeta. This PR unifies these two concepts by removing is_variable_ and change is_variable() to check existence of AutogradMeta instead.

Removing is_variable_ is part of the work in Variable/Tensor merge. After the merge, all at::Tensors are variables, and Tensor.is_variable() will always return true.

@yf225 yf225 force-pushed the remove_is_variable_field branch from c50b358 to d23bffd Compare April 11, 2019 02:40
@yf225 yf225 mentioned this pull request Apr 11, 2019
22 tasks
@yf225 yf225 requested review from ezyang and gchanan April 11, 2019 15:35
@yf225 yf225 changed the title [WIP] Change is_variable() to check existence of AutogradMeta, and remove is_variable_ Change is_variable() to check existence of AutogradMeta, and remove is_variable_ Apr 11, 2019
@gchanan
Copy link
Contributor

gchanan commented Apr 11, 2019

What's the long term plan for is_variable?

Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

change looks fine; updating the description to explain what the long-term plan here would be nice.

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.

@facebook-github-bot
Copy link
Contributor

@yf225 merged this pull request in c7b5a8a.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Apr 11, 2019
…s_variable_ (#19139)

Summary:
Currently, a TensorImpl's `is_variable_` is true if and only if the TensorImpl has AutogradMeta. This PR unifies these two concepts by removing `is_variable_` and change `is_variable()` to check existence of AutogradMeta instead.

Removing `is_variable_` is part of the work in Variable/Tensor merge.
Pull Request resolved: pytorch/pytorch#19139

Differential Revision: D14893339

Pulled By: yf225

fbshipit-source-id: ceb5e22c3c01f79b5d21d5bdbf4a7d1bc397796a
zhangguanheng66 pushed a commit to zhangguanheng66/pytorch that referenced this pull request May 6, 2019
…s_variable_ (pytorch#19139)

Summary:
Currently, a TensorImpl's `is_variable_` is true if and only if the TensorImpl has AutogradMeta. This PR unifies these two concepts by removing `is_variable_` and change `is_variable()` to check existence of AutogradMeta instead.

Removing `is_variable_` is part of the work in Variable/Tensor merge.
Pull Request resolved: pytorch#19139

Differential Revision: D14893339

Pulled By: yf225

fbshipit-source-id: ceb5e22c3c01f79b5d21d5bdbf4a7d1bc397796a
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.

3 participants