Skip to content

Conversation

@awgu
Copy link
Collaborator

@awgu awgu commented Oct 27, 2022

Stack from ghstack:

This PR makes a second pass over the constructor. The logic has been grouped into _init_<...> functions based on intent (e.g. _init_prefetching_state() or _init_runtime_state()). This makes the initialization code for composable FSDP much cleaner than having to re-write the same sequences of lower-level helper calls.

This PR also moves _ExecOrderData into its own file _exec_order_utils.py.

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 27, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87923

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Failures, 3 Pending

As of commit 5a5cd73:

The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@awgu awgu added the topic: not user facing topic category label Oct 31, 2022
This PR makes a second pass over the constructor. The logic has been grouped into `_init_<...>` functions based on intent (e.g. `_init_prefetching_state()` or `_init_runtime_state()`). This makes the initialization code for composable FSDP much cleaner than having to re-write the same sequences of lower-level helper calls.

This PR also moves `_ExecOrderData` into its own file `_exec_order_utils.py`.



[ghstack-poisoned]
This PR makes a second pass over the constructor. The logic has been grouped into `_init_<...>` functions based on intent (e.g. `_init_prefetching_state()` or `_init_runtime_state()`). This makes the initialization code for composable FSDP much cleaner than having to re-write the same sequences of lower-level helper calls.

This PR also moves `_ExecOrderData` into its own file `_exec_order_utils.py`.



[ghstack-poisoned]
This PR makes a second pass over the constructor. The logic has been grouped into `_init_<...>` functions based on intent (e.g. `_init_prefetching_state()` or `_init_runtime_state()`). This makes the initialization code for composable FSDP much cleaner than having to re-write the same sequences of lower-level helper calls.

This PR also moves `_ExecOrderData` into its own file `_exec_order_utils.py`.



[ghstack-poisoned]
awgu pushed a commit to awgu/pytorch that referenced this pull request Oct 31, 2022
ghstack-source-id: 1b0ac6e
Pull Request resolved: pytorch#87923
FSDP_SYNCED = "_fsdp_synced"

# TODO (awgu): Refactor this later
SHARDING_STRATEGY_MAP = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why it needs this map if the key and value are the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key is public facing ShardingStrategy. Value is private/internal HandleShardingStrategy. I think we should be able to refactor later to only use the public facing ShardingStrategy. I had two separate ones to avoid the circular import issues a while ago, but now that ShardingStrategy is in its own file api.py, hopefully this should be resolved.

@zhaojuanmao
Copy link
Contributor

ideally we still want to run the basic FSDP benchmarks to sanity check no crash, no regression before landing the PRs as it has huge changes to the core codes

kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Nov 5, 2022
This PR makes a second pass over the constructor. The logic has been grouped into `_init_<...>` functions based on intent (e.g. `_init_prefetching_state()` or `_init_runtime_state()`). This makes the initialization code for composable FSDP much cleaner than having to re-write the same sequences of lower-level helper calls.

This PR also moves `_ExecOrderData` into its own file `_exec_order_utils.py`.

Pull Request resolved: pytorch#87923
Approved by: https://github.com/mrshenli
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
This PR makes a second pass over the constructor. The logic has been grouped into `_init_<...>` functions based on intent (e.g. `_init_prefetching_state()` or `_init_runtime_state()`). This makes the initialization code for composable FSDP much cleaner than having to re-write the same sequences of lower-level helper calls.

This PR also moves `_ExecOrderData` into its own file `_exec_order_utils.py`.

Pull Request resolved: pytorch#87923
Approved by: https://github.com/mrshenli
@facebook-github-bot facebook-github-bot deleted the gh/awgu/153/head branch June 8, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request release notes: distributed (fsdp) release notes category topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants