[complex32] cat, fill_(partial), item #75010

kshitij12345 · 2022-03-31T08:23:04Z

Reference : #74537

cat_backwards (on CUDA) requires support for fill, have added support for fill. (Also fill requires item support)

Now fill backward requires sum (will add it in later PR).

facebook-github-bot · 2022-03-31T08:23:10Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/75010
Need help or want to give feedback on the CI? Visit our office hours

💊 CI failures summary and remediations

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

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

torch/testing/_internal/common_methods_invocations.py

kshitij12345 · 2022-03-31T08:24:28Z

torch/testing/_internal/common_methods_invocations.py

@@ -14301,8 +14304,10 @@ def ref_pairwise_distance(input1, input2):
           supports_fwgrad_bwgrad=True,
           # https://github.com/pytorch/pytorch/issues/66357
           check_batched_forward_grad=False,
-           dtypes=all_types_and_complex_and(torch.bool, torch.float16, torch.bfloat16),
+           dtypes=all_types_and_complex_and(torch.complex32, torch.bool, torch.float16, torch.bfloat16),
+           backward_dtypes=floating_and_complex_types_and(torch.float16, torch.bfloat16),


This should start failing once we support sum for complex32 (as it is used in the backward)

kshitij12345 · 2022-03-31T08:49:58Z

aten/src/ATen/native/Scalar.cpp

