-
Notifications
You must be signed in to change notification settings - Fork 15
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
[Examples] Move examples to slapo.model_schedule #53
Conversation
The epoi is removed? It seems the test failed because of the package dependency. @comaniac |
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.
Otherwise LGTM. We could later design an API to dispatch the right "schedule_model" function based on the given model.
It should still be there or unit tests will fail. I'm not sure why there's a lint error tho... |
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
I'm still changing the API. Will let you know when I'm done. @comaniac |
Added |
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.
We probably need an offline discussion on Monday about the APIs.
e13e8b5
to
b31e902
Compare
I've refactored all the code and changed the API to |
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.
Should be the last batch of comments.
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.
Per offline discussion this PR still needs to fix GPT-2 schedule.
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.
@chhzh123 I'll leave this PR to you for the final check and merge. Thanks.
I've tested. Thanks @comaniac. |
Description
This PR moves the schedules in the example models to
slapo.model_schedule
, so users can easily retrieve the scheduled models.Checklist