Skip to content

Compute info_B_num_bits from T to make it a constant #3748

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

Closed
wants to merge 1 commit into from

Conversation

spcyppt
Copy link
Contributor

@spcyppt spcyppt commented Feb 28, 2025

Summary:
X-link: https://github.com/facebookresearch/FBGEMM/pull/829

b_t_map contains information of batch (b) and feature (t). info_B_num_bits tells how many bits are used to cover batch information and is currently computed each iteration given the batch size.

The info_B_num_bits calculation is problematic when max_B_ is symbolic, causing issues with eagerAOT mode. If max_B_ is symbolic, info_B_num_bits is not recomputed and uses the default value which can fail or if there is not enough bits for B.

To resolve the issues, we can make info_B_num_bits constant. Current implementation adjusts info_B_num_bits based on the batch size, causing it to change every iteration. Fixing the values may cause the aforementioned issue of having insufficient bits for B.

This diff implements get_info_B_num_bits_from_T to make info_B_num_bits constant. We first calculate how many bits required to cover T information, as number of features are known at TBE initialization and will remain the same throughout the run. The rest of the bits will be for B information. Since info_T_num_bits remains the same, info_B_num_bits remains the same. If there's not enough bits for B, it will fail.

In V1 interface, since we hit the limit for the maximum number of arguments, we keep the interface the same.

In V2 interface (next diff), we compute info_B_num_bits and info_B_mask once, store them as module parameters, and pass them to lookup and corresponding Autograd and backend functions.

Reviewed By: sryap

Differential Revision: D69387123

@facebook-github-bot
Copy link
Contributor

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

Copy link

netlify bot commented Feb 28, 2025

Deploy Preview for pytorch-fbgemm-docs ready!

Name Link
🔨 Latest commit ae3f7cd
🔍 Latest deploy log https://app.netlify.com/sites/pytorch-fbgemm-docs/deploys/67c26fadbd0aed0008fd3589
😎 Deploy Preview https://deploy-preview-3748--pytorch-fbgemm-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@spcyppt spcyppt force-pushed the export-D69387123 branch from bc14ff6 to 46d9ccd Compare March 1, 2025 02:10
spcyppt added a commit to spcyppt/FBGEMM that referenced this pull request Mar 1, 2025
Summary:

X-link: facebookresearch/FBGEMM#829

` b_t_map` contains information of batch (`b`) and feature (`t`). `info_B_num_bits` tells how many bits are used to cover batch information and is currently computed each iteration given the batch size.

The `info_B_num_bits` calculation is problematic when `max_B_` is symbolic, causing issues with eagerAOT mode. If `max_B_` is symbolic, `info_B_num_bits`  is not recomputed and uses the default value which can fail or if there is not enough bits for B.

To resolve the issues, we can make `info_B_num_bits` constant. Current implementation adjusts `info_B_num_bits` based on the batch size, causing it to change every iteration. Fixing the values may cause the aforementioned issue of having insufficient bits for B. 

This diff implements `get_info_B_num_bits_from_T` to make `info_B_num_bits` constant. We first calculate how many bits required to cover `T` information, as number of features are known at TBE initialization and will remain the same throughout the run. The rest of the bits will be for `B` information. Since `info_T_num_bits` remains the same, `info_B_num_bits` remains the same. If there's not enough bits for B, it will fail.

In V1 interface, since we hit the limit for the maximum number of arguments, we keep the interface the same.

In V2 interface (next diff), we compute `info_B_num_bits` and `info_B_mask` once, store them as module parameters, and pass them to lookup and corresponding Autograd and backend functions.

Reviewed By: sryap

Differential Revision: D69387123
spcyppt added a commit to spcyppt/FBGEMM that referenced this pull request Mar 1, 2025
Summary:

X-link: facebookresearch/FBGEMM#829

` b_t_map` contains information of batch (`b`) and feature (`t`). `info_B_num_bits` tells how many bits are used to cover batch information and is currently computed each iteration given the batch size.

The `info_B_num_bits` calculation is problematic when `max_B_` is symbolic, causing issues with eagerAOT mode. If `max_B_` is symbolic, `info_B_num_bits`  is not recomputed and uses the default value which can fail or if there is not enough bits for B.

To resolve the issues, we can make `info_B_num_bits` constant. Current implementation adjusts `info_B_num_bits` based on the batch size, causing it to change every iteration. Fixing the values may cause the aforementioned issue of having insufficient bits for B. 

This diff implements `get_info_B_num_bits_from_T` to make `info_B_num_bits` constant. We first calculate how many bits required to cover `T` information, as number of features are known at TBE initialization and will remain the same throughout the run. The rest of the bits will be for `B` information. Since `info_T_num_bits` remains the same, `info_B_num_bits` remains the same. If there's not enough bits for B, it will fail.

In V1 interface, since we hit the limit for the maximum number of arguments, we keep the interface the same.

In V2 interface (next diff), we compute `info_B_num_bits` and `info_B_mask` once, store them as module parameters, and pass them to lookup and corresponding Autograd and backend functions.

Reviewed By: sryap

Differential Revision: D69387123
@spcyppt spcyppt force-pushed the export-D69387123 branch from 46d9ccd to 81cb119 Compare March 1, 2025 02:11
@facebook-github-bot
Copy link
Contributor

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

spcyppt added a commit to spcyppt/FBGEMM that referenced this pull request Mar 1, 2025
Summary:
Pull Request resolved: pytorch#3748

X-link: facebookresearch/FBGEMM#829

` b_t_map` contains information of batch (`b`) and feature (`t`). `info_B_num_bits` tells how many bits are used to cover batch information and is currently computed each iteration given the batch size.

The `info_B_num_bits` calculation is problematic when `max_B_` is symbolic, causing issues with eagerAOT mode. If `max_B_` is symbolic, `info_B_num_bits`  is not recomputed and uses the default value which can fail or if there is not enough bits for B.

To resolve the issues, we can make `info_B_num_bits` constant. Current implementation adjusts `info_B_num_bits` based on the batch size, causing it to change every iteration. Fixing the values may cause the aforementioned issue of having insufficient bits for B.

This diff implements `get_info_B_num_bits_from_T` to make `info_B_num_bits` constant. We first calculate how many bits required to cover `T` information, as number of features are known at TBE initialization and will remain the same throughout the run. The rest of the bits will be for `B` information. Since `info_T_num_bits` remains the same, `info_B_num_bits` remains the same. If there's not enough bits for B, it will fail.

In V1 interface, since we hit the limit for the maximum number of arguments, we keep the interface the same.

In V2 interface (next diff), we compute `info_B_num_bits` and `info_B_mask` once, store them as module parameters, and pass them to lookup and corresponding Autograd and backend functions.

Reviewed By: sryap

Differential Revision: D69387123
@spcyppt spcyppt force-pushed the export-D69387123 branch from 81cb119 to f67490d Compare March 1, 2025 02:14
Summary:
Pull Request resolved: pytorch#3748

X-link: facebookresearch/FBGEMM#829

` b_t_map` contains information of batch (`b`) and feature (`t`). `info_B_num_bits` tells how many bits are used to cover batch information and is currently computed each iteration given the batch size.

The `info_B_num_bits` calculation is problematic when `max_B_` is symbolic, causing issues with eagerAOT mode. If `max_B_` is symbolic, `info_B_num_bits`  is not recomputed and uses the default value which can fail or if there is not enough bits for B.

To resolve the issues, we can make `info_B_num_bits` constant. Current implementation adjusts `info_B_num_bits` based on the batch size, causing it to change every iteration. Fixing the values may cause the aforementioned issue of having insufficient bits for B.

