Fix torch.pow when the scalar base is a complex number #45259

RockingJavaBean · 2020-09-24T04:01:19Z

Fixes #43829

dr-ci · 2020-09-24T04:56:35Z

💊 CI failures summary and remediations

As of commit 9c9101e (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

Failed: pr/pytorch-linux-bionic-rocm3.7-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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 25 times.

RockingJavaBean · 2020-09-25T16:29:18Z

@JackCaoG This PR adds tests to check complex base numbers of the torch.pow function.
The xla tests failed due to precision issues.

Sep 25 09:34:10 FAIL [0.067s]: test_complex_scalar_pow_tensor_xla_float64 (__main__.TestTorchDeviceTypeXLA)
...
Sep 25 09:34:10 AssertionError: False is not true : Tensors failed to compare as equal! 
Imaginary parts failed to compare as equal! With rtol=0.001 and atol=0.001, 
found 100 element(s) (out of 100) whose difference(s) exceeded the margin of error (including 0 nan comparisons). 
The greatest difference was 0.5494986167779923 (0.5494986167779923 vs. 0.0), which occurred at index 19.

I noticed the testes for float bases are skipped in the xla CI job.

Sep 25 09:21:16   test_float_scalar_pow_float_tensor_xla (__main__.TestTorchDeviceTypeXLA) ... skip (0.002s)

Please kindly confirm whether I could annotate complex base tests with onlyOnCPUAndCUDA for now, and later we may add test_complex_scalar_pow_tensor to pytorch_test_base.py to skip the tests.

JackCaoG · 2020-09-25T17:08:36Z

@RockingJavaBean Thanks for the heads up, I think you can mark test to be onlyOnCPUAndCUDA for now.

anjali411

LGTM once the complex test is skipped for xla

…pow_complex

facebook-github-bot

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

codecov · 2020-09-28T14:09:27Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #45259      +/-   ##
==========================================
- Coverage   68.15%   68.15%   -0.01%     
==========================================
  Files         396      396              
  Lines       51133    51133              
==========================================
- Hits        34851    34850       -1     
- Misses      16282    16283       +1

Impacted Files	Coverage Δ
torch/testing/_internal/expecttest.py	`77.55% <0.00%> (-1.03%)`	⬇️

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 95a97e5...9c9101e. Read the comment docs.

facebook-github-bot · 2020-09-29T02:20:31Z

@anjali411 merged this pull request in 0c8a600.

zou3519 · 2020-10-29T16:20:16Z

test/test_torch.py

+    @dtypes(*(torch.testing.get_all_dtypes(include_bool=False, include_bfloat16=False)))
+    def test_complex_scalar_pow_tensor(self, device, dtype):
+        complexes = [0.5j, 1. + 1.j, -1.5j, 2.2 - 1.6j]
+        tensor = torch.rand(100).to(dtype=dtype, device=device)


For integer dtypes this resolves to a tensor full of zeros, which may not be the most interesting test case. We have a make_tensor function to generate a random tensor that would be nicer to use:

pytorch/torch/testing/_internal/common_utils.py

Lines 1441 to 1442 in ecdbea7

def make_tensor(size, device: torch.device, dtype: torch.dtype, *,

low, high, requires_grad: bool = False) -> torch.Tensor:

Thanks so much for pointing out this issue, and offering the kind tip of using the make_tensor function.
#47101 has been created to address this comment and special cases of zero exponents and the 1 + 0j base are added as well, please kindly help review.

Summary: Related #45259 This PR is to address the #45259 (comment) - leverage the `make_tensor` function to generate a random tensor as the exponent, preventing the full zeros for the integer exponent. - add some special cases for the zero exponents and the `1 + 0j` base. Pull Request resolved: #47101 Reviewed By: mruberry Differential Revision: D24682430 Pulled By: zou3519 fbshipit-source-id: f559dc0ba08f37ae070036fb25a52ede17a24149

RockingJavaBean marked this pull request as draft September 24, 2020 04:01

pytorchbot added the open source label Sep 24, 2020

RockingJavaBean marked this pull request as ready for review September 24, 2020 15:34

RockingJavaBean changed the title ~~[WIP] Fix torch.pow when self is complex scalar~~ Fix torch.pow when self is complex scalar Sep 24, 2020

RockingJavaBean changed the title ~~Fix torch.pow when self is complex scalar~~ Fix torch.pow when the scalar base is a complex number Sep 24, 2020

RockingJavaBean added 2 commits September 25, 2020 16:08

Fix checking logic for complex scalar base

5fca1f0

add tests for scalar complex bases

e387d43

RockingJavaBean force-pushed the fix_pow_complex branch from bffd0af to e387d43 Compare September 25, 2020 08:08

zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 25, 2020

zou3519 requested a review from anjali411 September 25, 2020 14:11

anjali411 approved these changes Sep 25, 2020

View reviewed changes

RockingJavaBean added 3 commits September 26, 2020 10:20

skip xla tests for complex base torch.pow

1fd1e52

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

903d525

…pow_complex

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

9c9101e

…pow_complex

facebook-github-bot reviewed Sep 28, 2020

View reviewed changes

facebook-github-bot closed this in 0c8a600 Sep 29, 2020

facebook-github-bot added the merged label Sep 29, 2020

RockingJavaBean deleted the fix_pow_complex branch September 29, 2020 03:09

mruberry added the Merged label Oct 28, 2020

zou3519 reviewed Oct 29, 2020

View reviewed changes

RockingJavaBean mentioned this pull request Oct 30, 2020

Enhance the torch.pow testcase for the complex scalar base #47101

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix torch.pow when the scalar base is a complex number #45259

Fix torch.pow when the scalar base is a complex number #45259

RockingJavaBean commented Sep 24, 2020

dr-ci bot commented Sep 24, 2020 •

edited

RockingJavaBean commented Sep 25, 2020

JackCaoG commented Sep 25, 2020

anjali411 left a comment

facebook-github-bot left a comment

codecov bot commented Sep 28, 2020 •

edited

facebook-github-bot commented Sep 29, 2020

zou3519 Oct 29, 2020

RockingJavaBean Oct 30, 2020

	def make_tensor(size, device: torch.device, dtype: torch.dtype, *,
	low, high, requires_grad: bool = False) -> torch.Tensor:

Fix torch.pow when the scalar base is a complex number #45259

Fix torch.pow when the scalar base is a complex number #45259

Conversation

RockingJavaBean commented Sep 24, 2020

dr-ci bot commented Sep 24, 2020 • edited

💊 CI failures summary and remediations

ci.pytorch.org: 1 failed

RockingJavaBean commented Sep 25, 2020

JackCaoG commented Sep 25, 2020

anjali411 left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 28, 2020 • edited

Codecov Report

facebook-github-bot commented Sep 29, 2020

zou3519 Oct 29, 2020

Choose a reason for hiding this comment

RockingJavaBean Oct 30, 2020

Choose a reason for hiding this comment

dr-ci bot commented Sep 24, 2020 •

edited

codecov bot commented Sep 28, 2020 •

edited