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

Support sending using lengths to TBE instead of just offsets #2557

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

Conversation

PaulZhang12
Copy link
Contributor

Differential Revision: D64906767

Felicity Liao and others added 2 commits November 12, 2024 16:55
…arded & Quantized EBC & PEA (pytorch#2553)

Summary:
Pull Request resolved: pytorch#2553

Reduce the number of TBEs created per Sharded & Quantized EBC & PEA.

We realized we are creating a separate TBE for different datatype for every EBC or PEA module. However, FBGEMM's TBE module actually supports having embedding_tables with different data types all within the same TBE.

This optimization can help reduce the # of inputs to the merge net after model split - which we realized was the portion with the most regression for TorchRec Inference

Differential Revision: D64066446
@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 14, 2024
@facebook-github-bot
Copy link
Contributor

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

PaulZhang12 added a commit to PaulZhang12/FBGEMM that referenced this pull request Nov 26, 2024
facebook-github-bot pushed a commit to pytorch/FBGEMM that referenced this pull request Nov 27, 2024
Summary:
X-link: facebookresearch/FBGEMM#508

X-link: pytorch/torchrec#2557

Here we modify FBGEMM inference TBE to have the forward logic live in `_forward_impl`. This change makes it easy for subclasses of TBE to extend without having to call `super()`, which is not TorchScriptable. An example of a subclass using this is D66515313.

Having subclasses TorchScriptable is vital for inference as TBE is generally not FX traced through.

Reviewed By: sryap

Differential Revision: D64906767

fbshipit-source-id: 41bab272c2611fc97dece7f89fdbbd820671843c
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