-
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: Multitask prompt tuning with other tuning init #1144
FIX: Multitask prompt tuning with other tuning init #1144
Conversation
This is WIP. I attempted to fix huggingface#1082. While adding tests for the bug, I discovered that I could not make prompt_tuning_init != RANDOM to work. Maybe I'm using it wrong, but I'm not sure what to change.
ping @mayank31398 |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. |
Another friendly ping @mayank31398 |
oops, hey @BenjaminBossan |
FAILED tests/test_multitask_prompt_tuning.py::MultiTaskPromptTuningTester::test_generate_text_1_AVERAGE_SOURCE_TASKS - ValueError: task_ids cannot be None
FAILED tests/test_multitask_prompt_tuning.py::MultiTaskPromptTuningTester::test_generate_text_2_EXACT_SOURCE_TASK - ValueError: task_ids cannot be None
FAILED tests/test_multitask_prompt_tuning.py::MultiTaskPromptTuningTester::test_generate_text_3_ONLY_SOURCE_SHARED - ValueError: task_ids cannot be None is this not expected? |
Yes, but the point is how do I pass them correctly? I tried this and it doesn't work: Probably I'm using it wrong but I'm not sure how :) |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. |
Gentle ping @mayank31398 |
|
I merge main branch into the branch and then ran it. The test seems to pass for me |
Can you try my branch: https://github.com/mayank31398/peft/tree/fix-multitask-prompt-tuning-other-inits |
Strange. I also merged the main branch and as you can see, CI is still red. The failing tests are:
|
@BenjaminBossan ok, I did a fresh conda env install and now I am able to replicate your issue |
@BenjaminBossan fixed. Opened PR against your branch: BenjaminBossan#8 |
Thanks @mayank31398, this has fixed the failing tests! |
@BenjaminBossan @pacman100 can we merge this soon? 🤗 |
Still waiting for a review :D |
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.
Thank you @BenjaminBossan and @mayank31398 for fixing Multitask prompt tuning to work with non-random init configs!
Resolves huggingface#1082. Also, adding tests for prompt_tuning_init != RANDOM. --------- Co-authored-by: Mayank Mishra <32954280+mayank31398@users.noreply.github.com>
This is WIP, do not merge.I attempted to fix #1082. While adding tests for the bug, I discovered that I could not make prompt_tuning_init != RANDOM to work. Maybe I'm using it wrong, but I'm not sure what to change.Tests are now passing, this is ready for review @pacman100 @younesbelkada