Support `rmod` #58476

asi1024 · 2021-05-18T11:35:01Z

This PR implements torch.Tensor.__rmod__ and torch.remainder(scalar, tensor) for the compatibility with NumPy’s interface.
(cc: @mruberry, @rgommers, @emcastillo, @kmaehashi)

TODO:

Update tensor_binary_op in test/test_binary_ufuncs.py after Fix some tensor operators to return NotImplemented for invalid inputs #58216 is merged.

facebook-github-bot · 2021-05-18T11:35:07Z

💊 CI failures summary and remediations

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

1/1 failures possibly* introduced in this PR
- 1/1 non-scanned failure(s)

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm4.2-py3.6

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.

Click here to manually regenerate this comment.

codecov · 2021-05-19T16:01:47Z

Codecov Report

Merging #58476 (f625897) into master (ccd7141) will increase coverage by 0.03%.
The diff coverage is 87.21%.

❗ Current head f625897 differs from pull request most recent head e6c283c. Consider uploading reports for the commit e6c283c to get more accurate results

@@            Coverage Diff             @@
##           master   #58476      +/-   ##
==========================================
+ Coverage   76.44%   76.47%   +0.03%     
==========================================
  Files        1990     1992       +2     
  Lines      199690   199895     +205     
==========================================
+ Hits       152651   152868     +217     
+ Misses      47039    47027      -12

mruberry

Cool! This is great Python Array API compatibility, @asi1024

This is going to need a "forward" test to validate that the operation computes what it should when give a scalar x tensor input.

We have some tests for remainder here:

pytorch/test/test_binary_ufuncs.py

Line 1638 in 470cd64

def test_fmod_remainder(self, device, dtype):

That can probably be extended

cc @rgommers, looks like the Python Array API doesn't elaborate on rmod, but it might want to be more explicit about whether it should call fmod or remainder. NumPy's rmod looks like it calls remainder, so I assume that's what's intended?

mruberry · 2021-05-20T08:30:14Z

torch/testing/_internal/common_methods_invocations.py

+           sample_inputs_func=sample_inputs_rbinops,
+           supports_out=False,
+           skips=(SkipInfo('TestCommon', 'test_variant_consistency_jit',),),
+           supports_autograd=False,


Interesting... why doesn't this support autograd? torch.remainder supports autograd typically, right?

torch.remainder seems not to support autograd of the second argument currently.

>>> x1 = torch.tensor([3.], requires_grad=True) >>> x2 = torch.tensor([5.], requires_grad=True) >>> y = x1 % x2 Traceback (most recent call last): File "<stdin>", line 1, in <module> RuntimeError: the derivative for 'other' is not implemented

Interesting. Would you add a comment to that effect here, too?

rgommers · 2021-05-20T13:11:28Z

cc @rgommers, looks like the Python Array API doesn't elaborate on rmod, but it might want to be more explicit about whether it should call fmod or remainder. NumPy's rmod looks like it calls remainder, so I assume that's what's intended?

__mod__ has a note saying it uses remainder: https://data-apis.org/array-api/latest/API_specification/array_object.html#mod-self-other. So __rmod__ should do the same.

asi1024 · 2021-05-21T20:55:16Z

This is going to need a "forward" test to validate that the operation computes what it should when give a scalar x tensor input.

@mruberry I added torch.remainder(Scalar, Tensor) forward test now!

mruberry · 2021-05-23T02:54:58Z

test/test_binary_ufuncs.py

            for fn, inplace_fn, ref_fn in fns_list:
-                np_x = x.cpu().numpy()
+                np_x = x.cpu().numpy() if torch.is_tensor(x) else x


Nice update

mruberry · 2021-05-23T02:56:18Z

test/test_binary_ufuncs.py

+                    ((torch.fmod, torch.Tensor.fmod_, np.fmod),
+                     (torch.remainder, torch.Tensor.remainder_, np.remainder),))
+
+        for dividend, mod in product([5, 3.14], mods):


