-
Notifications
You must be signed in to change notification settings - Fork 31.9k
Clean-up composite configs #34603
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
Clean-up composite configs #34603
Conversation
|
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. |
ArthurZucker
left a comment
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!
I would make sure all tests are run as for tie weight embedding here the changes would only work for the decoder (decoder=True) !
A commit with run-all is enough! 🤗
|
Tests are passing, the |
What does this PR do?
Based on #34410, I realized that BLIP models were the only ones failing a certain common test and the reason is that we manually set the
tie_word_embeddingfromtext_config. I think some config attributes, includingvocab_size(successfully deprecated in VLMs) ortie_word_embeddingsshould be only attributes of text config. We should access them viaconfig.get_text_config()whenever neededHaving an option to access through general composite config and through text config creates two sources of truth, and we might easily end up with situations like in the
check_config_arguments_init. If one sets the attribute throughsetattr, it will not be different from the attribute in text config.So we expect users to set the attr in the respective config if they want it to be used correctly with transformers code. We could restrict setting these common attr in correct sub-config by
attribute_mapprob, but I thought it was too muchPlus, clean up some
attn_implementation=config._attn_implementationthat were left, because it should be passed already as part of respective config