Skip to content

convert stride_per_key_per_rank to tensor inside KJT #2959

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 1 commit into
base: main
Choose a base branch
from

Conversation

TroyGarden
Copy link
Contributor

Summary:

context

  • this diff is part of the "variable-batch KJT refactoring" project (doc)
  • previously the stride_per_key_per_rank variable is List[List[int]] | None which can't be handled correctly in PT2 IR (torch.export)
  • this change makes the KJT class variable _stride_per_key_per_rank as torch.IntTensor | None so it would be compatible with PT2 IR.

equivalency

  • to check if self._stride_per_key_per_rank is None
    this logic is used to differentiate variable_batch case, and should have the same behavior after this diff
  • to use self._stride_per_key_per_rank as List[List[int]]
    most of the callsite use the function to get the list: def stride_per_key_per_rank(self) -> List[List[int]]:, and this function is modified to covert the torch.IntTensor to list as _stride_per_key_per_rank.tolist(), the results should be the same

NOTE: this self. _stride_per_key_per_rank.tolist() tensor should always be on CPU since it's effective the meta data of a KJT. For generic torch APIs like .to(...), record_stream(), etc. should in general avoid altering this variable.

Differential Revision: D74366343

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

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

TroyGarden added a commit to TroyGarden/torchrec that referenced this pull request May 8, 2025
Summary:

# context
* this diff is part of the "variable-batch KJT refactoring" project ([doc](https://fburl.com/gdoc/svfysfai))
* previously the `stride_per_key_per_rank` variable is `List[List[int]] | None` which can't be handled correctly in PT2 IR (torch.export)
* this change makes the KJT class variable `_stride_per_key_per_rank` as `torch.IntTensor | None` so it would be compatible with PT2 IR.

# equivalency
* to check if `self._stride_per_key_per_rank` is `None`
this logic is used to differentiate variable_batch case, and should have the same behavior after this diff
* to use `self._stride_per_key_per_rank` as `List[List[int]]`
most of the callsite use the function to get the list: `def stride_per_key_per_rank(self) -> List[List[int]]:`, and this function is modified to covert the `torch.IntTensor` to list as ` _stride_per_key_per_rank.tolist()`, the results should be the same

NOTE: this `self. _stride_per_key_per_rank.tolist()` tensor should always be on CPU since it's effective the meta data of a KJT. For generic torch APIs like `.to(...)`, `record_stream()`, etc. should in general avoid altering this variable.

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

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

TroyGarden added a commit to TroyGarden/torchrec that referenced this pull request May 8, 2025
Summary:

# context
* this diff is part of the "variable-batch KJT refactoring" project ([doc](https://fburl.com/gdoc/svfysfai))
* previously the `stride_per_key_per_rank` variable is `List[List[int]] | None` which can't be handled correctly in PT2 IR (torch.export)
* this change makes the KJT class variable `_stride_per_key_per_rank` as `torch.IntTensor | None` so it would be compatible with PT2 IR.

# equivalency
* to check if `self._stride_per_key_per_rank` is `None`
this logic is used to differentiate variable_batch case, and should have the same behavior after this diff
* to use `self._stride_per_key_per_rank` as `List[List[int]]`
most of the callsite use the function to get the list: `def stride_per_key_per_rank(self) -> List[List[int]]:`, and this function is modified to covert the `torch.IntTensor` to list as ` _stride_per_key_per_rank.tolist()`, the results should be the same

NOTE: this `self. _stride_per_key_per_rank.tolist()` tensor should always be on CPU since it's effective the meta data of a KJT. For generic torch APIs like `.to(...)`, `record_stream()`, etc. should in general avoid altering this variable.

Reviewed By: jd7-tr

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

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

Summary:

# context
* this diff is part of the "variable-batch KJT refactoring" project ([doc](https://fburl.com/gdoc/svfysfai))
* previously the `stride_per_key_per_rank` variable is `List[List[int]] | None` which can't be handled correctly in PT2 IR (torch.export)
* this change makes the KJT class variable `_stride_per_key_per_rank` as `torch.IntTensor | None` so it would be compatible with PT2 IR.

# equivalency
* to check if `self._stride_per_key_per_rank` is `None`
this logic is used to differentiate variable_batch case, and should have the same behavior after this diff
* to use `self._stride_per_key_per_rank` as `List[List[int]]`
most of the callsite use the function to get the list: `def stride_per_key_per_rank(self) -> List[List[int]]:`, and this function is modified to covert the `torch.IntTensor` to list as ` _stride_per_key_per_rank.tolist()`, the results should be the same

NOTE: this `self. _stride_per_key_per_rank.tolist()` tensor should always be on CPU since it's effective the meta data of a KJT. For generic torch APIs like `.to(...)`, `record_stream()`, etc. should in general avoid altering this variable.

Reviewed By: jd7-tr

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

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

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