Support `dtype` kwarg in `_foreach_norm` #125665

crcrpar · 2024-05-07T08:37:11Z

Fixes #125040

cc @awgu

pytorch-bot · 2024-05-07T08:37:14Z

🔗 Helpful Links

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

📄 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.

❌ 1 New Failure

As of commit 1e300d6 with merge base 5fb11cd ():

NEW FAILURE - The following job has failed:

trunk / linux-focal-rocm6.1-py3.8 / test (distributed, 1, 1, linux.rocm.gpu) (gh)
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! KeyboardInterrupt !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

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

janeyx99

Took a look at the changes, they seem reasonable but the meta test failures are real.

Typically I think we've skipped when the original OpInfos are broken, but I have not dug into the problem here so it may be a different concern.

crcrpar · 2024-05-15T01:36:23Z

Took a look at the changes, they seem reasonable but the meta test failures are real.

Typically I think we've skipped when the original OpInfos are broken, but I have not dug into the problem here so it may be a different concern.

yes, they are real failures but not that opinfos are broken. The cause is that dtype parameterization which is kind of hidden in test_foreach's parity check

janeyx99 · 2024-05-15T15:52:53Z

yes, they are real failures but not that opinfos are broken. The cause is that dtype parameterization which is kind of hidden in test_foreach's parity check

Could you elaborate what you mean by this? Or will you be pushing a fix for this?

crcrpar · 2024-05-16T02:03:54Z

yes, they are real failures but not that opinfos are broken. The cause is that dtype parameterization which is kind of hidden in test_foreach's parity check

Could you elaborate what you mean by this? Or will you be pushing a fix for this?

pushed a fix🙂

janeyx99 · 2024-05-16T15:09:42Z

torch/testing/_internal/common_methods_invocations.py

        for num_tensors, ord, out_dtype in product(
            num_input_tensors,
            (0, 1, 2, -1, -2, float('inf'), float('-inf')),
-            [None] + get_all_dtypes(),
+            (None,) + (torch.complex128,) if dtype in complex_types() else (torch.float64,),


what coverage does this remove?

integers, complex64, float32, float16, and bfloat16.
integers wouldn't be supported anyway, i.e., test_foreach's test_parity checks the error messages are identical.
some lower prec's could cause no direct cast or something

I'd rather still test more + just have expected skips vs silently testing less

I found it not really straightforward to give skips/xfail to some of the sample inputs generated which I'm speculating test_meta needs (thought the failures would be that broken opinfo ilk).
Would you happen to have some reference?

https://github.com/pytorch/pytorch/blob/95b2766864a02cdb6bc49b97a7f7e859b7167e70/torch/testing/_internal/common_methods_invocations.py#L9481-L9507

something like this but for norm

I'm sure my previous response is misleading. DecorateInfo to cover all the dtypes here would void the valuee of tests. The dtypes with if-else here doesn’t eliminate any of the existing test cases as it’s aimed to test the new kwarg. Inappropriate dtype to the kwarg just leads to an error, and unfortunately for meta test, all dtypes (meaning parameterization of inputs) end up being decorated as xfail if the dtype kwarg parameterization covers all the possible ones

Ah this parametrizes out_dtype, which is new.

crcrpar · 2024-05-20T06:01:50Z

@pytorchbot rebase

pytorchmergebot · 2024-05-20T06:03:31Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

pytorchmergebot · 2024-05-20T06:03:37Z

Successfully rebased crpa/foreach-norm-with-dtype-kwarg onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout crpa/foreach-norm-with-dtype-kwarg && git pull --rebase)

janeyx99 · 2024-05-22T14:32:55Z