Add a comment here that this tests the scalar x tensor case

mruberry

This is looking good, @asi1024!; I have some small suggestions to add comments

Also, it looks like the tests didn't run, which is odd. Maybe pushing the comment updates will trigger them properly? Once we see the tests passing I think this is good to go!

asi1024 · 2021-05-24T20:10:03Z

@mruberry CI seems to have failed in TestNNCOpInfoCPU::test_unsupported___rmod___cpu_float32 test, but this test case is skipped in my local. Do you know what should I do to reproduce the test failure in my environment? Here is the test log of pytest.

test/test_jit_fuser_te.py::TestNNCOpInfoCPU::test_unsupported___rmod___cpu_float32 SKIPPED (Compiles with TensorExprKernel)

mruberry · 2021-05-24T20:20:33Z

@mruberry CI seems to have failed in TestNNCOpInfoCPU::test_unsupported___rmod___cpu_float32 test, but this test case is skipped in my local. Do you know what should I do to reproduce the test failure in my environment? Here is the test log of pytest.
test/test_jit_fuser_te.py::TestNNCOpInfoCPU::test_unsupported___rmod___cpu_float32 SKIPPED (Compiles with TensorExprKernel)

Before bothering to reproduce it, what about doing what it suggests and adding dunder rmod to the list here?

pytorch/test/test_jit_fuser_te.py

Line 1811 in 6093161

