Fix RoPE to use width patch size for width dimension and minor refactoring in DINOv3

[Moriyuki-S]

This PR corrects the RoPE calculation to use patch_size[1] (width) instead of patch_size[0] (height) for the width dimension.

Changes

• Fixed Dinov3ViTRopePositionEmbedding to use patch_size[1] for width calculation.

# Current
self.num_patches_w = config.image_size // config.patch_size[0]

# Fix
self.num_patches_w = config.image_size // config.patch_size[1]

• Added a regression test for non-square patch sizes.

• Tightened config typing:
  hidden_act: Literal["gelu", "silu"] (instead of str).

• Fixed LayerScale param multiplication to explicitly read the param value:
  x * self.lambda1[...] (instead of x * self.lambda1).

This ensures the code is semantically correct and supports non-square patches properly.

Checklist

• I have read the Contribution Guidelines and used pre-commit hooks to format this commit.
• I have added all the necessary unit tests for my change. (run_model.py for model usage, test_outputs.py and/or model_validation_colab.ipynb for quality).
• (If using an LLM) I have carefully reviewed and removed all superfluous comments or unneeded, commented-out code. Only necessary and functional code remains.
• I have signed the Contributor License Agreement (CLA).

[gemini-code-assist]

Summary of Changes

Hello @Moriyuki-S, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug in the RoPE calculation for the Dinov3ViT model, specifically concerning how patch dimensions are handled. By correcting the indexing to use the width component of patch_size for width calculations, the model now accurately supports non-square input patches. This enhancement improves the model's robustness and flexibility, ensuring correct positional embeddings are generated regardless of patch aspect ratio, and is validated by a new dedicated test case.

Highlights

• RoPE Calculation Fix: Corrected the Rotational Positional Embedding (RoPE) calculation within Dinov3ViTRopePositionEmbedding to correctly use the width dimension (patch_size[1]) for width-related patch calculations, instead of the height dimension (patch_size[0]). This ensures proper handling of non-square patches.
• Type Hint Refinement: Updated the type hint for the hidden_act configuration parameter to Literal["gelu", "silu"], providing more specific type information.
• Regression Test Added: Introduced a new regression test case to verify the correct behavior of Dinov3ViTRopePositionEmbedding when processing non-square patch sizes, confirming the fix.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• bonsai/models/dinov3/modeling.py
  • Imported Literal from the typing module.
  • Refined the type annotation for hidden_act in ModelConfig to Literal["gelu", "silu"].
  • Corrected the calculation of self.num_patches_w in Dinov3ViTRopePositionEmbedding to use config.patch_size[1].
  • Corrected the calculation of num_patches_w within the __call__ method of Dinov3ViTRopePositionEmbedding to use self.config.patch_size[1].
  • Modified the LayerScale __call__ method to use self.lambda1[...] for explicit indexing.
• bonsai/models/dinov3/tests/test_outputs_dinov3.py
  • Added a new test class TestRopePositionEmbedding.
  • Implemented test_non_square_patch_size_uses_width_patch_dimension to validate RoPE calculations with non-square patch configurations.

Activity

• The author, Moriyuki-S, created this pull request to fix a bug in the RoPE calculation and added a regression test.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly fixes the RoPE calculation to support non-square patch sizes by using patch_size[1] for the width dimension. The addition of a regression test for non-square patches is also a great way to ensure this functionality remains correct. I've added a couple of suggestions to improve code clarity and maintainability by removing unused code and avoiding magic numbers in the new test.

[gemini-code-assist]

These attributes self.num_patches_h and self.num_patches_w are initialized here but are never used. In the __call__ method, local variables with the same names are computed from the input tensor's shape and used instead. To avoid confusion and redundant code, these lines should be removed.

[coder0143]

@Moriyuki-S Thanks for the changes, btw the model is gated, hence we had the colab link. Also, once you are done, do check if final atol is further reduced, later combine all commits into a single commit.