Skip to content

Conversation

@yipjustin
Copy link
Contributor

@yipjustin yipjustin commented Nov 25, 2024

Stack from ghstack (oldest at bottom):

The existing weight tensor for aten.embedding is created using a tensor_like from the output tensor, which defaults to channel packed.

However, the weight tensor is actually a 2D-tensor of (num_embedding, dim_of_embedding). It is better in space to use either width or height packing.

This diff changes the implementation to use height-packing.

Differential Revision: D66421366

The existing weight tensor for aten.embedding is created using a `tensor_like` from the output tensor, which defaults to channel packed.

However, the weight tensor is actually a 2D-tensor of `(num_embedding, dim_of_embedding)`. It is better in space to use either width or height packing.

This diff changes the implementation to use height-packing.

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

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 25, 2024

🔗 Helpful Links

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

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

✅ No Failures

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

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 25, 2024
yipjustin added a commit that referenced this pull request Nov 25, 2024
The existing weight tensor for aten.embedding is created using a `tensor_like` from the output tensor, which defaults to channel packed.

However, the weight tensor is actually a 2D-tensor of `(num_embedding, dim_of_embedding)`. It is better in space to use either width or height packing.

This diff changes the implementation to use height-packing.

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

ghstack-source-id: 255375459
Pull Request resolved: #7063
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66421366

The existing weight tensor for aten.embedding is created using a `tensor_like` from the output tensor, which defaults to channel packed.

However, the weight tensor is actually a 2D-tensor of `(num_embedding, dim_of_embedding)`. It is better in space to use either width or height packing.

This diff changes the implementation to use height-packing.

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

[ghstack-poisoned]
yipjustin added a commit that referenced this pull request Nov 26, 2024
Pull Request resolved: #7063

The existing weight tensor for aten.embedding is created using a `tensor_like` from the output tensor, which defaults to channel packed.

However, the weight tensor is actually a 2D-tensor of `(num_embedding, dim_of_embedding)`. It is better in space to use either width or height packing.

This diff changes the implementation to use height-packing.
ghstack-source-id: 255439082

Differential Revision: [D66421366](https://our.internmc.facebook.com/intern/diff/D66421366/)
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66421366

@facebook-github-bot facebook-github-bot merged commit 0e4b1a1 into gh/yipjustin/25/base Nov 26, 2024
41 checks passed
@facebook-github-bot facebook-github-bot deleted the gh/yipjustin/25/head branch November 26, 2024 05:01
kirklandsign added a commit that referenced this pull request Nov 26, 2024
Pull Request resolved: #7063

The existing weight tensor for aten.embedding is created using a `tensor_like` from the output tensor, which defaults to channel packed.

However, the weight tensor is actually a 2D-tensor of `(num_embedding, dim_of_embedding)`. It is better in space to use either width or height packing.

This diff changes the implementation to use height-packing.
ghstack-source-id: 255439082

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

Co-authored-by: Justin Yip <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants