-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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: fail when required args not passed when prompt_tuning_init==TEXT #1519
Conversation
Thanks for adding this quality of life feature. Two points:
|
129b8cc
to
f172b0d
Compare
Sure @BenjaminBossan, I tried looking for tests for prompt tuning implementation, but I could not find them, can you point me to that? |
The tests are a bit scattered around, as we group by model architecture and not by method. But a good fit would be to add them to |
f172b0d
to
282fc35
Compare
@BenjaminBossan Have added the tests and made required changes. |
Thanks a lot for adding the test, could you please run |
282fc35
to
0ea3102
Compare
@BenjaminBossan I have run |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
Thanks for the update. I found some very small things still, after that we should be good to go.
Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>
Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
Signed-off-by: Mehant Kammakomati <mehant.kammakomati2@ibm.com>
d4c19c5
to
5851a06
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.
Thanks a lot for adding these early checks.
When
prompt_tuning_init==TEXT
is set, there are some args needed to be provided. For instance,tokenizer_name_or_path
should be set to some tokenizer name or path, failing which would result in errors such astokenizer_name_or_path is taken as
None
. Such errors can be handled and reported meaningfully post config initialization and this PR makes that contribution.