Skip to content
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

Set cache_precision = weights_precision in TBE if it is not explicitly set #3370

Closed
wants to merge 1 commit into from

Conversation

ehsanardestani
Copy link
Contributor

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

For historical reasons, we have decoupled cache_precision and weights_precision. This was to allow for lower precision embedding, while the hot ones in the cache are using higher precision. But it is not used.

This decoupling has been source of unintentional difference in cache_precision and weights_precision for typical EMO usecases where we do want cache_precision == weights_precision. We had enforced this in torchrec (see this), but as new stacks are enabled, this could be overwritten.

Here we enforce cache_precision = weights_precision if cache precision is not explicitly set.

Differential Revision: D65865527

@facebook-github-bot
Copy link
Contributor

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

Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for pytorch-fbgemm-docs ready!

Name Link
🔨 Latest commit dceb83d
🔍 Latest deploy log https://app.netlify.com/sites/pytorch-fbgemm-docs/deploys/673548835a01410008218785
😎 Deploy Preview https://deploy-preview-3370--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.

…y set (pytorch#3370)

Summary:

X-link: facebookresearch/FBGEMM#461

For historical reasons, we have decoupled cache_precision and weights_precision. This was to allow for lower precision embedding, while the hot ones in the cache are using higher precision. But it is not used.

This decoupling has been source of unintentional difference in cache_precision and weights_precision for typical EMO usecases where we do want cache_precision == weights_precision. We had enforced this in torchrec (see [this](https://www.internalfb.com/code/fbsource/[3868325cdafd]/fbcode/torchrec/distributed/batched_embedding_kernel.py?lines=962-963%2C1446-1447)), but as new stacks are enabled, this could be overwritten. 

Here we enforce cache_precision = weights_precision if cache precision is not explicitly set.

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 10ae4f8.

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.

2 participants