KEMBAR78
Propagate NJT lengths through op calls by jbschlosser · Pull Request #138098 · pytorch/pytorch · GitHub
Skip to content

Conversation

@jbschlosser
Copy link
Contributor

@jbschlosser jbschlosser commented Oct 16, 2024

Stack from ghstack (oldest at bottom):

NJTs allow for specifying both offsets and lengths non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, lengths is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the lengths metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 16, 2024

🔗 Helpful Links

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

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

❌ 20 New Failures

As of commit a2ccb34 with merge base a2bc2e3 (image):

NEW FAILURES - The following jobs have failed:

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

NJTs allow for specifying both `offsets` and `lengths` non-redundantly to specify ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to specify ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to specify ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Oct 16, 2024
ghstack-source-id: 02ab399
Pull Request resolved: #138098
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to specify ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Oct 17, 2024
ghstack-source-id: 6944cdd
Pull Request resolved: #138098
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Oct 17, 2024
ghstack-source-id: 52b1b27
Pull Request resolved: #138098
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Oct 24, 2024
ghstack-source-id: ec6eabd
Pull Request resolved: #138098
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Oct 25, 2024
ghstack-source-id: f7d35fb
Pull Request resolved: #138098
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Nov 5, 2024
ghstack-source-id: 786696e
Pull Request resolved: #138098
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
NJTs allow for specifying both `offsets` and `lengths` non-redundantly to describe ragged structure that is "non-contiguous with holes". i.e. the offsets point to the beginning of each sequence and the corresponding length may not fully span the region to the next offset.

However, `lengths` is not appropriately propagated today through op calls. For example, calling a unary op on an NJT that is non-contiguous with holes will silently drop the `lengths` metadata, which is incorrect. This PR fixes this.

As an analogue to dense tensors, running a unary op on a non-contiguous dense tensor outputs a dense tensor with the same strides.

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Nov 6, 2024
ghstack-source-id: 7f78cfd
Pull Request resolved: #138098
@jbschlosser
Copy link
Contributor Author

Closing in favor of #140160.

@jbschlosser jbschlosser closed this Nov 8, 2024
jbschlosser added a commit that referenced this pull request Nov 8, 2024
This PR contains several fixes related to non-contiguous NJTs:
1. Propagates `lengths` through op calls appropriately (see desc of #138098)
    * SDPA now calls `nested_view_from_values_offsets_lengths()` instead of `nested_view_from_values_offsets()`
2. Allows non-contig NJTs in unsqueeze / transpose / select
3. Expands padded dense -> NJT conversion to support non-contig NJTs
4. (unrelated sorry) Updates `split` / `split_with_sizes` to allow for optional `dim`, matching the ATen signature

[ghstack-poisoned]
jbschlosser added a commit that referenced this pull request Nov 8, 2024
This PR contains several fixes related to non-contiguous NJTs:
1. Propagates `lengths` through op calls appropriately (see desc of #138098)
    * SDPA now calls `nested_view_from_values_offsets_lengths()` instead of `nested_view_from_values_offsets()`
2. Allows non-contig NJTs in unsqueeze / transpose / select
3. Expands padded dense -> NJT conversion to support non-contig NJTs
4. (unrelated sorry) Updates `split` / `split_with_sizes` to allow for optional `dim`, matching the ATen signature

[ghstack-poisoned]
pytorchmergebot pushed a commit that referenced this pull request Nov 9, 2024
This PR contains several fixes related to non-contiguous NJTs:
1. Propagates `lengths` through op calls appropriately (see desc of #138098)
    * SDPA now calls `nested_view_from_values_offsets_lengths()` instead of `nested_view_from_values_offsets()`
2. Allows non-contig NJTs in unsqueeze / transpose / select
3. Expands padded dense -> NJT conversion to support non-contig NJTs
4. (unrelated sorry) Updates `split` / `split_with_sizes` to allow for optional `dim`, matching the ATen signature
Pull Request resolved: #140160
Approved by: https://github.com/cpuhrsch
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
This PR contains several fixes related to non-contiguous NJTs:
1. Propagates `lengths` through op calls appropriately (see desc of pytorch#138098)
    * SDPA now calls `nested_view_from_values_offsets_lengths()` instead of `nested_view_from_values_offsets()`
2. Allows non-contig NJTs in unsqueeze / transpose / select
3. Expands padded dense -> NJT conversion to support non-contig NJTs
4. (unrelated sorry) Updates `split` / `split_with_sizes` to allow for optional `dim`, matching the ATen signature
Pull Request resolved: pytorch#140160
Approved by: https://github.com/cpuhrsch
@github-actions github-actions bot deleted the gh/jbschlosser/188/head branch December 9, 2024 02:14
fmo-mt pushed a commit to fmo-mt/pytorch that referenced this pull request Dec 11, 2024
This PR contains several fixes related to non-contiguous NJTs:
1. Propagates `lengths` through op calls appropriately (see desc of pytorch#138098)
    * SDPA now calls `nested_view_from_values_offsets_lengths()` instead of `nested_view_from_values_offsets()`
2. Allows non-contig NJTs in unsqueeze / transpose / select
3. Expands padded dense -> NJT conversion to support non-contig NJTs
4. (unrelated sorry) Updates `split` / `split_with_sizes` to allow for optional `dim`, matching the ATen signature
Pull Request resolved: pytorch#140160
Approved by: https://github.com/cpuhrsch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant