Warns when performing integer division with div and addcdiv by mruberry · Pull Request #34570 · pytorch/pytorch

mruberry · 2020-03-11T01:11:50Z

UPDATE

BC-breaking release note info:

In a future PyTorch release, torch.div (including the / operator) will perform "true" division as in Python3 and NumPy. Performing integer floor division with torch.div is deprecated. To floor divide integers use torch.floor_divide, instead.

// ------- (PR info below) ------- //

Per title.

In the future we want to make div(), the division operator, and addcdiv perform true division as in Python 3, NumPy, and JAX. To do this without silently breaking users we plan to:

Warn (once) in 1.5 when a user performs integer division using div or addcdiv
RuntimeError in 1.6 when a user attempts to perform integer division using div or addcdiv
Always perform true division in 1.7 using div, /, and addcdiv

Users can use true_divide or floor_divide today to explicitly specify the type of division they like.

A test for this behavior is added to test_type_promotion. Unfortunately, because we are only warning once (to avoid a deluge) the test only uses maybeWarns Regex.

The XLA failure is real but will be solved by #34552. I'll be sure to land that PR first to avoid temporarily breaking the XLA build.

dr-ci · 2020-03-11T03:56:14Z

💊 CircleCI build failures summary and remediations

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

✅ None of the build failures appear to be your fault 💚

4/4 broken upstream at merge base eef17ed on Mar 19 from 12:27am to 2:53pm (16 commits; 2a1c838 - 7c06b86)
Please rebase on the viable/strict branch (expand for instructions)

Since your merge base is older than viable/strict, run these commands:
```
git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD
```
Check out the recency history of this "viable master" tracking branch.

🚧 4 upstream failures:

These were probably caused by upstream breakages:

pytorch_windows_vs2019_py36_cuda10.1_build on Mar 19 from 12:27am to 2:53pm (16 commits; 2a1c838 - 7c06b86)
pytorch_linux_xenial_py3_6_gcc5_4_test on Mar 19 from 12:06am to 12:55pm (17 commits; 4521477 - 9c4683e)
pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test on Mar 19 from 12:06am to 4:56am (12 commits; 4521477 - be82e55)
pytorch_linux_xenial_py3_clang5_asan_test on Mar 19 from 12:06am to 4:56am (12 commits; 4521477 - be82e55)

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 on the GitHub issue tracker.

This comment has been revised 37 times.

gchanan · 2020-03-12T00:55:25Z

+  if (isIntegralType(result.scalar_type(), /*includeBool=*/ true)) {
+    TORCH_WARN_ONCE(
+      "Integer division of tensors using div or / is deprecated. ",
+      "In a future release div and / will ",


nit: I'd just specify the expected versions, I don't see why we'd deviate from the plan and even if we do, it's not the biggest deal.

Also, did we talk about having a future true_division for PyTorch? (So the error message could say "or import...").

I'll put up a separate PR shortly for that behavior.

gchanan · 2020-03-12T01:04:40Z

+        with self.maybeWarnsRegex(UserWarning, '^Integer division.+is deprecated.+'):
+            c = a / b
+            c = torch.div(a, b)
+            torch.div(a, b, out=o)


nit: add out= tests.

I'll add some more.

Hm... I'm not sure how much more it makes sense to add here, actually. Trying to perform division with float inputs and an integral out type will already throw a runtime error.

…_integer_div

gchanan · 2020-03-18T22:40:45Z

+    TORCH_WARN_ONCE(
+      "Integer division of tensors using div or / is deprecated, ",
+      "and in a future release div will perform true division as in Python 3. ",
+      "Use true_divide or floor_divide (// in Python) instead.");


should we say something about "import future"?

It would leave master in a bad state. I'll rebase #34794 once this is in and update the doc.

facebook-github-bot

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

…_integer_div

facebook-github-bot

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

facebook-github-bot · 2020-03-19T12:15:52Z

@mruberry merged this pull request in 0d8447a.

Summary: Per title. See related #34570. In PyTorch 1.7 the plan is for torch.div and Python's division operator to perform "true" division, like Python 3, JAX, and NumPy. To facilitate this change, this PR expands true_divide to be a method so it can cover all of torch.div's use cases. New true_divide tests are added to test_torch.py, test_type_promotion.py, and test_sparse.py. Pull Request resolved: #34794 Differential Revision: D20545507 Pulled By: mruberry fbshipit-source-id: 55286f819716c8823d1930441a69008560ac2bd5

…34570) Summary: Per title. In the future we want to make div(), the division operator, and addcdiv perform true division as in Python 3, NumPy, and JAX. To do this without silently breaking users we plan to: - Warn (once) in 1.5 when a user performs integer division using div or addcdiv - RuntimeError in 1.6 when a user attempts to perform integer division using div or addcdiv - Always perform true division in 1.7 using div, /, and addcdiv Users can use true_divide or floor_divide today to explicitly specify the type of division they like. A test for this behavior is added to test_type_promotion. Unfortunately, because we are only warning once (to avoid a deluge) the test only uses maybeWarns Regex. The XLA failure is real but will be solved by pytorch#34552. I'll be sure to land that PR first to avoid temporarily breaking the XLA build. Pull Request resolved: pytorch#34570 Differential Revision: D20529211 Pulled By: mruberry fbshipit-source-id: 65af5a9641c5825175d029e8413c9e1730c661d0

Summary: Per title. See related pytorch#34570. In PyTorch 1.7 the plan is for torch.div and Python's division operator to perform "true" division, like Python 3, JAX, and NumPy. To facilitate this change, this PR expands true_divide to be a method so it can cover all of torch.div's use cases. New true_divide tests are added to test_torch.py, test_type_promotion.py, and test_sparse.py. Pull Request resolved: pytorch#34794 Differential Revision: D20545507 Pulled By: mruberry fbshipit-source-id: 55286f819716c8823d1930441a69008560ac2bd5

Mike Ruberry added 2 commits March 10, 2020 18:03

Deprecates integer division using div and addcdiv

46b8f72

Doc improvements

9a5e570

mruberry requested a review from gchanan March 11, 2020 05:09

gchanan added the module: deprecation label Mar 12, 2020

gchanan reviewed Mar 12, 2020

View reviewed changes

Comment thread aten/src/ATen/native/BinaryOps.cpp Outdated

gchanan reviewed Mar 12, 2020

View reviewed changes

Comment thread aten/src/ATen/native/PointwiseOps.cpp Outdated

gchanan reviewed Mar 12, 2020

View reviewed changes

Comment thread torch/_torch_docs.py Outdated

gchanan reviewed Mar 12, 2020

View reviewed changes

Comment thread torch/_torch_docs.py Outdated

This was referenced Mar 12, 2020

Makes floor_divide a method, adds sparse floor division #34552

Closed

Extends true_divide to be a method #34794

Closed

Updates per feedback

8c0bb6d

gchanan reviewed Mar 17, 2020

View reviewed changes

Comment thread aten/src/ATen/native/PointwiseOps.cpp Outdated

gchanan reviewed Mar 17, 2020

View reviewed changes

Comment thread test/test_type_promotion.py

Updates per feedback, XLA tests

a3e546e

mruberry requested a review from gchanan March 18, 2020 20:56

Merge branch 'master' of https://github.com/pytorch/pytorch into warn…

2330e90

…_integer_div

gchanan reviewed Mar 18, 2020

View reviewed changes

gchanan approved these changes Mar 18, 2020

View reviewed changes

facebook-github-bot reviewed Mar 19, 2020

View reviewed changes

Merge branch 'master' of https://github.com/pytorch/pytorch into warn…

23d9a8b

…_integer_div

facebook-github-bot reviewed Mar 19, 2020

View reviewed changes

facebook-github-bot closed this in 0d8447a Mar 19, 2020

facebook-github-bot added the merged label Mar 19, 2020

mruberry deleted the warn_integer_div branch May 16, 2020 05:00

mruberry added the Merged label Oct 28, 2020

Conversation

mruberry commented Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci Bot commented Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CircleCI build failures summary and remediations

🚧 4 upstream failures:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mruberry commented Mar 11, 2020 •

edited

Loading

dr-ci Bot commented Mar 11, 2020 •

edited

Loading