Add support for inheritance from class with different suffix in modular #34077
+696
−11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
Overall sounds good, I applied similar changes here: a2a6a9b which you have probably seen!
Can you try to import them (ex: raise the error earlier, use camel case replace, no old_class_name and new_class_name for simplification etc)
and most importantly, work with a dummy example this way we can see exactly what you are enabling.
Hardest cases are when you inherit from CLIPMLP or CLIPTextAttention for MyModelAttention
yonigozlan
force-pushed
the
add-support-different-suffix-modular
branch
from
9fb8b1a to
b73f446
Compare
Member
Author
|
No I hadn't seen those changes sorry about that, I have added them now. |
ArthurZucker
approved these changes
Oct 15, 2024
Collaborator
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
Yup nice thanks for updating! 🤗 LGTM
16 tasks
BernardZach
pushed a commit
to BernardZach/transformers
that referenced
this pull request
Dec 5, 2024
…ar (huggingface#34077) * add support for different suffix in modular * add dummy example, pull new changes for modular * nide lines order change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Add support for inheritance from class with different suffix in modular, fixes #33900 (comment)
Useful when we want to create a model for a certain task, extending another model used for a different task, as is the case for
ColPaliForRetrievalandPaliGemmaForConditionalGenerationin #33736Not sure if that's in the spirit of modular or if it's the best way to do this though!
Fixes #33900
Who can review?
cc @ArthurZucker , @tonywu71