@@ -20,8 +20,8 @@ Scalar item(const Tensor& self) {

 Scalar _local_scalar_dense_cpu(const Tensor& self) {
  Scalar r;
-  AT_DISPATCH_ALL_TYPES_AND_COMPLEX_AND3(
-    at::ScalarType::Half, at::ScalarType::Bool, at::ScalarType::BFloat16, self.scalar_type(), "_local_scalar_dense_cpu", [&] {
+  AT_DISPATCH_ALL_TYPES_AND_COMPLEX_AND4(


Used by call to item which fill_ calls for 0-D Tensor overload.

kshitij12345 · 2022-03-31T17:48:52Z

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

@@ -19,7 +19,7 @@ struct FillFunctor {
 };

 void fill_kernel_cuda(TensorIterator& iter, const Scalar& value) {
-  AT_DISPATCH_ALL_TYPES_AND_COMPLEX_AND3(at::ScalarType::Bool, at::ScalarType::Half, at::ScalarType::BFloat16, iter.dtype(), "fill_cuda", [&]() {
+  AT_DISPATCH_ALL_TYPES_AND_COMPLEX_AND4(kComplexHalf, kBool, kHalf, kBFloat16, iter.dtype(), "fill_cuda", [&]() {
    gpu_kernel(iter, FillFunctor<scalar_t>(value.to<scalar_t>()));


note to self: jiterate in upcoming PR

kshitij12345 · 2022-03-31T17:52:09Z

test/test_ops.py

+    @ops(op_db, allowed_dtypes=(torch.complex32,))
+    def test_complex_half_reference_testing(self, device, dtype, op):
+        if not op.supports_dtype(torch.complex32, device):
+            unittest.skip("Does not support complex32")


Couldn't filter in @ops decorator as helper to decide if a dtype is supported requires device

I'm confused about this. Isn't setting allowed_dtypes in the decorator sufficient? I thought the decorator accounted for the device?

You are right. That is sufficient! Thank you

kshitij12345 · 2022-03-31T17:55:35Z

@anjali411
I had a talk with @mruberry regarding adding a new meta-data supports_complex32 to handle the failures. But instead we decided to skip complex32 support by updating the test logic.

mruberry · 2022-03-31T17:57:33Z

test/test_ops.py

+
+        for sample in op.sample_inputs(device, dtype):
+            actual = op(sample.input, *sample.args, **sample.kwargs)
+            (inp, args, kwargs) = sample.transform(lambda x: x.to(torch.complex64))


In the future we likely want to update the numpy() method to handle this cast like it handles bfloat16, too:

pytorch/torch/testing/_internal/common_methods_invocations.py

Line 186 in 1a1f0d4

def numpy(self):

mruberry · 2022-03-31T17:58:13Z

test/test_torch.py

@@ -5194,6 +5194,11 @@ def make_tensor_wrapper(shape, dtype):
                    atol = 1e-2
                self.assertEqual(src, dst.copy_(t), rtol=rtol, atol=atol)

+    @dtypes(*all_types_and_complex_and(torch.bool, torch.half, torch.bfloat16, torch.complex32))
+    def test_item(self, device, dtype):


Are there any existing item tests?

I couldn't find any with simple search

mruberry · 2022-03-31T17:58:33Z

torch/testing/_internal/common_methods_invocations.py

@@ -9198,11 +9198,6 @@ def ref_pairwise_distance(input1, input2):
                    skips=(
                        # RuntimeError: Expected object of scalar type Float but got scalar type Double for second argument
                        DecorateInfo(unittest.skip("Skipped!"), 'TestBinaryUfuncs', 'test_type_promotion'),
-                        # File "test/test_binary_ufuncs.py", line 334, in test_batch_vs_slicing


Nice cleanup

mruberry · 2022-03-31T17:59:42Z

test/test_ops.py

@@ -67,8 +67,13 @@ def tearDownClass(cls):
    @onlyNativeDeviceTypes
    @ops(op_db, dtypes=OpDTypes.none)
    def test_dtypes(self, device, op):
+        # Check complex32 support only if the op claims.
+        # TODO: Once the complex32 support is better, we should add check for complex32 unconditionally.
+        include_complex32 = ((torch.complex32,) if op.supports_dtype(torch.complex32, device) else ())


Nice updates

anjali411

@pytorchbot merge this please

pytorch-bot · 2022-04-01T15:17:40Z

+1

github-actions · 2022-04-01T15:19:40Z

Hey @kshitij12345.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: Reference : #74537 `cat_backwards` (on CUDA) requires support for `fill`, have added support for `fill`. (Also `fill` requires `item` support) Now `fill` backward requires `sum` (will add it in later PR). Pull Request resolved: #75010 Approved by: https://github.com/anjali411 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/65b65af2367c451a88672e9cffd40bd839d7ccb7 Reviewed By: atalman Differential Revision: D35317407 fbshipit-source-id: f735f5090539a4f598b413a3c40181a10bd0ad58

kshitij12345 added 2 commits March 31, 2022 07:34

[complex32] cat, fill_(partial)

abb6759

fixes

5d4cf01

facebook-github-bot added the cla signed label Mar 31, 2022

kshitij12345 commented Mar 31, 2022

View reviewed changes

torch/testing/_internal/common_methods_invocations.py Outdated Show resolved Hide resolved

kshitij12345 commented Mar 31, 2022

View reviewed changes

pytorchbot added the open source label Mar 31, 2022

Merge branch 'master' into develop/complex32/cat-fill

9fe32da

kshitij12345 commented Mar 31, 2022

View reviewed changes

kshitij12345 added 2 commits March 31, 2022 08:56

update comments

2f8ac91

remove c++ test

0b0eed7

kshitij12345 changed the title ~~[complex32] cat, fill_(partial)~~ [complex32] cat, fill_(partial), item Mar 31, 2022

kshitij12345 added 3 commits March 31, 2022 11:29

Merge branch 'master' into develop/complex32/cat-fill

17c3eb2

update decorator

5b08cd5

remove support_complex32 and add similar logic

45ad3b9

kshitij12345 commented Mar 31, 2022

View reviewed changes

kshitij12345 marked this pull request as ready for review March 31, 2022 17:53

kshitij12345 requested review from mruberry and anjali411 March 31, 2022 17:53

mruberry reviewed Mar 31, 2022

View reviewed changes

dagitses assigned anjali411 Apr 1, 2022

dagitses added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 1, 2022

anjali411 approved these changes Apr 1, 2022

View reviewed changes

pytorchmergebot closed this in 65b65af Apr 1, 2022

kshitij12345 added module: complex Related to complex number support in PyTorch module: half Related to float16 half-precision floats labels Apr 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[complex32] cat, fill_(partial), item #75010

[complex32] cat, fill_(partial), item #75010

kshitij12345 commented Mar 31, 2022 •

edited

facebook-github-bot commented Mar 31, 2022 •

edited

kshitij12345 Mar 31, 2022 •

edited

kshitij12345 Mar 31, 2022

kshitij12345 Mar 31, 2022 •

edited

kshitij12345 Mar 31, 2022

mruberry Mar 31, 2022

kshitij12345 Mar 31, 2022

kshitij12345 commented Mar 31, 2022

mruberry Mar 31, 2022

mruberry Mar 31, 2022

kshitij12345 Mar 31, 2022

mruberry Mar 31, 2022

mruberry Mar 31, 2022

anjali411 left a comment

pytorch-bot bot commented Apr 1, 2022

github-actions bot commented Apr 1, 2022

[complex32] cat, fill_(partial), item #75010

[complex32] cat, fill_(partial), item #75010

Conversation

kshitij12345 commented Mar 31, 2022 • edited

facebook-github-bot commented Mar 31, 2022 • edited

🔗 Helpful links

💊 CI failures summary and remediations

kshitij12345 Mar 31, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshitij12345 Mar 31, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshitij12345 commented Mar 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anjali411 left a comment

Choose a reason for hiding this comment

pytorch-bot bot commented Apr 1, 2022

github-actions bot commented Apr 1, 2022

kshitij12345 commented Mar 31, 2022 •

edited

facebook-github-bot commented Mar 31, 2022 •

edited

kshitij12345 Mar 31, 2022 •

edited

kshitij12345 Mar 31, 2022 •

edited