-
Notifications
You must be signed in to change notification settings - Fork 56
Fix additional callbacks #199
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
Fix additional callbacks #199
Conversation
09d10e6
to
4d414d1
Compare
LGTM. Thanks @VassilisVassiliadis I missed this check in my unit testing PR. |
No problem! |
…t_trainer:train() Signed-off-by: Vassilis Vassiliadis <vassilis.vassiliadis@ibm.com>
Signed-off-by: Vassilis Vassiliadis <vassilis.vassiliadis@ibm.com>
Signed-off-by: Vassilis Vassiliadis <vassilis.vassiliadis@ibm.com>
4d414d1
to
8905904
Compare
@anhuong @Ssukriti @alex-jw-brooks Just a bump on this as we'd like to get rid of the need for a fork ASAP and this is just a single line change :-) |
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.
Sorry for the delay in reviewing this PR, appreciate catching the bug and creating a unit test that covers the bug.
no worries! |
Description of the change
Fixes a small bug in the method of train() sft_trainer.py where trainer_callbacks would append an array of user provided callbacks instead of concatenating the 2 arrays.
Without this change you get this exception:
Related issue number
Contributes to #142
How to verify the PR
Invoke the train() method with an
additional_callbacks
parameter that's an array containing at least 1 callback.Was the PR tested