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.

Sign up for GitHub

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

Jump to bottom

unify the definition of `EmbeddingTableType` #2391

Open

beekbin wants to merge 1 commit into pytorch:main from beekbin:export-D62596655

Commits on Sep 13, 2024

unify the definition of EmbeddingTableType (pytorch#2391 )

Summary:
Pull Request resolved: pytorch#2391

# Problem
`EmbeddingTableType` is introduced recently for ZCH.

There are two same definitions of `EmbeddingTableType`
*  one is defined in [torchrec/fb/distributed/sparsenn_configs.py](https://www.internalfb.com/code/fbsource/[3a83814d95f27eec3bf425d468a5234d6ca8a381]/fbcode/torchrec/fb/distributed/sparsenn_configs.py?lines=110)
* The other is defined in [minimal_viable_ai/core/model_family_api/configs.py](https://www.internalfb.com/code/fbsource/[bbe727e13ae251889c5df57b58f2cbc05675662e]/fbcode/minimal_viable_ai/core/model_family_api/configs.py?lines=76)

**Problem is that**
The fields of `EmbeddingTable` defined in `minimal_viable_ai/core/model_family_api/configs.py` will be assigned to the `EmbeddingTable` defined in `torchrec/fb/distributed/sparsenn_configs.py`.

The assignment happens at [minimal_viable_ai/sandbox/umia_hstu/umia_configs_utils.py ](https://fburl.com/code/5gb24p9j) for model publish.
So if the EmbeddingTable are different, the assignment will fail.

# solution from this diff
* Option1: keep one of them, and the other import it. This would cause dependency loop;
* Option2(this diff): move the definition of `EmbeddingTableType` to [torchrec/modules/embedding_configs.py](https://fburl.com/code/tvob11sz)

Differential Revision: D62596655

Songbin Liu authored and facebook-github-bot committed Sep 13, 2024