Fix and test several NJT reductions #139317

jbschlosser · 2024-10-30T20:54:12Z

Stack from ghstack (oldest at bottom):

I'm sick of reductions not working properly - spotty dim coverage, missing backwards, etc. This PR fixes quite a bit.

It applies to the following ops:

sum / mean / prod
all / any
amin / amax
min / max
argmin / argmax

The general reduction logic has been factored out into a helper _apply_reduction(func, func_name, identity_element, *args, **kwargs). The idea is that by providing a valid identity element, we can utilize conversions to padded dense when needed for reducing over the ragged dim.

Extensive test coverage includes:

reductions across ragged dim
reductions across non-batch, non-ragged dims
reductions across both batch and ragged dims
multiple dim reductions (for ops that support this)
full reduction -> scalar

Bonus: the PR includes backwards fixes for sum and mean, which have never worked.

[ghstack-poisoned]

pytorch-bot · 2024-10-30T20:54:15Z

🔗 Helpful Links

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

📄 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 30e5057 with merge base 03ec250 ():
💚 Looks good so far! There are no failures yet. 💚

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

I'm sick of reductions not working properly - spotty dim coverage, missing backwards, etc. This PR fixes quite a bit. It applies to the following ops: * `sum` / `mean` / `prod` * `all` / `any` * `amin` / `amax` * `min` / `max` * `argmin` / `argmax` The general reduction logic has been factored out into a helper `_apply_reduction(func, func_name, identity_element, *args, **kwargs)`. The idea is that by providing a valid identity element, we can utilize conversions to padded dense when needed for reducing over the ragged dim. Extensive test coverage includes: * reductions across ragged dim * reductions across non-batch, non-ragged dims * reductions across both batch and ragged dims * multiple dim reductions (for ops that support this) * full reduction -> scalar Bonus: the PR includes backwards fixes for `sum` and `mean`, which have never worked. [ghstack-poisoned]

jbschlosser · 2024-10-30T21:24:30Z

@pytorchbot merge

pytorchmergebot · 2024-10-30T21:27:04Z

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-10-30T21:58:53Z

Merge failed

Reason: 3 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

I'm sick of reductions not working properly - spotty dim coverage, missing backwards, etc. This PR fixes quite a bit. It applies to the following ops: * `sum` / `mean` / `prod` * `all` / `any` * `amin` / `amax` * `min` / `max` * `argmin` / `argmax` The general reduction logic has been factored out into a helper `_apply_reduction(func, func_name, identity_element, *args, **kwargs)`. The idea is that by providing a valid identity element, we can utilize conversions to padded dense when needed for reducing over the ragged dim. Extensive test coverage includes: * reductions across ragged dim * reductions across non-batch, non-ragged dims * reductions across both batch and ragged dims * multiple dim reductions (for ops that support this) * full reduction -> scalar Bonus: the PR includes backwards fixes for `sum` and `mean`, which have never worked. [ghstack-poisoned]

jbschlosser · 2024-10-31T14:54:30Z

@pytorchbot merge

pytorchmergebot · 2024-10-31T14:56:11Z

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-10-31T15:17:23Z

Merge failed

Reason: 2 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

I'm sick of reductions not working properly - spotty dim coverage, missing backwards, etc. This PR fixes quite a bit. It applies to the following ops: * `sum` / `mean` / `prod` * `all` / `any` * `amin` / `amax` * `min` / `max` * `argmin` / `argmax` The general reduction logic has been factored out into a helper `_apply_reduction(func, func_name, identity_element, *args, **kwargs)`. The idea is that by providing a valid identity element, we can utilize conversions to padded dense when needed for reducing over the ragged dim. Extensive test coverage includes: * reductions across ragged dim * reductions across non-batch, non-ragged dims * reductions across both batch and ragged dims * multiple dim reductions (for ops that support this) * full reduction -> scalar Bonus: the PR includes backwards fixes for `sum` and `mean`, which have never worked. [ghstack-poisoned]

jbschlosser · 2024-10-31T16:15:50Z

@pytorchbot merge

pytorchmergebot · 2024-10-31T16:17:40Z

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-10-31T16:33:45Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / macos-py3-arm64 / test (default, 2, 3, macos-m1-stable)

Details for Dev Infra team

Raised by workflow job

I'm sick of reductions not working properly - spotty dim coverage, missing backwards, etc. This PR fixes quite a bit. It applies to the following ops: * `sum` / `mean` / `prod` * `all` / `any` * `amin` / `amax` * `min` / `max` * `argmin` / `argmax` The general reduction logic has been factored out into a helper `_apply_reduction(func, func_name, identity_element, *args, **kwargs)`. The idea is that by providing a valid identity element, we can utilize conversions to padded dense when needed for reducing over the ragged dim. Extensive test coverage includes: * reductions across ragged dim * reductions across non-batch, non-ragged dims * reductions across both batch and ragged dims * multiple dim reductions (for ops that support this) * full reduction -> scalar Bonus: the PR includes backwards fixes for `sum` and `mean`, which have never worked. [ghstack-poisoned]

jbschlosser · 2024-10-31T17:19:32Z

@pytorchbot merge

pytorchmergebot · 2024-10-31T17:21: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

pytorchmergebot · 2024-10-31T17:42:27Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Lint / lintrunner-noclang / linux-job

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

I'm sick of reductions not working properly - spotty dim coverage, missing backwards, etc. This PR fixes quite a bit. It applies to the following ops: * `sum` / `mean` / `prod` * `all` / `any` * `amin` / `amax` * `min` / `max` * `argmin` / `argmax` The general reduction logic has been factored out into a helper `_apply_reduction(func, func_name, identity_element, *args, **kwargs)`. The idea is that by providing a valid identity element, we can utilize conversions to padded dense when needed for reducing over the ragged dim. Extensive test coverage includes: * reductions across ragged dim * reductions across non-batch, non-ragged dims * reductions across both batch and ragged dims * multiple dim reductions (for ops that support this) * full reduction -> scalar Bonus: the PR includes backwards fixes for `sum` and `mean`, which have never worked. [ghstack-poisoned]

jbschlosser · 2024-10-31T17:59:32Z

@pytorchbot merge

pytorchmergebot · 2024-10-31T18:01:22Z

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

I'm sick of reductions not working properly - spotty dim coverage, missing backwards, etc. This PR fixes quite a bit. It applies to the following ops: * `sum` / `mean` / `prod` * `all` / `any` * `amin` / `amax` * `min` / `max` * `argmin` / `argmax` The general reduction logic has been factored out into a helper `_apply_reduction(func, func_name, identity_element, *args, **kwargs)`. The idea is that by providing a valid identity element, we can utilize conversions to padded dense when needed for reducing over the ragged dim. Extensive test coverage includes: * reductions across ragged dim * reductions across non-batch, non-ragged dims * reductions across both batch and ragged dims * multiple dim reductions (for ops that support this) * full reduction -> scalar Bonus: the PR includes backwards fixes for `sum` and `mean`, which have never worked. Pull Request resolved: pytorch#139317 Approved by: https://github.com/cpuhrsch

ghstack-source-id: 1e6bfa0 Pull Request resolved: pytorch/pytorch#139317

Fix and test several NJT reductions

a33a260

[ghstack-poisoned]

jbschlosser requested review from albanD and soulitzer as code owners October 30, 2024 20:54

This was referenced Oct 29, 2024

Support narrow() on batch dim for NJT #136444

Closed

Support view_as() on NJT; allow nested int swapping #139196

Closed

jbschlosser requested review from cpuhrsch and removed request for albanD October 30, 2024 20:54

jbschlosser added topic: improvements topic category release notes: nested tensor Changes that have a direct impact on nested tensors labels Oct 30, 2024

cpuhrsch approved these changes Oct 30, 2024

View reviewed changes

jbschlosser mentioned this pull request Oct 30, 2024

Add mean and var operation for Nested Tensors #138831

Open

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

pytorchmergebot added the merging label Oct 30, 2024

pytorchmergebot removed the merging label Oct 30, 2024

pytorchmergebot added the merging label Oct 31, 2024

pytorchmergebot removed the merging label Oct 31, 2024

pytorchmergebot added the merging label Oct 31, 2024

pytorchmergebot removed the merging label Oct 31, 2024

pytorchmergebot added the merging label Oct 31, 2024

pytorchmergebot removed the merging label Oct 31, 2024

pytorchmergebot added the merging label Oct 31, 2024

pytorchmergebot added the Merged label Oct 31, 2024

pytorchmergebot closed this in ddb291a Oct 31, 2024

pytorchmergebot removed the merging label Oct 31, 2024

github-actions bot deleted the gh/jbschlosser/196/head branch December 1, 2024 02:22

desai0007 pushed a commit to desai0007/test-repo-pytorch that referenced this pull request Feb 26, 2025

Fix and test several NJT reductions

6267656

ghstack-source-id: 1e6bfa0 Pull Request resolved: pytorch/pytorch#139317

Fix and test several NJT reductions #139317

Fix and test several NJT reductions #139317

Uh oh!

Conversation

jbschlosser commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Oct 30, 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/139317

✅ No Failures

Uh oh!

jbschlosser commented Oct 30, 2024

Uh oh!

pytorchmergebot commented Oct 30, 2024

Merge started

Uh oh!

pytorchmergebot commented Oct 30, 2024

Merge failed

Uh oh!

jbschlosser commented Oct 31, 2024

Uh oh!

pytorchmergebot commented Oct 31, 2024

Merge started

Uh oh!

pytorchmergebot commented Oct 31, 2024

Merge failed

Uh oh!

jbschlosser commented Oct 31, 2024

Uh oh!

pytorchmergebot commented Oct 31, 2024

Merge started

Uh oh!

pytorchmergebot commented Oct 31, 2024

Merge failed

Uh oh!

jbschlosser commented Oct 31, 2024

Uh oh!

pytorchmergebot commented Oct 31, 2024

Merge started

Uh oh!

pytorchmergebot commented Oct 31, 2024

Merge failed

Uh oh!

jbschlosser commented Oct 31, 2024

Uh oh!

pytorchmergebot commented Oct 31, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jbschlosser commented Oct 30, 2024 •

edited

Loading

pytorch-bot bot commented Oct 30, 2024 •

edited

Loading