Skip to content

Don't allow requires_grad to be set on integer Tensor constructors in… #7185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
May 18, 2018
Merged

Don't allow requires_grad to be set on integer Tensor constructors in… #7185

merged 7 commits into from
May 18, 2018

Conversation

gchanan
Copy link
Contributor

@gchanan gchanan commented May 2, 2018

… tensor_new.

@gchanan gchanan requested a review from colesbury May 2, 2018 18:11
@onnxbot-worker-1 onnxbot-worker-1 mentioned this pull request May 2, 2018
Closed
@gchanan
Copy link
Contributor Author

gchanan commented May 2, 2018

This completes #7034; I would argue this should perhaps be a (python) warning rather than an error (see discussion in #7034), but let's make it consistent for now.

@gchanan
Copy link
Contributor Author

gchanan commented May 2, 2018

@pytorchbot retest this please.

goldsborough
goldsborough reviewed May 3, 2018
View reviewed changes
torch/csrc/autograd/utils/python_variables.h

namespace torch { namespace autograd { namespace utils {

inline at::Tensor set_requires_grad(at::Tensor self, bool requires_grad) {

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@gchanan
Copy link
Contributor Author

gchanan commented May 3, 2018

#7034 also didn't put this error Variable(..., requires_grad=True)

@ezyang
Copy link
Contributor

ezyang commented May 13, 2018 

6:09:33 ======================================================================
16:09:33 ERROR: test_cdf_log_prob (test_distributions.TestDistributions)
16:09:33 ----------------------------------------------------------------------
16:09:33 Traceback (most recent call last):
16:09:33   File "/var/lib/jenkins/workspace/test/test_distributions.py", line 1871, in test_cdf_log_prob
16:09:33     samples = torch.tensor(dist.sample().data, requires_grad=True)
16:09:33 RuntimeError: only Tensors of floating point dtype can require gradients

@gchanan gchanan merged commit 7abdc30 into pytorch:master May 18, 2018
onnxbot added a commit to onnxbot/onnx-fb-universe that referenced this pull request May 18, 2018
@onnxbot
[auto] Update pytorch to 7abdc30 - Don't allow requires_grad to be se…
981c26c 
…t on integer Tensor constructors in… (pytorch/pytorch#7185)

pytorch/pytorch@7abdc30
weiyangfb pushed a commit to weiyangfb/pytorch that referenced this pull request Jun 11, 2018
@gchanan @weiyangfb
Don't allow requires_grad to be set on integer Tensor constructors in… (
14e7482 
pytorch#7185)

* Don't allow requires_grad to be set on integer Tensor constructors in tensor_new.

* Fix autograd test.

* Fix test_distributions.

* Fix test_jit.

* Fix NN tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ezyang ezyang ezyang approved these changes

@colesbury colesbury Awaiting requested review from colesbury

@apaszke apaszke Awaiting requested review from apaszke

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

+1 more reviewer

@goldsborough goldsborough goldsborough left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gchanan @ezyang @apaszke @goldsborough