Skip to content

Conversation

@kshitij12345
Copy link
Collaborator

Fixes #43360

@kshitij12345
Copy link
Collaborator Author

@nairbv Please review:)

Also not sure where the test should be?

test_type_promotion (which I think tests the general type promotion logic and not each operator)
Or
Test against numpy which verifies output value as well as dtype in test_torch.py (using compare_with_numpy)?

@nairbv
Copy link
Collaborator

nairbv commented Aug 24, 2020

Also not sure where the test should be?

test_type_promotion (which I think tests the general type promotion logic and not each operator)
Or
Test against numpy which verifies output value as well as dtype in test_torch.py (using compare_with_numpy)?

I'd go with test_type_promotion. There are tests in there already for unary versions of trigonometric ops, and we'd specifically be testing for the promotion behavior. In some cases our promotion behavior might also differ from numpy.

@ngimel ngimel requested a review from nairbv August 25, 2020 03:52
@ngimel ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 25, 2020
@kshitij12345
Copy link
Collaborator Author

Sure. Will add test there. Thanks.

@kshitij12345
Copy link
Collaborator Author

@nairbv Please review :)

@kshitij12345
Copy link
Collaborator Author

Gentle Ping :)

@kshitij12345
Copy link
Collaborator Author

@nairbv Gentle Ping :)

@kshitij12345
Copy link
Collaborator Author

Gentle Ping :)

1 similar comment
@kshitij12345
Copy link
Collaborator Author

Gentle Ping :)

@mruberry mruberry self-requested a review September 17, 2020 16:30
Tensor result = at::empty({0}, self.options());
return native::atan2_out(result, self, other);
Tensor result;
auto iter = TensorIterator::binary_op(result, self, other);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is cool and completely correct. Would you mind updating it, though? We now have TensorIterator::binary_float_op, which applies to atan2. That makes it so integer inputs to atan2 are always promoted to float values (just like div).

Would you make atan2 a binary_float_op and test that integer inputs are accepted and return the expected float values? This would require expanding your test in test_type_promotion and the list of types we test atan2 to with here:

('atan2', '', _medium_2d, lambda t, d: [_medium_2d(t, d)], 1e-2, 1e-5, 1e-5, _float_types),

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. Thanks!

# https://github.com/pytorch/pytorch/issues/28502
a = torch.tensor([[True, True], [False, True]], device=device)
self.assertEqual(a.t() == 0, a.t() == False) # noqa: E712

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use the @dtypes decorator with pairs of dtypes if you like. For example:

@dtypes(*product(torch.testing.get_all_dtypes(), torch.testing.get_all_dtypes())
def test_atan2_type_promotion(self, device, dtypes):
  dtype1, dtype2 = dtypes
  ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice! Will use that thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually it will be hard to use as include_half is chosen based on device type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use

@dtypesIfCUDA(...)
@dtypes(...)

CUDA will get the dtypes in the first decorator, all other device types will get the dtypes in the second decorator. If you want CPU-specific dtypes there's also @dtypesIfCPU.

@codecov
Copy link

codecov bot commented Sep 17, 2020

Codecov Report

Merging #43466 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #43466   +/-   ##
=======================================
  Coverage   67.83%   67.83%           
=======================================
  Files         384      384           
  Lines       49962    49962           
=======================================
+ Hits        33892    33893    +1     
+ Misses      16070    16069    -1     
Impacted Files Coverage Δ
torch/testing/_internal/expecttest.py 78.57% <0.00%> (+1.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e5045e...a558f43. Read the comment docs.

@dr-ci
Copy link

dr-ci bot commented Sep 17, 2020

💊 CI failures summary and remediations

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


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

ci.pytorch.org: 1 failed


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 or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 21 times.

@kshitij12345
Copy link
Collaborator Author

The updated tests fail with

======================================================================
ERROR [0.004s]: test_atan2_inplace_cpu_int16 (__main__.TestTensorDeviceOpsCPU)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Short

======================================================================
ERROR [0.004s]: test_atan2_inplace_cpu_int32 (__main__.TestTensorDeviceOpsCPU)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Int

======================================================================
ERROR [0.004s]: test_atan2_inplace_cpu_int64 (__main__.TestTensorDeviceOpsCPU)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Long

======================================================================
ERROR [0.004s]: test_atan2_inplace_cpu_int8 (__main__.TestTensorDeviceOpsCPU)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Char

======================================================================
ERROR [0.003s]: test_atan2_inplace_cpu_uint8 (__main__.TestTensorDeviceOpsCPU)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Byte

======================================================================
ERROR [0.004s]: test_atan2_inplace_cuda_int16 (__main__.TestTensorDeviceOpsCUDA)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Short

======================================================================
ERROR [0.004s]: test_atan2_inplace_cuda_int32 (__main__.TestTensorDeviceOpsCUDA)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Int

======================================================================
ERROR [0.004s]: test_atan2_inplace_cuda_int64 (__main__.TestTensorDeviceOpsCUDA)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Long

======================================================================
ERROR [0.004s]: test_atan2_inplace_cuda_int8 (__main__.TestTensorDeviceOpsCUDA)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Char

======================================================================
ERROR [0.003s]: test_atan2_inplace_cuda_uint8 (__main__.TestTensorDeviceOpsCUDA)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 270, in instantiated_test
    result = test_fn(self, *args)
  File "test_torch.py", line 20173, in fn
    cpu_result = getattr(cpu_tensor, op_str)(*cpu_args)
RuntimeError: result type Float can't be cast to the desired output type Byte

Which makes sense. Updating the generated test code for this.

Note that div is not tested for behavior with integer types currently. Will update that as well.

def is_float(dtype):
return dtype in torch.testing.get_all_fp_dtypes(include_half=include_half, include_bfloat16=False)

def get_binary_float_result_type(x, y):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there any function (like torch.result_type) which actually does this for binary_float_op promotion behavior?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately no. Which makes torch.result_type kind of misleading.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh ok.

@kshitij12345
Copy link
Collaborator Author

@mruberry

PR might need an update from XLA side as well, I guess.
https://app.circleci.com/jobs/github/pytorch/pytorch/7578739

======================================================================
Sep 18 16:08:01 ERROR [0.043s]: test_atan2_xla_int16 (__main__.TestTensorDeviceOpsXLA)
Sep 18 16:08:01 ----------------------------------------------------------------------
Sep 18 16:08:01 Traceback (most recent call last):
Sep 18 16:08:01   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 270, in instantiated_test
Sep 18 16:08:01     result = test_fn(self, *args)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 20200, in fn
Sep 18 16:08:01     self.assertEqual(cpu_result, device_result, atol=precision, rtol=0, exact_dtype=False)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 542, in assertEqual
Sep 18 16:08:01     x, y = self.prepare_for_compare(x, y)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 489, in prepare_for_compare
Sep 18 16:08:01     y = ty.to(device='cpu')
Sep 18 16:08:01 RuntimeError: Unimplemented: From /job:localservice/replica:0/task:0:
Sep 18 16:08:01 2 root error(s) found.
Sep 18 16:08:01   (0) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01   (1) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01 	 [[XRTCompile_G3]]
Sep 18 16:08:01 0 successful operations.
Sep 18 16:08:01 0 derived errors ignored.
Sep 18 16:08:01 
Sep 18 16:08:01 ======================================================================
Sep 18 16:08:01 ERROR [0.034s]: test_atan2_xla_int32 (__main__.TestTensorDeviceOpsXLA)
Sep 18 16:08:01 ----------------------------------------------------------------------
Sep 18 16:08:01 Traceback (most recent call last):
Sep 18 16:08:01   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 270, in instantiated_test
Sep 18 16:08:01     result = test_fn(self, *args)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 20200, in fn
Sep 18 16:08:01     self.assertEqual(cpu_result, device_result, atol=precision, rtol=0, exact_dtype=False)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 542, in assertEqual
Sep 18 16:08:01     x, y = self.prepare_for_compare(x, y)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 489, in prepare_for_compare
Sep 18 16:08:01     y = ty.to(device='cpu')
Sep 18 16:08:01 RuntimeError: Unimplemented: From /job:localservice/replica:0/task:0:
Sep 18 16:08:01 2 root error(s) found.
Sep 18 16:08:01   (0) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01   (1) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01 	 [[XRTCompile_G3]]
Sep 18 16:08:01 0 successful operations.
Sep 18 16:08:01 0 derived errors ignored.
Sep 18 16:08:01 
Sep 18 16:08:01 ======================================================================
Sep 18 16:08:01 ERROR [0.035s]: test_atan2_xla_int64 (__main__.TestTensorDeviceOpsXLA)
Sep 18 16:08:01 ----------------------------------------------------------------------
Sep 18 16:08:01 Traceback (most recent call last):
Sep 18 16:08:01   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 270, in instantiated_test
Sep 18 16:08:01     result = test_fn(self, *args)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 20200, in fn
Sep 18 16:08:01     self.assertEqual(cpu_result, device_result, atol=precision, rtol=0, exact_dtype=False)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 542, in assertEqual
Sep 18 16:08:01     x, y = self.prepare_for_compare(x, y)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 489, in prepare_for_compare
Sep 18 16:08:01     y = ty.to(device='cpu')
Sep 18 16:08:01 RuntimeError: Unimplemented: From /job:localservice/replica:0/task:0:
Sep 18 16:08:01 2 root error(s) found.
Sep 18 16:08:01   (0) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01   (1) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01 	 [[XRTCompile_G3]]
Sep 18 16:08:01 0 successful operations.
Sep 18 16:08:01 0 derived errors ignored.
Sep 18 16:08:01 
Sep 18 16:08:01 ======================================================================
Sep 18 16:08:01 ERROR [0.035s]: test_atan2_xla_int8 (__main__.TestTensorDeviceOpsXLA)
Sep 18 16:08:01 ----------------------------------------------------------------------
Sep 18 16:08:01 Traceback (most recent call last):
Sep 18 16:08:01   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 270, in instantiated_test
Sep 18 16:08:01     result = test_fn(self, *args)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 20200, in fn
Sep 18 16:08:01     self.assertEqual(cpu_result, device_result, atol=precision, rtol=0, exact_dtype=False)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 542, in assertEqual
Sep 18 16:08:01     x, y = self.prepare_for_compare(x, y)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 489, in prepare_for_compare
Sep 18 16:08:01     y = ty.to(device='cpu')
Sep 18 16:08:01 RuntimeError: Unimplemented: From /job:localservice/replica:0/task:0:
Sep 18 16:08:01 2 root error(s) found.
Sep 18 16:08:01   (0) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01   (1) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01 	 [[XRTCompile_G20]]
Sep 18 16:08:01 0 successful operations.
Sep 18 16:08:01 0 derived errors ignored.
Sep 18 16:08:01 
Sep 18 16:08:01 ======================================================================
Sep 18 16:08:01 ERROR [0.034s]: test_atan2_xla_uint8 (__main__.TestTensorDeviceOpsXLA)
Sep 18 16:08:01 ----------------------------------------------------------------------
Sep 18 16:08:01 Traceback (most recent call last):
Sep 18 16:08:01   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 270, in instantiated_test
Sep 18 16:08:01     result = test_fn(self, *args)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 20200, in fn
Sep 18 16:08:01     self.assertEqual(cpu_result, device_result, atol=precision, rtol=0, exact_dtype=False)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 542, in assertEqual
Sep 18 16:08:01     x, y = self.prepare_for_compare(x, y)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/pytorch_test_base.py", line 489, in prepare_for_compare
Sep 18 16:08:01     y = ty.to(device='cpu')
Sep 18 16:08:01 RuntimeError: Unimplemented: From /job:localservice/replica:0/task:0:
Sep 18 16:08:01 2 root error(s) found.
Sep 18 16:08:01   (0) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01   (1) Unimplemented: binary integer op 'atan2'
Sep 18 16:08:01 	 [[{{node XRTCompile}}]]
Sep 18 16:08:01 	 [[XRTCompile_G20]]
Sep 18 16:08:01 0 successful operations.
Sep 18 16:08:01 0 derived errors ignored.
Sep 18 16:08:01 
Sep 18 16:08:01 ======================================================================
Sep 18 16:08:01 FAIL [0.019s]: test_atan2_inplace_xla_int16 (__main__.TestTensorDeviceOpsXLA)
Sep 18 16:08:01 ----------------------------------------------------------------------
Sep 18 16:08:01 Traceback (most recent call last):
Sep 18 16:08:01   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 270, in instantiated_test
Sep 18 16:08:01     result = test_fn(self, *args)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 20185, in fn
Sep 18 16:08:01     device_result = getattr(device_tensor, op_str)(*device_args)
Sep 18 16:08:01 AssertionError: RuntimeError not raised
Sep 18 16:08:01 
Sep 18 16:08:01 ======================================================================
Sep 18 16:08:01 FAIL [0.019s]: test_atan2_inplace_xla_int32 (__main__.TestTensorDeviceOpsXLA)
Sep 18 16:08:01 ----------------------------------------------------------------------
Sep 18 16:08:01 Traceback (most recent call last):
Sep 18 16:08:01   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 270, in instantiated_test
Sep 18 16:08:01     result = test_fn(self, *args)
Sep 18 16:08:01   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 20185, in fn
Sep 18 16:08:01     device_result = getattr(device_tensor, op_str)(*device_args)
Sep 18 16:08:01 AssertionError: RuntimeError not raised
Sep 18 16:08:01 

@JackCaoG
Copy link
Collaborator

I will take a look at the XLA failure. @kshitij12345 Could you open an issue under pytorch/xla to track this?

@mruberry
Copy link
Collaborator

I will take a look at the XLA failure. @kshitij12345 Could you open an issue under pytorch/xla to track this?

We can also skip the XLA tests for now: @onlyOnCPUAndCUDA.

@JackCaoG
Copy link
Collaborator

I will take a look at the XLA failure. @kshitij12345 Could you open an issue under pytorch/xla to track this?

We can also skip the XLA tests for now: @onlyOnCPUAndCUDA.

It is fine, I made the change on xla side and verified that test passed with the fix.

@kshitij12345
Copy link
Collaborator Author

@JackCaoG Thanks for the fix. However it looks like the XLA failures still exist.

@JackCaoG
Copy link
Collaborator

I just merged the fix, you should see the test passing now

@kshitij12345
Copy link
Collaborator Author

Great thanks!

@kshitij12345
Copy link
Collaborator Author

@mruberry Gentle Ping:)

@mruberry mruberry self-requested a review September 22, 2020 04:21
Copy link
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.

Nice work as usual, @kshitij12345!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@facebook-github-bot
Copy link
Contributor

@mruberry merged this pull request in 0b6b735.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

torch.atan2 type promotion is silently wrong

7 participants