-
Notifications
You must be signed in to change notification settings - Fork 62
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
[Bug-Fix] fix attention head intervention for multiple models #159
Conversation
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! Thanks for the change!
@Bakser would you provide an unit test for one of the model type you changed? e.g., llama thanks. i can add in a new unit test based on your script. |
Sure, I can help with that. I'm not quite familiar with the unit tests of this repo. Do you mean something like the |
yes! that would be great! and you can initialize a much smaller llama for test! e.g., just a single layer llama for instance, since we want the unit test to be quick. thanks |
@Bakser hey! any updates on the progress? thanks! |
I didn't work on it on the weekend but I think I can finish it today. sorry for worrying |
I find that I underestimated the workload. I will try to finish it in a couple of days. |
@frankaging I've finished the test for Llama. Sorry for the delay since I was on travel. Basically, I just copied the tests in It can be run with
|
Thanks! @Bakser |
Description
Fix the bug reported in #158 by modifying the modeling scripts of
gemma
,gpt_neo
,gpt_neox
,llama
,llava
,mistral
, mainly involving three points:split_head_and_permute
operation to split the hidden representations by attention heads_lm_type_to_module_mapping
), which missed elements inv[1:]
Testing Done
All tests passed.
Checklist:
[Your Priority] Your Title