-
Notifications
You must be signed in to change notification settings - Fork 430
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
Correctly process parallelism_config['tp']
when it's a dict
#3434
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oof good catch
@mvpatel2000 I keep getting errors on 4 gpu tests because it's detecting the |
pytest.mark.filterwarning? |
@mvpatel2000 I already added this |
I think its a regex issue, should be |
ah that makes sense @mvpatel2000 lemme try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ml#3434) * big fix * testing * ignore * ignore * ignore * Update test_fsdp_checkpoint.py * Update test_fsdp_checkpoint.py --------- Co-authored-by: Mihir Patel <[email protected]>
* big fix * testing * ignore * ignore * ignore * Update test_fsdp_checkpoint.py * Update test_fsdp_checkpoint.py --------- Co-authored-by: Mihir Patel <[email protected]>
What does this PR do?
This fixes a bug where if the TP configuration (specified through
parallelism_config['tp']
) was passed in as a dict, it would not be correctly processed into the finalparallelism_config
. This also means that whenever we were specifying the TP config as a dict before, it was not actually being set.Modified unit tests to make sure the TP config is actually being set.
What issue(s) does this change relate to?
Before submitting
pre-commit
on your change? (see thepre-commit
section of prerequisites)