Skip to content

Add DINOv2 with registers (using diff)#32905

Closed
NielsRogge wants to merge 12 commits intohuggingface:mainfrom
NielsRogge:add_dinov2_with_registers_diff
Closed

Add DINOv2 with registers (using diff)#32905
NielsRogge wants to merge 12 commits intohuggingface:mainfrom
NielsRogge:add_dinov2_with_registers_diff

Conversation

@NielsRogge
Copy link
Copy Markdown
Collaborator

@NielsRogge NielsRogge commented Aug 20, 2024

What does this PR do?

This PR is a follow-up of #31832 but uses the diff instead of rewriting from scratch.

Had 2 issues with the diff:

  • the drop_path function was causing an issue. Applying the suggestion at #31832 fixed it.
  • the diff doesn't properly add super().__init__() to the Backbone class. I manually added it in the modeling file as otherwise there's an error.

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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.

Copy link
Copy Markdown
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nice! thanks for using diff, really fits well

Comment thread src/transformers/models/dinov2_with_registers/diff_dinov2_with_registers.py Outdated
Comment thread src/transformers/models/dinov2_with_registers/diff_dinov2_with_registers.py Outdated
Comment thread src/transformers/models/dinov2_with_registers/diff_dinov2_with_registers.py Outdated
@NielsRogge
Copy link
Copy Markdown
Collaborator Author

@ArthurZucker thanks for the review, addressed your comments. Note that there were still some issues with the diff converter (see my first message), but other than that this PR seems ready

@codybum
Copy link
Copy Markdown

codybum commented Sep 6, 2024

Support for dinov2 with registers would be very much appreciated!

@ArthurZucker
Copy link
Copy Markdown
Collaborator

#33248 will resolve all your issues 🤗

@ArthurZucker
Copy link
Copy Markdown
Collaborator

I'll ask you to wait a tiny little bit as it will conflict with this work!

@codybum
Copy link
Copy Markdown

codybum commented Sep 24, 2024

@ArthurZucker thanks for the review, addressed your comments. Note that there were still some issues with the diff converter (see my first message), but other than that this PR seems ready

Looks like Modular transformers: modularity and inheritance for new model additions #33248 was merged a few hours ago!

@ArthurZucker
Copy link
Copy Markdown
Collaborator

Yep! Make sure to rebase!

@codybum
Copy link
Copy Markdown

codybum commented Sep 28, 2024

I think I did the wrong @ initially. @NielsRogge do you have time to rebase and update? If not, we can give it a go.

@ArthurZucker thanks for the review, addressed your comments. Note that there were still some issues with the diff converter (see my first message), but other than that this PR seems ready

Looks like Modular transformers: modularity and inheritance for new model additions #33248 was merged a few hours ago!

@NielsRogge
Copy link
Copy Markdown
Collaborator Author

Hi, feel free to take over, it's probably easy as only one method needs to be overwritten.

@NielsRogge
Copy link
Copy Markdown
Collaborator Author

Closing this one, will try out the new modular Transformers when I have time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants