-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Use Mcore ModelParallelConfig in strategy parallelism property #11232
base: main
Are you sure you want to change the base?
Conversation
ffd8eb3
to
6ac5252
Compare
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.
This makes more sense than the ParallelismConfig, thank you.
Signed-off-by: Hemil Desai <[email protected]>
Signed-off-by: Hemil Desai <[email protected]>
Signed-off-by: Hemil Desai <[email protected]>
13bbea3
to
ef5131a
Compare
@hemildesai can you hold on this, I think my previous pr might get affected, since we are deleting encoder_pp_size and encoder_tp_size. #10643 |
This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days. |
This uses Mcore's ModelParallelConfig directly in
strategy.parallelism
. This should also enable backwards compatibility with different Mcore versions as it relies on the Mcore class now.