Add division overload with rounding_mode selection#50280
Closed
peterbell10 wants to merge 26 commits intogh/peterbell10/36/basefrom
Closed
Add division overload with rounding_mode selection#50280peterbell10 wants to merge 26 commits intogh/peterbell10/36/basefrom
peterbell10 wants to merge 26 commits intogh/peterbell10/36/basefrom
Conversation
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. [ghstack-poisoned]
This was referenced Jan 8, 2021
Contributor
💊 CI failures summary and remediationsAs of commit 71b0cfe (more details on the Dr. CI page):
🕵️ 3 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. [ghstack-poisoned]
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. [ghstack-poisoned]
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. [ghstack-poisoned]
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. [ghstack-poisoned]
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. [ghstack-poisoned]
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. [ghstack-poisoned]
mruberry
reviewed
Jan 11, 2021
| BINARY_POINTWISE(mul); | ||
| BINARY_POINTWISE(div); | ||
| { | ||
| using Binop = Tensor (*)(const Tensor&, const Tensor&, std::string); |
Collaborator
Author
Collaborator
There was a problem hiding this comment.
It was, thanks @peterbell10. Darn autocomplete!
cc @zou3519
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
| } else if (isIntegralType(dtype, /*includeBool*/ false)) { | ||
| // There's no SIMD integer division, so don't try to vectorize it. | ||
| // TODO: if the divisor is a scalar, rewrite as multiplication by a constant. | ||
| AT_DISPATCH_INTEGRAL_TYPES(iter.common_dtype(), "div_floor_cpu", [&]() { |
Collaborator
There was a problem hiding this comment.
This is inconsistent between using dtype and iter.common_dtype().
Collaborator
Author
There was a problem hiding this comment.
I have removed all uses of iter.dtype(). If instead you meant the variable dtype, then I would note that it's assigned from iter.common_dtype() above. Just a bit less to type.
Collaborator
There was a problem hiding this comment.
I realize the value is the same, just for readability the code might want to stick to either dtype or iter.common_dtype(). No big deal either way.
mruberry
reviewed
Jan 11, 2021
| }); | ||
| }); | ||
| } else { | ||
| AT_DISPATCH_FLOATING_TYPES_AND2(kBFloat16, kHalf, iter.common_dtype(), "div_floor_cpu", [&]() { |
Collaborator
There was a problem hiding this comment.
Same dtype vs iter.common_dtype here, too.
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
mruberry
reviewed
Jan 11, 2021
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. Differential Revision: [D26123271](https://our.internmc.facebook.com/intern/diff/D26123271) [ghstack-poisoned]
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. Differential Revision: [D26123271](https://our.internmc.facebook.com/intern/diff/D26123271) [ghstack-poisoned]
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. Differential Revision: [D26123271](https://our.internmc.facebook.com/intern/diff/D26123271) [ghstack-poisoned]
Collaborator
|
Hi @mruberry, I think I can get pt/xla pr ready this week. I will ping you when that is ready. |
Collaborator
Author
|
@mruberry PTAL when you can. Have addressed your comments and rebased. |
mruberry
reviewed
Feb 1, 2021
| * ``"true"`` - default behavior. Performs no rounding and, if both :attr:`input` and | ||
| :attr:`other` are integer types, promotes the inputs to the default scalar type. | ||
| Equivalent to true division in Python (the ``/`` operator) and NumPy's ``np.true_divide``. | ||
| * ``"trunc"`` - rounds the results of the division down. |
Collaborator
There was a problem hiding this comment.
The description for trunc and floor are identical, "rounds the results of the division down".
For trunc I think we can say rounds towards zero?
mruberry
reviewed
Feb 1, 2021
| torch.set_default_dtype(dtype) | ||
| yield | ||
| torch.set_default_dtype(saved_dtype) | ||
| try: |
Collaborator
There was a problem hiding this comment.
Thank you for fixing this.
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. Differential Revision: [D26123271](https://our.internmc.facebook.com/intern/diff/D26123271) [ghstack-poisoned]
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Feb 1, 2021
As mentioned in pytorchgh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. ghstack-source-id: 372c4be Pull Request resolved: pytorch#50280
Collaborator
|
One of the test failures is real: test_div_rounding_numpy_cuda_bfloat16. We can skip the test for simplicity to unblock this landing. @JackCaoG's PR is ready to go so I'd like to land this during PST business hours on Tuesday, February 2nd. |
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. Differential Revision: [D26123271](https://our.internmc.facebook.com/intern/diff/D26123271) [ghstack-poisoned]
Collaborator
Author
|
In hindsight, the BFloat16 comparison with random test data is unlikely to work perfectly. It's sensitive to exact rounding and since NumPy is rounding to float32 precision, it will occasionally get different answers. |
Collaborator
We can disable the test for now. In the future we could use a fixture to ensure there are no rounding issues. |
mruberry
reviewed
Feb 2, 2021
| } | ||
| return floordiv; | ||
| }, | ||
| [](Vec256<scalar_t> a, Vec256<scalar_t> b) -> Vec256<scalar_t>{ |
Collaborator
There was a problem hiding this comment.
This is triggering some internal build issues. Adding a vectorized function can be a little tricky because we often have to stub them out on some platforms, like Android.
Since we're so close to the branch cut, I propose removing the copysign implementation and this vectorized implementation. We can file an issue and add them back in a later PR where we can take our time and focus on that issue.
Collaborator
Author
There was a problem hiding this comment.
@mruberry this should be good now. Removed all Vec256 changes and unvectorized floor_divide.
As mentioned in gh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. Differential Revision: [D26123271](https://our.internmc.facebook.com/intern/diff/D26123271) [ghstack-poisoned]
Collaborator
|
FYI there's another internal issue that I'm reviewing now. I'll keep this updated. |
Collaborator
|
Update: hacking through internal infra issues still. |
Collaborator
|
Update: blocking infra team confirms it's fixed its issue. This should land today. |
Contributor
Collaborator
|
Landed. Some changes had to be made internally:
|
laurentdupin
pushed a commit
to laurentdupin/pytorch
that referenced
this pull request
Apr 24, 2026
Summary: Pull Request resolved: pytorch#51706 Pull Request resolved: pytorch#50280 As mentioned in pytorchgh-43874, this adds a `rounding_mode={'true', 'trunc', 'floor'}` argument so `torch.div` can be used as a replacement for `floor_divide` during the transitional period. I've included dedicated kernels for truncated and floor division which aren't strictly necessary for float, but do perform significantly better (~2x) than doing true division followed by a separate rounding kernel. Note: I introduce new overloads for `aten::div` instead of just adding a default `rounding_mode` because various JIT passes rely on the exact operator schema. Test Plan: Imported from OSS Reviewed By: ngimel Differential Revision: D26123271 Pulled By: mruberry fbshipit-source-id: 51a83717602114597ec9c4d946e35a392eb01d46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
As mentioned in gh-43874, this adds a
rounding_mode={'true', 'trunc', 'floor'}argument so
torch.divcan be used as a replacement forfloor_divideduringthe transitional period.
I've included dedicated kernels for truncated and floor division which
aren't strictly necessary for float, but do perform significantly better (~2x) than
doing true division followed by a separate rounding kernel.
Note: I introduce new overloads for
aten::divinstead of just adding a defaultrounding_modebecause various JIT passes rely on the exact operator schema.Differential Revision: D26123271