Skip to content

Update the _maybe_compute_stride_kjt logic to calculate stride based off of inverse_indices for VBE KJTs. #2925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jd7-tr
Copy link
Contributor

@jd7-tr jd7-tr commented Apr 29, 2025

Summary:
See D73051959 for context.

Update the _maybe_compute_stride_kjt logic to calculate stride based off of inverse_indices for VBE KJTs.

Currently, stride of VBE KJT with stride_per_key_per_rank is calculated as the max "stride per key". This is different from the batch size of the EBC output KeyedTensor which is based off of inverse_indices. This causes issues in IR module serialization: [debug doc].(https://docs.google.com/document/d/1yQhI484cgVloSqIBPAeTQhzfb3ltjvMRiLaQDceHGOU/edit?tab=t.0#heading=h.c66chahhl8df).

Differential Revision: D73824764

@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 Apr 29, 2025
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

…ic (pytorch#2903)

Summary:
Pull Request resolved: pytorch#2903

# Context
* Currently torchrec IR serializer can't handle variable batch use case.
* `torch.export` only captures the keys, values, lengths, weights, offsets of a KJT, however, some variable-batch related parameters like `stride_per_rank` or `inverse_indices` would be ignored.
* This test case (expected failure right now) covers the vb-KJT scenario for verifying that the serialize_deserialize_ebc use case works fine with KJTs with variable batch size.

# Ref

Differential Revision: D73051959
jd7-tr added a commit to jd7-tr/torchrec that referenced this pull request May 1, 2025
…off of inverse_indices for VBE KJTs. (pytorch#2925)

Summary:

See D73051959 for context.

Update the `_maybe_compute_stride_kjt` logic to calculate stride based off of `inverse_indices` for VBE KJTs.

Currently, stride of VBE KJT with `stride_per_key_per_rank` is calculated as the max "stride per key". This is different from the batch size of the EBC output KeyedTensor which is based off of `inverse_indices`. This causes issues in IR module serialization: [debug doc](https://docs.google.com/document/d/1yQhI484cgVloSqIBPAeTQhzfb3ltjvMRiLaQDceHGOU/edit?tab=t.0#heading=h.c66chahhl8df).

Differential Revision: D73824764
@jd7-tr jd7-tr force-pushed the export-D73824764 branch from 9e3d5e6 to 932b4ef Compare May 1, 2025 00:36
…off of inverse_indices for VBE KJTs. (pytorch#2925)

Summary:
Pull Request resolved: pytorch#2925

See D73051959 for context.

Update the `_maybe_compute_stride_kjt` logic to calculate stride based off of `inverse_indices` for VBE KJTs.

Currently, stride of VBE KJT with `stride_per_key_per_rank` is calculated as the max "stride per key". This is different from the batch size of the EBC output KeyedTensor which is based off of `inverse_indices`. This causes issues in IR module serialization: [debug doc](https://docs.google.com/document/d/1yQhI484cgVloSqIBPAeTQhzfb3ltjvMRiLaQDceHGOU/edit?tab=t.0#heading=h.c66chahhl8df).

Differential Revision: D73824764
@facebook-github-bot
Copy link
Contributor

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

@jd7-tr jd7-tr force-pushed the export-D73824764 branch from 932b4ef to da753b5 Compare May 1, 2025 00:40
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.

2 participants