works_list = [

asi1024 · 2021-05-25T18:07:09Z

@mruberry Thanks! I fixed it as you commented and confirmed the test has passed!
The remaining CI failure is due to exceeding the time limit without output. Will this be green if CI is retriggered?

mruberry · 2021-05-29T07:52:06Z

@mruberry Thanks! I fixed it as you commented and confirmed the test has passed!
The remaining CI failure is due to exceeding the time limit without output. Will this be green if CI is retriggered?

Thank you, @asi1024. And don't worry about the remaining failure: it's unrelated. Also, if I don't respond to you in two business days please ping me. Sometimes notifications are lost. I'm sorry you waited so long for a response. I'll review the PR now, too.

mruberry

Great work, @asi1024! And PyTorch is one step closer to supporting the Python Array API!

facebook-github-bot · 2021-05-29T07:54:03Z

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

asi1024 · 2021-05-29T09:04:41Z

@mruberry Thanks! 😃

mruberry · 2021-05-30T05:25:20Z

Interestingly this hit an internal error:

stderr: caffe2/aten/src/ATen/native/BinaryOps.cpp:633:10: error: no matching function for call to 'remainder'
  return native::remainder(wrapped_scalar_tensor(self), other);
         ^~~~~~~~~~~~~~~~~
stderr: caffe2/aten/src/ATen/native/BinaryOps.cpp:633:10: error: no matching function for call to 'remainder'
  return native::remainder(wrapped_scalar_tensor(self), other);

maybe change the namespace to at?

asi1024 · 2021-05-31T17:35:27Z

Fixed the namespace and rebase, but the CI failed with rocm build failure. The message No problems were identified. is left. Could you help me figure out why it's failing?

facebook-github-bot · 2021-05-31T23:54:53Z

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

mruberry · 2021-05-31T23:55:16Z

No worries about the ROCm failure, it's in the base. We fixed it yesterday. Running this through internal tests now.

mruberry · 2021-06-05T21:49:06Z

Internal tests look good; landing this now

facebook-github-bot · 2021-06-05T23:20:45Z

@mruberry merged this pull request in 0a5bfa9.

asi1024 · 2021-06-06T09:45:53Z

@mruberry Thanks for your review!

Summary: Fixes pytorch#58035. This PR implements `torch.Tensor.__rmod__` and `torch.remainder(scalar, tensor)` for the compatibility with NumPy’s interface. (cc: mruberry, rgommers, emcastillo, kmaehashi) TODO: - [x] Update `tensor_binary_op` in test/test_binary_ufuncs.py after pytorch#58216 is merged. Pull Request resolved: pytorch#58476 Reviewed By: ngimel Differential Revision: D28776810 Pulled By: mruberry fbshipit-source-id: 74f8aea80f439ef2cc370333524e39971eeb7bf4

asi1024 requested a review from ezyang as a code owner May 18, 2021 11:35

facebook-github-bot added the cla signed label May 18, 2021

pytorchbot added the open source label May 18, 2021

asi1024 changed the title ~~Support __rmod__~~ [WIP] Support __rmod__ May 18, 2021

ezyang removed their request for review May 18, 2021 15:05

asi1024 force-pushed the rmod branch from 8eefe0e to f625897 Compare May 19, 2021 11:37

asi1024 force-pushed the rmod branch from f625897 to e6c283c Compare May 19, 2021 22:40

mruberry reviewed May 20, 2021

View reviewed changes

asi1024 force-pushed the rmod branch from e6c283c to 92a25be Compare May 21, 2021 20:49

asi1024 changed the title ~~[WIP] Support __rmod__~~ Support __rmod__ May 21, 2021

H-Huang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 21, 2021

mruberry reviewed May 23, 2021

View reviewed changes

asi1024 force-pushed the rmod branch from bed8765 to edef4e4 Compare May 24, 2021 08:04

asi1024 force-pushed the rmod branch from edef4e4 to d831a64 Compare May 25, 2021 05:38

mruberry approved these changes May 29, 2021

View reviewed changes

Add __rmod__

8c87de4

asi1024 added 6 commits May 30, 2021 19:31

Add __rmod__ test

b360506

Test __rmod__ in not_implemented tests

dcba294

Add torch.remainder(Scalar, Tensor) forward test

51a7f9f

Add a comment to test

21ab589

Fix works_list

a6034db

Change namespace

b669975

asi1024 force-pushed the rmod branch from d831a64 to b669975 Compare May 31, 2021 04:13

facebook-github-bot closed this in 0a5bfa9 Jun 5, 2021

facebook-github-bot added the Merged label Jun 5, 2021

asi1024 deleted the rmod branch June 6, 2021 09:46

pmeier added the module: python array api Issues related to the Python Array API label Jun 10, 2021

pmeier mentioned this pull request Jun 10, 2021

Python Array API New Operators Tracker #58742

Open

28 tasks

Support __rmod__ #58476

Support __rmod__ #58476

Uh oh!

Conversation

asi1024 commented May 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

💊 CI failures summary and remediations

ci.pytorch.org: 1 failed

Uh oh!

codecov bot commented May 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

mruberry May 20, 2021

Choose a reason for hiding this comment

Uh oh!

asi1024 May 21, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry May 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgommers commented May 20, 2021

Uh oh!

asi1024 commented May 21, 2021

Uh oh!

mruberry May 23, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry May 23, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

asi1024 commented May 24, 2021

Uh oh!

mruberry commented May 24, 2021

Uh oh!

asi1024 commented May 25, 2021

Uh oh!

mruberry commented May 29, 2021

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented May 29, 2021

Uh oh!

asi1024 commented May 29, 2021

Uh oh!

mruberry commented May 30, 2021

Uh oh!

asi1024 commented May 31, 2021

Uh oh!

facebook-github-bot commented May 31, 2021

Uh oh!

mruberry commented May 31, 2021

Uh oh!

mruberry commented Jun 5, 2021

Uh oh!

facebook-github-bot commented Jun 5, 2021

Uh oh!

asi1024 commented Jun 6, 2021

Uh oh!

Uh oh!

Support `rmod` #58476

Support `rmod` #58476

asi1024 commented May 18, 2021 •

edited

Loading

facebook-github-bot commented May 18, 2021 •

edited

Loading

codecov bot commented May 19, 2021 •

edited

Loading

mruberry May 23, 2021 •

edited

Loading