Create and Expose SamVisionModel as public for better accessibility

[geetu040]

What does this PR do?

This PR makes SamVisionEncoder publicly accessible while keeping its name unchanged. This improves usability by allowing users to instantiate the vision encoder directly, similar to how SamModel is used.

from transformers import SamVisionConfig, SamVisionEncoder
config = SamVisionConfig()
model = SamVisionEncoder(config)

Previously discussed here: #36248 (comment)

Before submitting

• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue or the forum? Please add a link
  to it if that's the case.
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• Did you write any new necessary tests?

Who can review?

@amyeroberts, @qubvel, @zucchini-nlp

[github-actions]

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. When it is ready for review, please click the Ready for review button (at the bottom of the PR page).

[geetu040]

Hi @qubvel and @zucchini-nlp

I have updated SamVisionEncoder to make it public, but I am having a problem making TFSamVisionEncoder public.

First, I am not even sure, if it is necessary to have TFSamVisionEncoder public, I am just following the test utils/check_repo.py which fails to find TFSamVisionEncoder

TFSamVisionEncoder saves the vision layers in attribute self.layers, this works fine when TFSamVisionEncoder inherits from keras.layers.Layer but fails when inherited from TFPreTrainedModel which inherits from keras.models.Model. Because keras.models.Model already has layers as a read-only-property.

Changing the name would break backward compatibility, we can either:

1. completely disregard the idea of making SamVisionEncoder public and use modular where it is needed as backbone (in context of Deepseek-VL #36248).
2. make SamVisionEncoder public and leave TFSamVisionEncoder as it as, and exempt this from relevant tests.

[zucchini-nlp]

LGTM overall!

For the TF model, you mean making the model a PreTrainedModel is causing issues or just making it public (i.e. adding in high level init)? I am not very familiar with TF models in transformers, but I think we can figure out a way. Maybe @qubvel has more expertise on that?

Keeping SamVIsionEncoder as pre-trained model is important, if we want to use it as vision backbone with VLMS. Especially useful for setting different load-time configuration for each backbone.

[geetu040]

@zucchini-nlp

I have added TFSamVisionEncoder to __init__, that fixes the failing check.

Though TFSamVisionEncoder still doesnot inherit TFSamPreTrainedModel and therefore is not added to auto modeling.
Fails to inherit because #36493 (comment)

TFSamVisionEncoder saves the vision layers in attribute self.layers, this works fine when TFSamVisionEncoder inherits from keras.layers.Layer but fails when inherited from TFPreTrainedModel which inherits from keras.models.Model. Because keras.models.Model already has layers as a read-only-property.

---

And by the way SamVisionEncoder works all fine. To conclude everything:

SamVisionEncoder

• imported in __init__, available via from transformers import SamVisionEncoder
• part of auto modeling
• imports PreTrainedModel
• available in docs

TFSamVisionEncoder

• imported in __init__, available via from transformers import TFSamVisionEncoder
• not part of auto modeling
• doesnot import PreTrainedModel
• not available in docs

[geetu040]

The failing checks at ci/circleci: tests_torch are fixed in #36422.

[zucchini-nlp]

Thanks! LGTM and can be merged after one small iteration. Left a tiny comment. Also I see test_attention_outputs is failing for Sam Models currently, can you take a look at that?

[geetu040]

Also I see test_attention_outputs is failing for Sam Models currently, can you take a look at that?

They fail because of the SamVisionSdpaAttention which is fixed in #36422

[zucchini-nlp]

Perfect! Then we'll wait until the blocking PR is merged to make CI green and can merge this one

[zucchini-nlp]

@geetu040 can you rebase main, the blocker was merged already! I request review from qubvel, so we don't wait for core maintainer and unblock your for DeekSeek

[qubvel]

Hey @geetu040, thanks for working on the PR, I suppose to make it public we should add a VisionModel class for consistency with other models in the repo, see CLIP / SigLIP for example

[qubvel]

We should have it named SamVisionModel for consistency, for example see CLIP/Siglip

[qubvel]

Thus we should add a SamVisionModel class which will be a subclass of PreTrainedModel

[geetu040]

@qubvel you are suggesting to rename SamVisionEncoder to SamVisionModel right? I hope it doesnot break backward compatibility.

[zucchini-nlp]

+1, it breaks BC if we just rename it, for people who were loading the vision encoder class directly. We can either do something like SamVisionEncoder = SamVisionModel or add it separately

[qubvel]

Yes, renaming is going to be too breaking, it's better to add it as a separate class

[geetu040]

@qubvel
SamVisionModel added alongside SamVisionEncoder

[qubvel]

Thanks for the update, just a small nit to make it simpler and reuse existing code

[geetu040]

ping @qubvel for review

[geetu040]

Hello @qubvel, a soft reminder for review

[qubvel]

Thanks for the ping @geetu040, will review today

[qubvel]

Ok, looks good to me! Thanks for working on it.

P.S. Test failures are unrelated.

cc @ArthurZucker to confirm and merge