Skip to content

Conversation

davidberard98
Copy link
Contributor

@davidberard98 davidberard98 commented Jan 25, 2024

Stack from ghstack (oldest at bottom):

Uses case: _unsafe_view is used in aot_autograd to create a view that doesn't register as a view:

if CompiledFunction.metadata.num_unsafe_view_outputs > 0:
for idx in CompiledFunction.metadata.unsafe_view_out_indices:
raw_return_idx = num_mutated_runtime_inps + idx
o = raw_returns[raw_return_idx]
raw_returns[raw_return_idx] = torch.ops.aten._unsafe_view(
o, o.shape
)

If a transposed nested tensor (i.e. NT with ragged_idx != 1) encounters this code path, it previously would fail for two reasons: 1) because _unsafe_view isn't registered, and 2) because ragged_idx != 1 is not supported. This PR adds support for _unsafe_view (completely reusing the implementation of view; this just registers _unsafe_view as another op using the same implementation). It also adds support for ragged_idx != 1, but only for trivial cases where inp._size == size (the use case used by aot_autograd).

Tests: verify that the result of _unsafe_view doesn't have a _base, and that simple views on transposed NTs work.

Differential Revision: D53096814

…view

Uses case: `_unsafe_view` is used in aot_autograd to create a view that doesn't register as a view:

https://github.com/pytorch/pytorch/blob/eebe7e1d37f1baa995c694d540cc2fc98884fa18/torch/_functorch/_aot_autograd/jit_compile_runtime_wrappers.py#L470-L476

If a transposed nested tensor (i.e. NT with ragged_idx != 1) encounters this code path, it previously would fail for two reasons: 1) because `_unsafe_view` isn't registered, and 2) because ragged_idx != 1 is not supported. This PR adds support for `_unsafe_view` (completely reusing the implementation of `view`; this just registers `_unsafe_view` as another op using the same implementation). It also adds support for ragged_idx != 1, but only for trivial cases where inp._size == size (the use case used by aot_autograd).

Tests: verify that the result of `_unsafe_view` doesn't have a `_base`, and that simple views on transposed NTs work.

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Jan 25, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 8428891 with merge base 3cdd4e2 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@davidberard98
Copy link
Contributor Author

@davidberard98 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…nd _unsafe_view"

Uses case: `_unsafe_view` is used in aot_autograd to create a view that doesn't register as a view:

https://github.com/pytorch/pytorch/blob/eebe7e1d37f1baa995c694d540cc2fc98884fa18/torch/_functorch/_aot_autograd/jit_compile_runtime_wrappers.py#L470-L476

If a transposed nested tensor (i.e. NT with ragged_idx != 1) encounters this code path, it previously would fail for two reasons: 1) because `_unsafe_view` isn't registered, and 2) because ragged_idx != 1 is not supported. This PR adds support for `_unsafe_view` (completely reusing the implementation of `view`; this just registers `_unsafe_view` as another op using the same implementation). It also adds support for ragged_idx != 1, but only for trivial cases where inp._size == size (the use case used by aot_autograd).

Tests: verify that the result of `_unsafe_view` doesn't have a `_base`, and that simple views on transposed NTs work.

Differential Revision: [D53096814](https://our.internmc.facebook.com/intern/diff/D53096814)

[ghstack-poisoned]
davidberard98 added a commit that referenced this pull request Jan 25, 2024
…view

Uses case: `_unsafe_view` is used in aot_autograd to create a view that doesn't register as a view:

https://github.com/pytorch/pytorch/blob/eebe7e1d37f1baa995c694d540cc2fc98884fa18/torch/_functorch/_aot_autograd/jit_compile_runtime_wrappers.py#L470-L476

If a transposed nested tensor (i.e. NT with ragged_idx != 1) encounters this code path, it previously would fail for two reasons: 1) because `_unsafe_view` isn't registered, and 2) because ragged_idx != 1 is not supported. This PR adds support for `_unsafe_view` (completely reusing the implementation of `view`; this just registers `_unsafe_view` as another op using the same implementation). It also adds support for ragged_idx != 1, but only for trivial cases where inp._size == size (the use case used by aot_autograd).

Tests: verify that the result of `_unsafe_view` doesn't have a `_base`, and that simple views on transposed NTs work.

ghstack-source-id: 7c15f5b
Pull Request resolved: #118317
@davidberard98 davidberard98 changed the title [Nested Tensor] view: simple support for ragged_idx != 1 and _unsafe_view [Nested Tensor] view: basic support for ragged_idx != 1 and _unsafe_view Jan 25, 2024
@davidberard98 davidberard98 added topic: not user facing topic category release notes: jit release notes category labels Jan 25, 2024
@davidberard98 davidberard98 marked this pull request as ready for review January 25, 2024 23:56
Copy link
Contributor

@soulitzer soulitzer left a comment

Choose a reason for hiding this comment

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

SGTM

@davidberard98
Copy link
Contributor Author

@pytorchbot merge -r

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

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

…d _unsafe_view"

Uses case: `_unsafe_view` is used in aot_autograd to create a view that doesn't register as a view:

https://github.com/pytorch/pytorch/blob/eebe7e1d37f1baa995c694d540cc2fc98884fa18/torch/_functorch/_aot_autograd/jit_compile_runtime_wrappers.py#L470-L476

If a transposed nested tensor (i.e. NT with ragged_idx != 1) encounters this code path, it previously would fail for two reasons: 1) because `_unsafe_view` isn't registered, and 2) because ragged_idx != 1 is not supported. This PR adds support for `_unsafe_view` (completely reusing the implementation of `view`; this just registers `_unsafe_view` as another op using the same implementation). It also adds support for ragged_idx != 1, but only for trivial cases where inp._size == size (the use case used by aot_autograd).

Tests: verify that the result of `_unsafe_view` doesn't have a `_base`, and that simple views on transposed NTs work.

Differential Revision: [D53096814](https://our.internmc.facebook.com/intern/diff/D53096814)

[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Successfully rebased gh/davidberard98/254/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/118317)

pytorchmergebot pushed a commit that referenced this pull request Jan 26, 2024
…view

Uses case: `_unsafe_view` is used in aot_autograd to create a view that doesn't register as a view:

https://github.com/pytorch/pytorch/blob/eebe7e1d37f1baa995c694d540cc2fc98884fa18/torch/_functorch/_aot_autograd/jit_compile_runtime_wrappers.py#L470-L476

If a transposed nested tensor (i.e. NT with ragged_idx != 1) encounters this code path, it previously would fail for two reasons: 1) because `_unsafe_view` isn't registered, and 2) because ragged_idx != 1 is not supported. This PR adds support for `_unsafe_view` (completely reusing the implementation of `view`; this just registers `_unsafe_view` as another op using the same implementation). It also adds support for ragged_idx != 1, but only for trivial cases where inp._size == size (the use case used by aot_autograd).

Tests: verify that the result of `_unsafe_view` doesn't have a `_base`, and that simple views on transposed NTs work.

ghstack-source-id: 1e0b057
Pull Request resolved: #118317
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR has internal changes and must be landed via Phabricator

Details for Dev Infra team Raised by workflow job

@davidberard98
Copy link
Contributor Author

@davidberard98 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@davidberard98
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR has internal changes and must be landed via Phabricator

Details for Dev Infra team Raised by workflow job

@davidberard98
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

@facebook-github-bot facebook-github-bot deleted the gh/davidberard98/254/head branch January 30, 2024 15:22
jeffdaily pushed a commit to ROCm/pytorch that referenced this pull request Feb 8, 2024
…iew (pytorch#118317)

Uses case: `_unsafe_view` is used in aot_autograd to create a view that doesn't register as a view:

https://github.com/pytorch/pytorch/blob/eebe7e1d37f1baa995c694d540cc2fc98884fa18/torch/_functorch/_aot_autograd/jit_compile_runtime_wrappers.py#L470-L476

If a transposed nested tensor (i.e. NT with ragged_idx != 1) encounters this code path, it previously would fail for two reasons: 1) because `_unsafe_view` isn't registered, and 2) because ragged_idx != 1 is not supported. This PR adds support for `_unsafe_view` (completely reusing the implementation of `view`; this just registers `_unsafe_view` as another op using the same implementation). It also adds support for ragged_idx != 1, but only for trivial cases where inp._size == size (the use case used by aot_autograd).

Tests: verify that the result of `_unsafe_view` doesn't have a `_base`, and that simple views on transposed NTs work.

Differential Revision: [D53096814](https://our.internmc.facebook.com/intern/diff/D53096814)
Pull Request resolved: pytorch#118317
Approved by: https://github.com/soulitzer
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 release notes: jit release notes category topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants