fix torch.linalg.norm and torch.norm for torch.complex32 datatype #133661

jiayisunx · 2024-08-16T04:02:27Z

Stack from ghstack (oldest at bottom):

-> fix torch.linalg.norm and torch.norm for torch.complex32 datatype #133661

Fix #132634.

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10

[ghstack-poisoned]

pytorch-bot · 2024-08-16T04:02:30Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/133661

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 9cb69b2 with merge base 028c5d3 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

ghstack-source-id: e1006ef Pull Request resolved: #133661

jiayisunx · 2024-08-16T07:09:07Z

@mingfeima , could you please review this PR? Thanks!

Skylion007 · 2024-08-16T16:41:58Z

We probably need to add these dtypes to our current testing framework.

mingfeima

LGTM, just remember to update test cases.

[ghstack-poisoned]

nikitaved · 2024-08-19T09:03:50Z

test/test_linalg.py

+            for ord in vector_ords:
+                res = torch.linalg.norm(x, ord, keepdim=keepdim)
+                res_float = torch.linalg.norm(x_cfloat, ord, keepdim=keepdim)


Isn't this op tested with OpInfos? Shall we add tests for the CPU there instead?

I got this warning when this op tested with OpInfos, ComplexHalf seems not fully supported yet:

/home/jiayisun/pytorch/torch/testing/_creation.py:233: UserWarning: ComplexHalf support is experimental and many operators don't support it yet. (Triggered internally at /home/jiayisun/pytorch/aten/src/ATen/EmptyTensor.cpp:46.) result = torch.empty(shape, device=device, dtype=dtype) Some dtypes for norm on device type cpu are only partially supported! The following dtypes only worked on some samples during backward: {torch.complex32}.

[ghstack-poisoned]

ghstack-source-id: fbb286d Pull Request resolved: #133661

[ghstack-poisoned]

ghstack-source-id: 3dcf76a Pull Request resolved: #133661

Skylion007 · 2024-08-21T14:41:53Z

torch/testing/_internal/common_methods_invocations.py

        sample_inputs_func=sample_inputs_norm,
-        dtypes=floating_and_complex_types_and(torch.float16, torch.bfloat16),
+        dtypes=floating_and_complex_types_and(torch.float16, torch.bfloat16, torch.chalf),
+        dtypesIfCUDA=floating_and_complex_types_and(torch.float16, torch.bfloat16),


Can we just add chalf support to CUDA too while we are at it? We already support complex types on GPU so should be an easy enough change. Just add kComplexHalf to the cuda operator as well and delete this line. :)

pytorch/aten/src/ATen/native/cuda/ReduceNormKernel.cu

Line 46 in 68425e6

AT_DISPATCH_FLOATING_AND_COMPLEX_TYPES(iter.input_dtype(), "norm_cuda", [&] {

No, issue #132634 also exists on CUDA, there is the same bug in GPU kernel, it runs in here: https://github.com/pytorch/pytorch/blob/main/aten/src/ATen/native/cuda/ReduceNormKernel.cu#L35, so we cannot fix it by modifying this line, I am not familiar with GPU kernels, maybe you can open a new PR to fix it.

@Skylion007 can you please review this PR again? thanks!

Skylion007 · 2024-08-21T14:45:20Z

aten/src/ATen/native/cpu/ReduceOpsKernel.cpp

    }

-    AT_DISPATCH_FLOATING_AND_COMPLEX_TYPES(iter.input_dtype(), "norm_cpu", [&] {
+    AT_DISPATCH_FLOATING_AND_COMPLEX_TYPES_AND3(kHalf, kBFloat16, kComplexHalf, iter.input_dtype(), "norm_cpu", [&] {


Are the kHalf and kBfloat16 types in the AND3 redundant? It seemed to support those types before??? or it suggests we aren't testing the right thing.

This is pretty complicated logic, let's update the GPU operator at the same time for chalf for parity.

Are the kHalf and kBfloat16 types in the AND3 redundant? It seemed to support those types before??? or it suggests we aren't testing the right thing.

Because I modified the logic above, now kHalf and kBfloat16 types may also be here (the case of iter.input_dtype() == kHalf/kBFloat16 and iter.dtype(0)!= kFloat)

I checked the UT is for this operator, but I got this warning when run the UT, ComplexHalf seems not fully supported yet:

/home/jiayisun/pytorch/torch/testing/_creation.py:233: UserWarning: ComplexHalf support is experimental and many operators don't support it yet. (Triggered internally at /home/jiayisun/pytorch/aten/src/ATen/EmptyTensor.cpp:46.) result = torch.empty(shape, device=device, dtype=dtype) Some dtypes for norm on device type cpu are only partially supported! The following dtypes only worked on some samples during backward: {torch.complex32}.

[ghstack-poisoned]

ghstack-source-id: 6cfa823 Pull Request resolved: #133661

jiayisunx · 2024-11-05T07:01:01Z

hi @Skylion007 , could you please review this PR again? thanks!

ghstack-source-id: 6cdfe65 Pull Request resolved: #133661

[ghstack-poisoned]

jiayisunx · 2024-11-07T00:46:33Z

@pytorchbot merge

pytorchmergebot · 2024-11-07T00:48:17Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

…torch#133661) Fix pytorch#132634. Pull Request resolved: pytorch#133661 Approved by: https://github.com/mingfeima, https://github.com/Skylion007

Update

5217bda

[ghstack-poisoned]

pytorch-bot bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Aug 16, 2024

jiayisunx added a commit that referenced this pull request Aug 16, 2024

fix torch.linalg.norm and torch.norm for torch.complex32 datatype

02b99e9

ghstack-source-id: e1006ef Pull Request resolved: #133661

pytorchbot added the open source label Aug 16, 2024

jiayisunx requested a review from mingfeima August 16, 2024 06:40

mingfeima approved these changes Aug 19, 2024

View reviewed changes

Update

4ef994c

[ghstack-poisoned]

jiayisunx requested review from IvanYashchuk, lezcano and nikitaved as code owners August 19, 2024 03:32

pytorch-bot bot added the release notes: linalg_frontend release notes category label Aug 19, 2024

Update

5a1b3ba

[ghstack-poisoned]

nikitaved reviewed Aug 19, 2024

View reviewed changes

lezcano removed their request for review August 19, 2024 09:32

Update

dab3060

[ghstack-poisoned]

jiayisunx requested a review from mruberry as a code owner August 19, 2024 14:07

jiayisunx added a commit that referenced this pull request Aug 19, 2024

fix torch.linalg.norm and torch.norm for torch.complex32 datatype

822706f

ghstack-source-id: fbb286d Pull Request resolved: #133661

Update

19d3c02

[ghstack-poisoned]

Update

31ad73a

[ghstack-poisoned]

jiayisunx added a commit that referenced this pull request Aug 20, 2024

fix torch.linalg.norm and torch.norm for torch.complex32 datatype

b560cae

ghstack-source-id: 3dcf76a Pull Request resolved: #133661

jiayisunx requested a review from nikitaved August 20, 2024 08:11

Skylion007 reviewed Aug 21, 2024

View reviewed changes

Update

1dd682b

[ghstack-poisoned]

jiayisunx added a commit that referenced this pull request Aug 22, 2024

fix torch.linalg.norm and torch.norm for torch.complex32 datatype

08775ce

ghstack-source-id: 6cfa823 Pull Request resolved: #133661

jiayisunx requested a review from Skylion007 August 22, 2024 07:42

jiayisunx added a commit that referenced this pull request Nov 6, 2024

fix torch.linalg.norm and torch.norm for torch.complex32 datatype

1dbae38

ghstack-source-id: 6cdfe65 Pull Request resolved: #133661

Update

9cb69b2

[ghstack-poisoned]

Skylion007 approved these changes Nov 6, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 7, 2024

pytorchmergebot added the merging label Nov 7, 2024

pytorchmergebot added the Merged label Nov 7, 2024

pytorchmergebot closed this in a59132b Nov 7, 2024

pytorchmergebot removed the merging label Nov 7, 2024

github-actions bot deleted the gh/jiayisunx/19/head branch December 8, 2024 02:17

fix torch.linalg.norm and torch.norm for torch.complex32 datatype #133661

fix torch.linalg.norm and torch.norm for torch.complex32 datatype #133661

Uh oh!

Conversation

jiayisunx commented Aug 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/133661

✅ No Failures

Uh oh!

jiayisunx commented Aug 16, 2024

Uh oh!

Skylion007 commented Aug 16, 2024

Uh oh!

mingfeima left a comment

Choose a reason for hiding this comment

Uh oh!

nikitaved Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

jiayisunx Aug 20, 2024 • 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!

Skylion007 Aug 21, 2024 • 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!

jiayisunx Aug 22, 2024 • 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!

jiayisunx Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

Skylion007 Aug 21, 2024

Choose a reason for hiding this comment

Uh oh!

Skylion007 Aug 21, 2024

Choose a reason for hiding this comment

Uh oh!

jiayisunx Aug 22, 2024

Choose a reason for hiding this comment

Uh oh!

jiayisunx Aug 22, 2024

Choose a reason for hiding this comment

Uh oh!

jiayisunx commented Nov 5, 2024

Uh oh!

jiayisunx commented Nov 7, 2024

Uh oh!

pytorchmergebot commented Nov 7, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jiayisunx commented Aug 16, 2024 •

edited

Loading

pytorch-bot bot commented Aug 16, 2024 •

edited

Loading

jiayisunx Aug 20, 2024 •

edited

Loading

Skylion007 Aug 21, 2024 •

edited

Loading

jiayisunx Aug 22, 2024 •

edited

Loading