This diff implements `get_info_B_num_bits_from_T` to make `info_B_num_bits` constant. We first calculate how many bits required to cover `T` information, as number of features are known at TBE initialization and will remain the same throughout the run. The rest of the bits will be for `B` information. Since `info_T_num_bits` remains the same, `info_B_num_bits` remains the same. If there's not enough bits for B, it will fail.

In V1 interface, since we hit the limit for the maximum number of arguments, we keep the interface the same.

In V2 interface (next diff), we compute `info_B_num_bits` and `info_B_mask` once, store them as module parameters, and pass them to lookup and corresponding Autograd and backend functions.

Reviewed By: sryap

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

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

@spcyppt spcyppt force-pushed the export-D69387123 branch from f67490d to ae3f7cd Compare March 1, 2025 02:23
spcyppt added a commit to spcyppt/FBGEMM that referenced this pull request Mar 3, 2025
Summary:

X-link: facebookresearch/FBGEMM#829

` b_t_map` contains information of batch (`b`) and feature (`t`). `info_B_num_bits` tells how many bits are used to cover batch information and is currently computed each iteration given the batch size.

The `info_B_num_bits` calculation is problematic when `max_B_` is symbolic, causing issues with eagerAOT mode. If `max_B_` is symbolic, `info_B_num_bits`  is not recomputed and uses the default value which can fail or if there is not enough bits for B.

To resolve the issues, we can make `info_B_num_bits` constant. Current implementation adjusts `info_B_num_bits` based on the batch size, causing it to change every iteration. Fixing the values may cause the aforementioned issue of having insufficient bits for B. 

This diff implements `get_info_B_num_bits_from_T` to make `info_B_num_bits` constant. We first calculate how many bits required to cover `T` information, as number of features are known at TBE initialization and will remain the same throughout the run. The rest of the bits will be for `B` information. Since `info_T_num_bits` remains the same, `info_B_num_bits` remains the same. If there's not enough bits for B, it will fail.

In V1 interface, since we hit the limit for the maximum number of arguments, we keep the interface the same.

In V2 interface (next diff), we compute `info_B_num_bits` and `info_B_mask` once, store them as module parameters, and pass them to lookup and corresponding Autograd and backend functions.

Reviewed By: sryap

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

This pull request has been merged in fc9863e.

q10 pushed a commit to q10/FBGEMM that referenced this pull request Apr 10, 2025
Summary:
X-link: pytorch#3748

Pull Request resolved: facebookresearch/FBGEMM#829

` b_t_map` contains information of batch (`b`) and feature (`t`). `info_B_num_bits` tells how many bits are used to cover batch information and is currently computed each iteration given the batch size.

The `info_B_num_bits` calculation is problematic when `max_B_` is symbolic, causing issues with eagerAOT mode. If `max_B_` is symbolic, `info_B_num_bits`  is not recomputed and uses the default value which can fail or if there is not enough bits for B.

To resolve the issues, we can make `info_B_num_bits` constant. Current implementation adjusts `info_B_num_bits` based on the batch size, causing it to change every iteration. Fixing the values may cause the aforementioned issue of having insufficient bits for B.

This diff implements `get_info_B_num_bits_from_T` to make `info_B_num_bits` constant. We first calculate how many bits required to cover `T` information, as number of features are known at TBE initialization and will remain the same throughout the run. The rest of the bits will be for `B` information. Since `info_T_num_bits` remains the same, `info_B_num_bits` remains the same. If there's not enough bits for B, it will fail.

In V1 interface, since we hit the limit for the maximum number of arguments, we keep the interface the same.

In V2 interface (next diff), we compute `info_B_num_bits` and `info_B_mask` once, store them as module parameters, and pass them to lookup and corresponding Autograd and backend functions.

Reviewed By: sryap

Differential Revision: D69387123

fbshipit-source-id: 3ddc1a3e3822525db907a58d66fedc4148e7bc7a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants