fix set_data permitting requires_grad=True on integer tensor by yuguo68 · Pull Request #78436 · pytorch/pytorch

2022-06-01T01:32:33.1527843Z processing existing schema:  text(__torch__.torch.classes.profiling.SourceRef _0) -> str _0
2022-06-01T01:32:33.1529380Z processing existing schema:  count(__torch__.torch.classes.profiling.InstructionStats _0) -> int _0
2022-06-01T01:32:33.1531142Z processing existing schema:  duration_ns(__torch__.torch.classes.profiling.InstructionStats _0) -> int _0
2022-06-01T01:32:33.1533050Z processing existing schema:  source(__torch__.torch.classes.profiling.SourceStats _0) -> __torch__.torch.classes.profiling.SourceRef _0
2022-06-01T01:32:33.1535710Z processing existing schema:  line_map(__torch__.torch.classes.profiling.SourceStats _0) -> Dict(int, __torch__.torch.classes.profiling.InstructionStats) _0
2022-06-01T01:32:33.1536589Z processing existing schema:  __init__(__torch__.torch.classes.profiling._ScriptProfile _0) -> NoneType _0
2022-06-01T01:32:33.1538444Z processing existing schema:  enable(__torch__.torch.classes.profiling._ScriptProfile _0) -> NoneType _0
2022-06-01T01:32:33.1540026Z processing existing schema:  disable(__torch__.torch.classes.profiling._ScriptProfile _0) -> NoneType _0
2022-06-01T01:32:33.1542697Z processing existing schema:  _dump_stats(__torch__.torch.classes.profiling._ScriptProfile _0) -> __torch__.torch.classes.profiling.SourceStats[] _0
2022-06-01T01:32:33.1543874Z processing existing schema:  __init__(__torch__.torch.classes.dist_rpc.WorkerInfo _0, str _1, int _2) -> NoneType _0
2022-06-01T01:32:33.1546821Z The PR is introducing backward incompatible changes to the operator library. Please contact PyTorch team to confirm whether this change is wanted or not. 
2022-06-01T01:32:33.1546856Z 
2022-06-01T01:32:33.1546991Z Broken ops: [
2022-06-01T01:32:33.1547396Z 	aten::special_laguerre_polynomial_l(Tensor x, Tensor n) -> Tensor
2022-06-01T01:32:33.1547785Z 	aten::special_laguerre_polynomial_l.out(Tensor x, Tensor n, *, Tensor(a!) out) -> Tensor(a!)
2022-06-01T01:32:33.1548324Z 	aten::special_laguerre_polynomial_l.n_scalar_out(Tensor x, Scalar n, *, Tensor(a!) out) -> Tensor(a!)
2022-06-01T01:32:33.1548661Z 	aten::special_laguerre_polynomial_l.x_scalar(Scalar x, Tensor n) -> Tensor
2022-06-01T01:32:33.1549142Z 	aten::special_laguerre_polynomial_l.x_scalar_out(Scalar x, Tensor n, *, Tensor(a!) out) -> Tensor(a!)
2022-06-01T01:32:33.1549491Z 	aten::special_laguerre_polynomial_l.n_scalar(Tensor x, Scalar n) -> Tensor
2022-06-01T01:32:33.1549823Z 	aten::special_hermite_polynomial_he(Tensor x, Tensor n) -> Tensor
2022-06-01T01:32:33.1550212Z 	aten::special_hermite_polynomial_he.out(Tensor x, Tensor n, *, Tensor(a!) out) -> Tensor(a!)

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

…on integer tensor" fix #76008 [ghstack-poisoned]

ghstack-source-id: 3edfc5d Pull Request resolved: #78436

albanD

That makes sense to me as a fix even though it is BC-breaking.
cc @soulitzer do you agree?

albanD · 2022-05-31T19:57:37Z

            # Calls model with a LongTensor input but DoubleTensor weights
            input = torch.randn(1, 1, 1, 6, dtype=torch.double)
-            weight = torch.zeros(1, 1, 1, 3, dtype=torch.long)
+            weight = torch.zeros(1, 1, 1, 3, dtype=torch.complex64)


Why change this?

Since torch.long does not support grad, with this PR, this test will fail at this assignment. However, the original test fails at out = model(input) with a dtype mismatch error.

Ho right. That sounds good!

albanD

SGTM
I'll let @soulitzer take a look and merge if he's happy with this!

soulitzer

Thanks for the change! Just a small nit on the wording.

soulitzer · 2022-05-31T21:18:39Z

That makes sense to me as a fix even though it is BC-breaking.

Yup, SGTM as well

…True on integer tensor" fix #76008 [ghstack-poisoned]

…ad=True on integer tensor" fix #76008 [ghstack-poisoned]

ghstack-source-id: 1fc25ee Pull Request resolved: #78436

yuguo68 · 2022-06-01T03:54:11Z

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

soulitzer

Thanks!

soulitzer · 2022-06-01T14:33:09Z

@yuguo68 why did you import this, are there fb-only changes that need to be made?

yuguo68 · 2022-06-01T15:34:09Z

@yuguo68 why did you import this, are there fb-only changes that need to be made?

just want to check whether it causes internal test errors in advance, will use pytorchbot to merge.

yuguo68 · 2022-06-01T15:55:17Z

@pytorchbot merge this

github-actions · 2022-06-01T15:57:17Z

Hey @yuguo68.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: Pull Request resolved: #78436 Approved by: https://github.com/albanD, https://github.com/soulitzer Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/efdb4192bc9620a9ef93fe2fc18969af42a2ebfb Reviewed By: seemethere Differential Revision: D36800179 Pulled By: yuguo68 fbshipit-source-id: e8a92d3e9412a462cff1cebf8e74f1e7213cdac6

Fixes #79871 Make `module.cpp` tests respect change that was made in #78436 (no int types in autograd). Note that there still a gap in Cmake test -- it's unclear why it didn't fail CI before. As far as I can tell it should be executed, because it's included here https://github.com/pytorch/pytorch/blob/79507d2a9d06d4a3fb50eb21b30e08cc044776ce/test/cpp/api/CMakeLists.txt#L17:L17 Pull Request resolved: #79926 Approved by: https://github.com/soulitzer

Summary: Fixes #79871 Make `module.cpp` tests respect change that was made in #78436 (no int types in autograd). Note that there still a gap in Cmake test -- it's unclear why it didn't fail CI before. As far as I can tell it should be executed, because it's included here https://github.com/pytorch/pytorch/blob/79507d2a9d06d4a3fb50eb21b30e08cc044776ce/test/cpp/api/CMakeLists.txt#L17:L17 Pull Request resolved: #79926 Approved by: https://github.com/soulitzer Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/a8b098859688a3f1993821eecc036be973a15605 Reviewed By: atalman Differential Revision: D37333256 Pulled By: atalman fbshipit-source-id: 0b914fbcc5da72fbadf0f3c2f6b6af1a198cf6fb

Fixes pytorch#79871 Make `module.cpp` tests respect change that was made in pytorch#78436 (no int types in autograd). Note that there still a gap in Cmake test -- it's unclear why it didn't fail CI before. As far as I can tell it should be executed, because it's included here https://github.com/pytorch/pytorch/blob/79507d2a9d06d4a3fb50eb21b30e08cc044776ce/test/cpp/api/CMakeLists.txt#L17:L17 Pull Request resolved: pytorch#79926 Approved by: https://github.com/soulitzer

Pull Request resolved: pytorch#78436 Approved by: https://github.com/albanD, https://github.com/soulitzer

Fixes pytorch#79871 Make `module.cpp` tests respect change that was made in pytorch#78436 (no int types in autograd). Note that there still a gap in Cmake test -- it's unclear why it didn't fail CI before. As far as I can tell it should be executed, because it's included here https://github.com/pytorch/pytorch/blob/e4a7ecafa61d9d9a7ac1f154264d6f5ccffe12bb/test/cpp/api/CMakeLists.txt#L17:L17 Pull Request resolved: pytorch#79926 Approved by: https://github.com/soulitzer

set data permits requires_grad=True on integer tensor

b64b4fc

[ghstack-poisoned]

yuguo68 requested review from albanD and soulitzer as code owners May 27, 2022 22:03

facebook-github-bot added the cla signed label May 27, 2022

yuguo68 changed the title ~~set data permits requires_grad=True on integer tensor~~ fix set_data permitting requires_grad=True on integer tensor May 27, 2022

fix test_dot_data_use on "fix set_data permitting requires_grad=True …

e169747

…on integer tensor" fix #76008 [ghstack-poisoned]

yuguo68 added a commit that referenced this pull request May 27, 2022

set data permits requires_grad=True on integer tensor

7aac6f3

ghstack-source-id: 3edfc5d Pull Request resolved: #78436

albanD reviewed May 31, 2022

View reviewed changes

albanD added the module: bc-breaking Related to a BC-breaking change label May 31, 2022

albanD approved these changes May 31, 2022

View reviewed changes

soulitzer reviewed May 31, 2022

View reviewed changes

Comment thread torch/csrc/autograd/python_variable.cpp Outdated

move check to variable.cpp on "fix set_data permitting requires_grad=…

c1fc574

…True on integer tensor" fix #76008 [ghstack-poisoned]

fix model.to(torch::Int) test on "fix set_data permitting requires_gr…

4c022d6

…ad=True on integer tensor" fix #76008 [ghstack-poisoned]

yuguo68 added a commit that referenced this pull request Jun 1, 2022

set data permits requires_grad=True on integer tensor

c8ed5ab

ghstack-source-id: 1fc25ee Pull Request resolved: #78436

soulitzer approved these changes Jun 1, 2022

View reviewed changes

pytorchmergebot added the Merged label Jun 1, 2022

pytorchmergebot closed this in efdb419 Jun 1, 2022

facebook-github-bot deleted the gh/yuguo68/6/head branch June 5, 2022 14:17

vors mentioned this pull request Jun 20, 2022

[bazel] cpp fronted nn.linear->to(int) throws exception, but Cmake tests pass #79871

Closed

vors mentioned this pull request Jun 21, 2022

Fix //:module_test Conversion_MultiCUDA #79926

Closed

soulitzer added release notes: autograd release notes category topic: bc breaking topic category labels Jun 21, 2022

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 25, 2026

set data permits requires_grad=True on integer tensor

6f37472

Pull Request resolved: pytorch#78436 Approved by: https://github.com/albanD, https://github.com/soulitzer

Conversation

yuguo68 commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

❌ 1 New Failures

🕵️ 1 new failure recognized by patterns

pull / linux-focal-py3.7-gcc7 / test (backwards_compat, 1, 1, linux.2xlarge) (1/1)

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

albanD May 31, 2022

Choose a reason for hiding this comment

Uh oh!

yuguo68 May 31, 2022

Choose a reason for hiding this comment

Uh oh!

albanD May 31, 2022

Choose a reason for hiding this comment

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

soulitzer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

soulitzer commented May 31, 2022

Uh oh!

yuguo68 commented Jun 1, 2022

Uh oh!

soulitzer left a comment

Choose a reason for hiding this comment

Uh oh!

soulitzer commented Jun 1, 2022

Uh oh!

yuguo68 commented Jun 1, 2022

Uh oh!

yuguo68 commented Jun 1, 2022

Uh oh!

github-actions Bot commented Jun 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yuguo68 commented May 27, 2022 •

edited

Loading

facebook-github-bot commented May 27, 2022 •

edited

Loading