Skip to content
This repository was archived by the owner on Aug 7, 2024. It is now read-only.

[3/x]: simplify FSDP1 test and add coverage for dynamic scaling #293

Closed
wants to merge 1 commit into from

Conversation

vkuzo
Copy link
Contributor

@vkuzo vkuzo commented Jul 1, 2024

Summary:

1. simplify the FSDP test, instead of testing 1 GPU vs N GPUs, instead
   hold the number of GPUs constant and test bf16 vs float8. Remove
   various technical debt that accumulated in this test.
2. add testing for dynamic scaling of weights

Test Plan:

```
./test/test_fsdp.sh
```

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@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 Jul 1, 2024
vkuzo added a commit that referenced this pull request Jul 1, 2024
Summary:

1. simplify the FSDP test, instead of testing 1 GPU vs N GPUs, instead
   hold the number of GPUs constant and test bf16 vs float8. Remove
   various technical debt that accumulated in this test.
2. add testing for dynamic scaling of weights

Test Plan:

```
./test/test_fsdp.sh
```

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: fcd6368
Pull Request resolved: #293
@vkuzo vkuzo requested a review from drisspg July 1, 2024 20:44
Copy link
Contributor

@drisspg drisspg left a comment

Choose a reason for hiding this comment

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

❤️

@vkuzo
Copy link
Contributor Author

vkuzo commented Jul 2, 2024

@vkuzo has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 006f89a.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants