Skip to content

Conversation

jbschlosser
Copy link
Contributor

@jbschlosser jbschlosser commented Jan 10, 2024

Stack from ghstack (oldest at bottom):

Provides symbolic C++-side reshape_as() / reshape() decomps for jagged layout NTs to make the backwards pass work.

Copy link

pytorch-bot bot commented Jan 10, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 34502fc with merge base 81b7a09 (image):
💚 Looks good so far! There are no failures yet. 💚

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

if func is torch._C._nn.scaled_dot_product_attention:
return jagged_scaled_dot_product_attention(*args, **kwargs)

# Handle reshape() / reshape_as() here because they're CompositeImplicit.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: Handling these in __torch_function__() this isn't enough to get them working for autograd formulas.

@jbschlosser
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 10, 2024
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

Provides symbolic C++-side `reshape_as()` / `reshape()` decomps for jagged layout NTs to make the backwards pass work.

[ghstack-poisoned]
@jbschlosser jbschlosser added the topic: not user facing topic category label Jan 10, 2024
jbschlosser added a commit that referenced this pull request Jan 10, 2024
ghstack-source-id: de1831f
Pull Request resolved: #117137
@jbschlosser
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

return std::make_pair(M, N);
}

Tensor reshape_nested(const Tensor& self, IntArrayRef proposed_shape);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noob question. Why we do need to add this now? And it seems that reshape_nested is not used anywhere after you changed it to reshape_nested_symint in native_functions.yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing - good question! This was originally added for strided (C++) NTs, and we don't support nt.sizes() on those. To expand this to support jagged layout NTs, we need to operate in symbolic shapes land, as we always expect them to have a symbolic shape (i.e. a SingletonSymInt for the jagged dimension).

And it seems that reshape_nested is not used anywhere after you changed it to reshape_nested_symint in native_functions.yaml?

Note that reshape_nested_symint() calls the old reshape_nested() for the case of strided NT.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks. Totally missed the usage inside reshape_nested_symint.
I am wondering why do we need to define "reshape_nested" in the header file while most other functions don't? Does it make any difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering why do we need to define "reshape_nested" in the header file while most other functions don't? Does it make any difference?

The reason for this is because reshape_nested() is no longer referenced from native_functions.yaml. Functions in the dispatch table there will automatically have a header signature generated.

@facebook-github-bot facebook-github-bot deleted the gh/jbschlosser/117/head branch January 14, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants