Add complex support for torch.mean [CUDA] #47048

RockingJavaBean · 2020-10-29T11:25:50Z

Fixes #46982

dr-ci · 2020-10-29T12:23:19Z

💊 CI failures summary and remediations

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

1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_vulkan_linux_bionic_py3_6_clang9_build (1/1)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

Nov 06 16:53:16 sccache: error: couldn't connect to server

Nov 06 16:53:16 +++ eval 'extract_trap_cmd ' 
Nov 06 16:53:16 ++++ extract_trap_cmd 
Nov 06 16:53:16 ++++ printf '%s\n' '' 
Nov 06 16:53:16 +++ printf '%s\n' cleanup 
Nov 06 16:53:16 ++ trap -- ' 
Nov 06 16:53:16 cleanup' EXIT 
Nov 06 16:53:16 ++ [[ pytorch-vulkan-linux-bionic-py3.6-clang9-build != *pytorch-win-* ]] 
Nov 06 16:53:16 ++ which sccache 
Nov 06 16:53:16 ++ sccache --stop-server 
Nov 06 16:53:16 Stopping sccache server... 
Nov 06 16:53:16 sccache: error: couldn't connect to server 
Nov 06 16:53:16 sccache: caused by: Connection refused (os error 111) 
Nov 06 16:53:16 ++ true 
Nov 06 16:53:16 ++ rm /var/lib/jenkins/sccache_error.log 
Nov 06 16:53:16 rm: cannot remove '/var/lib/jenkins/sccache_error.log': No such file or directory 
Nov 06 16:53:16 ++ true 
Nov 06 16:53:16 ++ [[ pytorch-vulkan-linux-bionic-py3.6-clang9-build == *rocm* ]] 
Nov 06 16:53:16 ++ SCCACHE_ERROR_LOG=/var/lib/jenkins/sccache_error.log 
Nov 06 16:53:16 ++ SCCACHE_IDLE_TIMEOUT=1200 
Nov 06 16:53:16 ++ RUST_LOG=sccache::server=error 
Nov 06 16:53:16 ++ sccache --start-server

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 33 times.

aten/src/ATen/native/cuda/ReduceMomentKernel.cu

codecov · 2020-11-02T07:01:59Z

Codecov Report

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

@@             Coverage Diff             @@
##           master   #47048       +/-   ##
===========================================
+ Coverage   35.95%   53.27%   +17.31%     
===========================================
  Files         438     2747     +2309     
  Lines       55454   254304   +198850     
===========================================
+ Hits        19939   135476   +115537     
- Misses      35515   118828    +83313

RockingJavaBean · 2020-11-02T12:51:00Z

@anjali411 Thanks so much for reviewing this PR.
The latest PR leverages the explicit template specialization to handle factor with complex tensors, instead of casting factor to scalar_t.
The CI checks all passed, please kindly help review.

anjali411 · 2020-11-03T16:24:29Z

aten/src/ATen/native/cuda/ReduceMomentKernel.cu

@@ -36,26 +36,40 @@ static void std_var_kernel_cuda(TensorIterator& iter, bool unbiased, bool take_s

 template <typename scalar_t, typename acc_t=scalar_t, typename out_t=scalar_t>


setting acc_t = typename c10::scalar_value_type<scalar_t>::type should resolve the issue here

c10::scalar_value_type<scalar_t>::type returns scalar_t for all non-complex dtypes and returns T for c10::complex<T>.

Thanks so much for the tip. The latest code now manipulates c10::scalar_value_type<acc_t>::type to get the type of the factor, the overload functions for complex numbers are not needed.

anjali411 · 2020-11-03T16:28:08Z

@anjali411 Thanks so much for reviewing this PR.
The latest PR leverages the explicit template specialization to handle factor with complex tensors, instead of casting factor to scalar_t.

Hi @RockingJavaBean I think we shouldn't need to define overload functions for complex types, after the change I suggested in my comment. But this PR looks great overall, and should be ready to merge after that change!

…h_mean_complex

facebook-github-bot

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

anjali411 · 2020-11-04T17:06:15Z

test/test_torch.py

@@ -514,7 +514,8 @@ def test_mean_dim(self):
            self._test_dim_ops(
                lambda t, d: t.mean(d),
                lambda n, d: n.mean(d),
-                use_integral=False)
+                use_integral=False,
+                use_complex=True)


This test doesn't run on CUDA. can you please extend the test for mean in tensor_op_tests to also test complex dtypes?

Thanks for pointing this out, the tests for complex dtypes are added to tensor_op_tests.

anjali411

let's update the CUDA test for mean to test complex dtypes as well

…h_mean_complex

RockingJavaBean · 2020-11-05T12:48:46Z

@anjali411 I'm really grateful for your tip on c10::scalar_value_type, and the tests for complex inputs of torch.mean function are updated.
Please kindly help review.

anjali411

lgtm and the windows test failure is an upstream test failure

facebook-github-bot

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

RockingJavaBean · 2020-11-05T16:11:34Z

@anjali411 thanks so much for reviewing this PR, the CUDA tests for torch.mean passed before the upstream failure CI check terminated according to the circleci log.

