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

Remove test installation #709

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

Remove test installation #709

wants to merge 4 commits into from

Conversation

YLGH
Copy link
Contributor

@YLGH YLGH commented Oct 11, 2022

No description provided.

Ying Liu and others added 4 commits October 7, 2022 18:28
Summary:
Pull Request resolved: #662

See  https://docs.google.com/document/d/1ASiz0Fu3c8cCHmsbseUdulU4jfTgdwZAxvlywgXiBAU/edit for more context

tl;dr:

Currently the only “feasible” way to specify a sharding plan is by using the OSS planner, and using constraints. I say “feasible” because although we can manually construct a ShardingPlan/ParameterSharding, it’s quite cumbersome to do so. Additionally, users do not have the expressibility to say “I want to place this parameter on rank 0” when using planner.

With this new API we can construct sharding plans in a direct way:

```
ebc = EmbeddingBagCollection(...)
plan = construct_parameter_sharding_plans(
            ebc,
            {
                "table_0": data_parallel_sharding(),
                "table_1": row_wise_sharding(),
                "table_2": column_wise_sharding(),
                "table_3": column_wise_sharding(ranks=[0,1,2]),
                "table_4": table_row_wise_sharding(host_index=2),
            },
            EmbeddingBagCollectionSharder()
        )
ebc = sharder.shard(
   ebc,
   plan
)
```

Note, that this is not intended to replace the greedy planner. Instead it allows users to shard modules without being FORCED to use planner. Users will be able to user planner in the exact same way.

Having alternatives instead of planner has a couple of benefits

1. There are bugs in the planner (we should be able to shard something, but planner may have a false negative rejection)
2. Planner doesn't correctly account for new architectures/models /optimizers
3. Easier to directly use torchrec (new users don't need to understand how to use planner).

-----

So I decided to use a generator function because alternative ways would require creating a new dataclass called like TableWiseConfig(row=...), and I didn't like the look of it.

Having just the atomic generator functions is also nice since it doesn't couple these configs with the construct_parameter_sharding_plans

Differential Revision: D39803183

fbshipit-source-id: b8cb5da632f72c809559d4a4ebcc958cd9f6f92a
@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 Oct 11, 2022
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants