Skip to content

Replace rounding_mode="true" with rounding_mode=None#51988

Closed
peterbell10 wants to merge 13 commits intogh/peterbell10/50/basefrom
gh/peterbell10/50/head
Closed

Replace rounding_mode="true" with rounding_mode=None#51988
peterbell10 wants to merge 13 commits intogh/peterbell10/50/basefrom
gh/peterbell10/50/head

Conversation

@peterbell10
Copy link
Copy Markdown
Collaborator

@peterbell10 peterbell10 commented Feb 9, 2021 •
edited
Loading

Stack from ghstack:

Differential Revision: D27561817

BC-breaking note

torch.divide with rounding_mode='true' is replaced with rounding_mode=None

The torch.divide's undocumented rounding_mode='true' option has been removed, and instead rounding_mode=None can be passed to indicate no rounding should take place. This is equivalent to omitting the argument entirely.

Example:

Version 1.8.1:

>>> a, b = torch.full((2,), 4.2), torch.full((2,), 2)
>>> torch.divide(a, b, rounding_mode='true')
tensor([2.1000, 2.1000])

Version 1.9.0:

>>> a, b = torch.full((2,), 4.2), torch.full((2,), 2)
>>> torch.divide(a, b, rounding_mode=None)
tensor([2.1000, 2.1000])

This was referenced Feb 9, 2021
Closed
Closed
Merged
Closed
Closed
@peterbell10 peterbell10 changed the title Replace rounding_mode="true" with rounding_mode="None" Replace rounding_mode="true" with rounding_mode=None Feb 9, 2021
@peterbell10 peterbell10 requested a review from mruberry February 9, 2021 19:33
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Feb 9, 2021 •
edited
Loading

💊 CI failures summary and remediations

As of commit f8a5a4a (more details on the Dr. CI page):

  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_xla_linux_bionic_py3_6_clang9_build (1/1)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

Apr 02 15:56:12 Failed to generate ATEN bindings: ['/var/lib/jenkins/workspace/xla/scripts/generate_code.sh'] 
Apr 02 15:56:12 AtenXlaType function missed override: Tensor& div_(Tensor& self, const Tensor& other, std::string rounding_mode); // div_(Tensor,Tensor,std::string)->Tensor
Apr 02 15:56:12 Traceback (most recent call last):
Apr 02 15:56:12   File "/var/lib/jenkins/workspace/xla/scripts/gen.py", line 1195, in <module>
Apr 02 15:56:12     generate(args)
Apr 02 15:56:12   File "/var/lib/jenkins/workspace/xla/scripts/gen.py", line 1165, in generate
Apr 02 15:56:12     assert check_overrides(overrides, overridden)
Apr 02 15:56:12 AssertionError
Apr 02 15:56:12 Building torch_xla version: 1.9
Apr 02 15:56:12 XLA Commit ID: 371bdf462a6b7576e294ae39e5ba23d8509d7834
Apr 02 15:56:12 PyTorch Commit ID: 9d19b6d0c9b1f7edf0908c5cc7e2743f543c5f54
Apr 02 15:56:12 Failed to generate ATEN bindings: ['/var/lib/jenkins/workspace/xla/scripts/generate_code.sh']
Apr 02 15:56:12 =================== sccache compilation log ===================
Apr 02 15:56:12 + cleanup
Apr 02 15:56:12 + retcode=1
Apr 02 15:56:12 + set +x
Apr 02 15:56:12 =========== If your build fails, please take a look at the log above for possible reasons ===========
Apr 02 15:56:12 Compile requests                    4894
Apr 02 15:56:12 Compile requests executed           4570
Apr 02 15:56:12 Cache hits                          3425
Apr 02 15:56:12 Cache hits (C/C++)                  3425
Apr 02 15:56:12 Cache misses                        1126

XLA failure

Job pytorch_xla_linux_bionic_py3_6_clang9_build is failing. Please create an issue with title prefixed by [PT_BREAK] in pytorch/xla and link to to this PR. If you have questions, please reach out to @ailzhang / @dlibenzi / @JackCaoG.

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

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

peterbell10 added a commit to peterbell10/pytorch that referenced this pull request Feb 9, 2021
@peterbell10
Replace rounding_mode="true" with rounding_mode="None"
34955d8 
ghstack-source-id: b8c37fe
Pull Request resolved: pytorch#51988
mruberry
mruberry approved these changes Feb 10, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for the quick fix, @peterbell10!

@peterbell10 peterbell10 mentioned this pull request Feb 11, 2021
Closed
@peterbell10
Update on "Replace rounding_mode="true" with rounding_mode=None"
520aaba 
Differential Revision: [D26375070](https://our.internmc.facebook.com/intern/diff/D26375070)

[ghstack-poisoned]
@peterbell10
Update on "Replace rounding_mode="true" with rounding_mode=None"
fba146e 
Differential Revision: [D26375070](https://our.internmc.facebook.com/intern/diff/D26375070)

[ghstack-poisoned]
@peterbell10
Update on "Replace rounding_mode="true" with rounding_mode=None"
ecaae29 
Differential Revision: [D26375070](https://our.internmc.facebook.com/intern/diff/D26375070)

[ghstack-poisoned]
@peterbell10
Update on "Replace rounding_mode="true" with rounding_mode=None"
fd0db74 
Differential Revision: [D26375070](https://our.internmc.facebook.com/intern/diff/D26375070)

[ghstack-poisoned]
@peterbell10
Update on "Replace rounding_mode="true" with rounding_mode=None"
fb8e482 
Differential Revision: [D26375070](https://our.internmc.facebook.com/intern/diff/D26375070)

[ghstack-poisoned]
@peterbell10
Update on "Replace rounding_mode="true" with rounding_mode=None"
08f1931 
Differential Revision: [D26375070](https://our.internmc.facebook.com/intern/diff/D26375070)

[ghstack-poisoned]
This was referenced Apr 2, 2021
Closed
Merged
@peterbell10
Copy link
Copy Markdown
Collaborator Author

peterbell10 commented Apr 2, 2021

@mruberry PTAL. Rebased, test are passing, and sent PR updating XLA pytorch/xla#2859.

mruberry
mruberry reviewed Apr 5, 2021
View reviewed changes
Comment thread test/test_autograd.py
a = torch.tensor([1.], requires_grad=True)
out = torch.div(a, 2., rounding_mode="trunc")
self.assertEqual(out.grad_fn._saved_rounding_mode, "trunc") # std::string -> str
a = torch.ones(1, 1, requires_grad=True)
Copy link
Copy Markdown
Collaborator

@mruberry mruberry Apr 5, 2021

Choose a reason for hiding this comment

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

Why change this to be a linalg.qr test?

Copy link
Copy Markdown
Collaborator Author

@peterbell10 peterbell10 Apr 5, 2021

Choose a reason for hiding this comment

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

Because it doesn't have a str argument any more. See #55225 where I add it back.

mruberry
mruberry approved these changes Apr 5, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

I just have one small question. This is awesome, @peterbell10! Thank you for picking up the initiatives on following-through with these deprecations.

@JackCaoG let me know if/when you're ready to merge the XLA side of this, then we can coordinate a merge like we typically do.

@mruberry
Copy link
Copy Markdown
Collaborator

mruberry commented Apr 5, 2021

Imported to hit internal tests. There are already some uses of rounding_mode within FB! But I don't think any were using the undocumented "true" mode.

@JackCaoG
Copy link
Copy Markdown
Collaborator

JackCaoG commented Apr 5, 2021

@mruberry Peter submitted pytorch/xla#2859 and it is ready now. I will merge xla pr once tis one is merged.

@mruberry
Copy link
Copy Markdown
Collaborator

mruberry commented Apr 5, 2021

@JackCaoG Awesome! Beginning the land process now. I'll update here when it finishes.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Apr 5, 2021

@mruberry merged this pull request in 2ee02b3.

@peterbell10 peterbell10 mentioned this pull request Apr 6, 2021
Closed
@facebook-github-bot facebook-github-bot deleted the gh/peterbell10/50/head branch April 9, 2021 14:16
@anjali411 anjali411 added the module: bc-breaking Related to a BC-breaking change label Jun 2, 2021
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@peterbell10 @facebook-github-bot
Replace rounding_mode="true" with rounding_mode=None (pytorch#51988)
2d9ae4b 
Summary:
Pull Request resolved: pytorch#51988

* **pytorch#51988 Replace rounding_mode="true" with rounding_mode=None**

Test Plan: Imported from OSS

Reviewed By: ngimel

Differential Revision: D27561817

Pulled By: mruberry

fbshipit-source-id: 60d1d9c389570f60d599fc1876518717367fb368
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mruberry mruberry mruberry approved these changes

@albanD albanD Awaiting requested review from albanD

@soulitzer soulitzer Awaiting requested review from soulitzer

Assignees

No one assigned

Labels

cla signed Merged module: bc-breaking Related to a BC-breaking change open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@peterbell10 @facebook-github-bot @mruberry @JackCaoG @anjali411