torch/testing/_internal/common_methods_invocations.py

        for num_tensors, ord, out_dtype in product(
            num_input_tensors,
            (0, 1, 2, -1, -2, float('inf'), float('-inf')),
-            [None] + get_all_dtypes(),
+            (None,) + (torch.complex128,) if dtype in complex_types() else (torch.float64,),


Ah this parametrizes out_dtype, which is new.

janeyx99 · 2024-05-22T14:33:13Z

@pytorchbot merge

pytorchmergebot · 2024-05-22T14:35:18Z

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

pytorchmergebot · 2024-05-22T17:05:08Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / linux-focal-rocm6.1-py3.8 / test (distributed, 1, 1, linux.rocm.gpu)

Details for Dev Infra team

Raised by workflow job

janeyx99 · 2024-05-22T17:07:49Z

@pytorchbot merge -i

ROCm dist failures seem not related.

pytorchmergebot · 2024-05-22T17:10:36Z

Merge started

Your change will be merged while ignoring the following 1 checks: trunk / linux-focal-rocm6.1-py3.8 / test (distributed, 1, 1, linux.rocm.gpu)

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

Fixes pytorch#125040 Pull Request resolved: pytorch#125665 Approved by: https://github.com/janeyx99

crcrpar · 2024-05-23T16:41:46Z

is this still being checked...? The checks look okay

janeyx99 · 2024-05-23T16:57:51Z

@pytorchbot merge -i

pytorchmergebot · 2024-05-23T16:59:32Z

Merge started

Your change will be merged while ignoring the following 1 checks: trunk / linux-focal-rocm6.1-py3.8 / test (distributed, 1, 1, linux.rocm.gpu)

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

pytorchmergebot · 2024-05-23T16:59:55Z

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch commit --author="Masaki Kozuki <mkozuki@nvidia.com>" -m Support dtypekwarg in_foreach_norm` (#125665)

Fixes #125040

Pull Request resolved: #125665
Approved by: https://github.com/janeyx99
` returned non-zero exit code 1

On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean

Details for Dev Infra team

Raised by workflow job

janeyx99 · 2024-05-24T13:37:16Z

Ah this was merged but just not closed on Wednesday around 4pm. Closing as this commit has been merged.

pytorch-bot bot added the release notes: foreach_frontend release notes category label May 7, 2024

This comment was marked as outdated.

Sign in to view

pytorchbot added the open source label May 7, 2024

crcrpar force-pushed the crpa/foreach-norm-with-dtype-kwarg branch from 9d0bb68 to ae98aff Compare May 13, 2024 13:30

crcrpar marked this pull request as ready for review May 13, 2024 13:31

crcrpar requested review from eqy, mruberry, albanD and soulitzer as code owners May 13, 2024 13:31

albanD requested review from janeyx99 and removed request for albanD May 13, 2024 22:21

janeyx99 reviewed May 14, 2024

View reviewed changes

ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 14, 2024

janeyx99 reviewed May 16, 2024

View reviewed changes

crcrpar changed the title ~~Support dypte kwarg in _foreach_norm~~ Support dtype kwarg in _foreach_norm May 16, 2024

crcrpar added 8 commits May 20, 2024 06:03

add dtype to _foreach_norm args

d407312

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

propagate dtype to aten impl in slow path

50ae10b

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

nest dispatch to support the case where dtype is specified

c911ef1

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

use out dtype as result dtype

eb4a7c9

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

product of num tensors, lp norm type, and dtype incl. None

4d060e2

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

fewer dtypes as dtype kwarg

997d7b4

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

foreach_norm specific meta

d8b258a

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

remove redundant parentheses

1e300d6

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>

pytorchmergebot force-pushed the crpa/foreach-norm-with-dtype-kwarg branch from 546421e to 1e300d6 Compare May 20, 2024 06:03

janeyx99 approved these changes May 22, 2024

View reviewed changes

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

pytorchmergebot added the merging label May 22, 2024

pytorchmergebot removed the merging label May 22, 2024

pytorchmergebot added the merging label May 22, 2024

pytorchmergebot added Merged and removed merging labels May 22, 2024

r-barnes pushed a commit to r-barnes/pytorch that referenced this pull request May 22, 2024

Support dtype kwarg in _foreach_norm (pytorch#125665)

0939b68

Fixes pytorch#125040 Pull Request resolved: pytorch#125665 Approved by: https://github.com/janeyx99

pytorchmergebot added the merging label May 23, 2024

pytorchmergebot removed the merging label May 23, 2024

janeyx99 closed this May 24, 2024

crcrpar deleted the crpa/foreach-norm-with-dtype-kwarg branch May 24, 2024 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `dtype` kwarg in `_foreach_norm` #125665

Support `dtype` kwarg in `_foreach_norm` #125665

crcrpar commented May 7, 2024

pytorch-bot bot commented May 7, 2024 •

edited

This comment was marked as outdated.

janeyx99 left a comment

crcrpar commented May 15, 2024

janeyx99 commented May 15, 2024

crcrpar commented May 16, 2024

janeyx99 May 16, 2024

crcrpar May 16, 2024

janeyx99 May 16, 2024

crcrpar May 16, 2024 •

edited

janeyx99 May 17, 2024

crcrpar May 18, 2024 •

edited

janeyx99 May 22, 2024

crcrpar commented May 20, 2024

pytorchmergebot commented May 20, 2024

pytorchmergebot commented May 20, 2024

janeyx99 May 22, 2024

janeyx99 commented May 22, 2024

pytorchmergebot commented May 22, 2024

pytorchmergebot commented May 22, 2024

janeyx99 commented May 22, 2024

pytorchmergebot commented May 22, 2024

crcrpar commented May 23, 2024

janeyx99 commented May 23, 2024

pytorchmergebot commented May 23, 2024

pytorchmergebot commented May 23, 2024

janeyx99 commented May 24, 2024

Support dtype kwarg in _foreach_norm #125665

Support dtype kwarg in _foreach_norm #125665

Conversation

crcrpar commented May 7, 2024

pytorch-bot bot commented May 7, 2024 • edited

🔗 Helpful Links

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

❌ 1 New Failure

This comment was marked as outdated.

janeyx99 left a comment

Choose a reason for hiding this comment

crcrpar commented May 15, 2024

janeyx99 commented May 15, 2024

crcrpar commented May 16, 2024

janeyx99 May 16, 2024

Choose a reason for hiding this comment

crcrpar May 16, 2024

Choose a reason for hiding this comment

janeyx99 May 16, 2024

Choose a reason for hiding this comment

crcrpar May 16, 2024 • edited

Choose a reason for hiding this comment

janeyx99 May 17, 2024

Choose a reason for hiding this comment

crcrpar May 18, 2024 • edited

Choose a reason for hiding this comment

janeyx99 May 22, 2024

Choose a reason for hiding this comment

crcrpar commented May 20, 2024

pytorchmergebot commented May 20, 2024

pytorchmergebot commented May 20, 2024

janeyx99 May 22, 2024

Choose a reason for hiding this comment

janeyx99 commented May 22, 2024

pytorchmergebot commented May 22, 2024

Merge started

pytorchmergebot commented May 22, 2024

Merge failed

janeyx99 commented May 22, 2024

pytorchmergebot commented May 22, 2024

Merge started

crcrpar commented May 23, 2024

janeyx99 commented May 23, 2024

pytorchmergebot commented May 23, 2024

Merge started

pytorchmergebot commented May 23, 2024

Merge failed

janeyx99 commented May 24, 2024

Support `dtype` kwarg in `_foreach_norm` #125665

Support `dtype` kwarg in `_foreach_norm` #125665

pytorch-bot bot commented May 7, 2024 •

edited

crcrpar May 16, 2024 •

edited

crcrpar May 18, 2024 •

edited