@zucchini-nlp , @Rocketknight1, @Cyrilvallez

The Deepseek-VL uses Sam as backbone for encoding high-resolution images.
And to be more specific, the backbone is SamVisionEncoder instead of SamModel, which is not available as a public class. By which I mean that, you can do following with SamModel but not with SamVisionEncoder

from transformers import SamConfig, SamModel
config = SamConfig()
model = SamModel(config)

I think that we should rename SamVisionEncoder -> SamVisionModel, inherit it from SamPreTrainedModel and make it accessible to the user. I don't think it breaks backward compatibility in any way.

Otherwise, we would have to copy all the classes that build SamVisionEncoder for deepseek. There is nothing wrong with this either but having a SamVisionModel along with a SamModel makes sense, since it might benefit someone else as well.

If you think having a SamVisionModel makes sense, should that be done in a separate PR?

Btw, final results would look like this

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

and SamVisionConfig is already available publically.

@geetu040 we had similar situation with ideficsVision afair. Yes, in that case, we can just make it public and add in the docs. Renaming though would be breaking, imo we can leave name as is

@zucchini-nlp is it okay to do it in the same PR? or should I create a new one

@geetu040 imo a new PR will make it easier for us to iterate and review

Hi @zucchini-nlp, I am working on the SamVisionEncoder (going to create the PR soon) and I have a quick question.
I realized that SamVisionAttention and SamVisionSdpaAttention produce attn_weights of different shapes when output_attentions=True.

Can you please answer these 2 questions:

1. Is this allowed in transformers for the 2 attentions to produce outputs of different shapes?
2. And lets suppose we do something that changes the shape of output_attentions, does that break backward compatibility?

@geetu040 no, that is not expected to have different shapes. Usually using sdpa attention means that no attn_weights are returned, so it should be available only through 'eager' attention modules

I see that the weights are calculated on top of SDPA by manual matmul of key and query, which imo defeats the purpose of using SDPA in the first place. Can you remove the returned attention and raise warning similar to what is done in ViT?

@zucchini-nlp sure I'll do that.

@zucchini-nlp , @Rocketknight1, @Cyrilvallez

Hi everyone, I've moved this PR out of draft. Everything is complete except for the model cards. It's now ready for review. Thanks!

Thanks @geetu040 , I will take a look tomorrow

Hi @zucchini-nlp, Thanks for a review, I have updated accordingly.

Also we could use modular, to copy parts of processing and image processing. Not required though

I wanted to apply modular but can't really find much code that is reusable. We can reuse the 2 ModelOutput classes, except for that I cannot really think of anything else that could benefit from modular. I was using the preprocess from ViT, but that's also changed now since #36248 (comment). Do you still suggest that I use modular to keep everything under one file and to reuse the few lines we possibly can?

PS: Failing tests are unrelated