test_mean_cuda_bfloat16 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.005s)
test_mean_cuda_complex128 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_cuda_complex64 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_cuda_float16 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_cuda_float32 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.003s)
test_mean_cuda_float64 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_dim_cuda_bfloat16 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_dim_cuda_complex128 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.005s)
test_mean_dim_cuda_complex64 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_dim_cuda_float16 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_dim_cuda_float32 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_dim_cuda_float64 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.003s)
test_mean_neg_dim_cuda_bfloat16 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.005s)
test_mean_neg_dim_cuda_complex128 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_neg_dim_cuda_complex64 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_neg_dim_cuda_float16 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_neg_dim_cuda_float32 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.004s)
test_mean_neg_dim_cuda_float64 (__main__.TestTensorDeviceOpsCUDA) ... ok (0.003s

anjali411 · 2020-11-06T16:33:41Z

@RockingJavaBean can you please rebase?

…h_mean_complex

RockingJavaBean · 2020-11-06T16:46:07Z

@anjali411 thank you so much for the kind reminder, I just rebased this PR with the latest code.

facebook-github-bot

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

facebook-github-bot · 2020-11-07T07:10:40Z

@anjali411 merged this pull request in f90da88.

pytorchbot added the open source label Oct 29, 2020

anjali411 reviewed Oct 29, 2020

View reviewed changes

aten/src/ATen/native/cuda/ReduceMomentKernel.cu Outdated Show resolved Hide resolved

anjali411 added the module: complex Related to complex number support in PyTorch label Oct 30, 2020

RockingJavaBean changed the title ~~[WIP] Enable complex numbers for torch.mean of CUDA path~~ Enable complex numbers for torch.mean of CUDA path Oct 30, 2020

ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Oct 30, 2020

facebook-github-bot added the cla signed label Nov 2, 2020

Enable complex numbers for torch.mean of CUDA path

b0de1be

RockingJavaBean force-pushed the torch_mean_complex branch from d618111 to b0de1be Compare November 2, 2020 07:01

RockingJavaBean requested a review from anjali411 November 3, 2020 13:15

anjali411 reviewed Nov 3, 2020

View reviewed changes

RockingJavaBean added 2 commits November 4, 2020 19:33

use c10::scalar_value_type to get factor type

5154e70

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

82d43fe

…h_mean_complex

facebook-github-bot reviewed Nov 4, 2020

View reviewed changes

anjali411 approved these changes Nov 4, 2020

View reviewed changes

anjali411 reviewed Nov 4, 2020

View reviewed changes

RockingJavaBean added 2 commits November 5, 2020 11:42

expand the complex numbers test for mean to cuda

25615ba

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

79e93e0

…h_mean_complex

RockingJavaBean requested a review from anjali411 November 5, 2020 12:48

anjali411 approved these changes Nov 5, 2020

View reviewed changes

facebook-github-bot reviewed Nov 5, 2020

View reviewed changes

anjali411 changed the title ~~Enable complex numbers for torch.mean of CUDA path~~ Add complex support for torch.mean [CUDA] Nov 5, 2020

anjali411 mentioned this pull request Nov 5, 2020

Complex Numbers Support #33152

Closed

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

de87d22

…h_mean_complex

facebook-github-bot reviewed Nov 6, 2020

View reviewed changes

facebook-github-bot closed this in f90da88 Nov 7, 2020

facebook-github-bot added the Merged label Nov 7, 2020

RockingJavaBean deleted the torch_mean_complex branch November 10, 2020 05:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add complex support for torch.mean [CUDA] #47048

Add complex support for torch.mean [CUDA] #47048

RockingJavaBean commented Oct 29, 2020

dr-ci bot commented Oct 29, 2020 •

edited

codecov bot commented Nov 2, 2020 •

edited

RockingJavaBean commented Nov 2, 2020

anjali411 Nov 3, 2020

RockingJavaBean Nov 5, 2020

anjali411 commented Nov 3, 2020

facebook-github-bot left a comment

anjali411 Nov 4, 2020

RockingJavaBean Nov 5, 2020

anjali411 left a comment

RockingJavaBean commented Nov 5, 2020

anjali411 left a comment

facebook-github-bot left a comment

RockingJavaBean commented Nov 5, 2020

anjali411 commented Nov 6, 2020

RockingJavaBean commented Nov 6, 2020

facebook-github-bot left a comment

facebook-github-bot commented Nov 7, 2020

		@@ -36,26 +36,40 @@ static void std_var_kernel_cuda(TensorIterator& iter, bool unbiased, bool take_s

		template <typename scalar_t, typename acc_t=scalar_t, typename out_t=scalar_t>

Add complex support for torch.mean [CUDA] #47048

Add complex support for torch.mean [CUDA] #47048

Conversation

RockingJavaBean commented Oct 29, 2020

dr-ci bot commented Oct 29, 2020 • edited

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_vulkan_linux_bionic_py3_6_clang9_build (1/1)

codecov bot commented Nov 2, 2020 • edited

Codecov Report

RockingJavaBean commented Nov 2, 2020

anjali411 Nov 3, 2020

Choose a reason for hiding this comment

RockingJavaBean Nov 5, 2020

Choose a reason for hiding this comment

anjali411 commented Nov 3, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

anjali411 Nov 4, 2020

Choose a reason for hiding this comment

RockingJavaBean Nov 5, 2020

Choose a reason for hiding this comment

anjali411 left a comment

Choose a reason for hiding this comment

RockingJavaBean commented Nov 5, 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

RockingJavaBean commented Nov 5, 2020

anjali411 commented Nov 6, 2020

RockingJavaBean commented Nov 6, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Nov 7, 2020

dr-ci bot commented Oct 29, 2020 •

edited

codecov bot commented Nov 2, 2020 •

edited