-
Notifications
You must be signed in to change notification settings - Fork 51
Migrate hf trainer #287
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
Migrate hf trainer #287
Conversation
Co-authored-by: Alex-Brooks <[email protected]> Signed-off-by: gkumbhat <[email protected]>
Co-authored-by: Alex-Brooks <[email protected]> Signed-off-by: gkumbhat <[email protected]>
Signed-off-by: gkumbhat <[email protected]>
Co-authored-by: Alex-Brooks <[email protected]> Signed-off-by: gkumbhat <[email protected]>
Co-authored-by: Alex-Brooks <[email protected]> Signed-off-by: gkumbhat <[email protected]>
Co-authored-by: Alex-Brooks <[email protected]> Signed-off-by: gkumbhat <[email protected]>
Signed-off-by: gkumbhat <[email protected]>
Signed-off-by: gkumbhat <[email protected]>
Co-authored-by: Alex-Brooks <[email protected]> Signed-off-by: gkumbhat <[email protected]>
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.
from the changes it seems like fine tuning was already using Trainer, you have now added it to prompt tuning as well. I think you meant to re-use some functions, hence you moved them to toolkit.
Prompt tuning is using launch_training
and preprocessing from toolkit, but text_generation_local has its own definition of both functions, _launch_training
. Code looks the same between both launch training, so maybe you forgot to change fine tuning to use toolkit as well?
@Ssukriti , I did this on purpose to avoid making too many refactors in one PR. I added a comment regarding that: https://github.com/caikit/caikit-nlp/pull/287/files#diff-3ca8e28141febc0ff5a812a8b7a2f92997f0098406b924bd4bdbcab680813cc3R73 |
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.
I have tested PT before and after the change with llama model on some quick text samples, and results are same.
Changes