ClipSeg broken
Author: mcmonkey4eva
Pull request: no
Labels: name: bug

### System Info

irrelevant, happens on any env with updated transformers, replicated myself on `Transformers v4.46.0` (current release on pip at time of writing)

### Who can help?

@amyeroberts @manuelsh 

### Information

- [X] The official example scripts
- [ ] My own modified scripts

### Tasks

- [x] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

- `pip install transformers==4.46.0` (latest currently)
- Follow official documentation and examples for ClipSeg https://huggingface.co/docs/transformers/model_doc/clipseg#transformers.CLIPSegModel.forward.example

Here's a fully contained python file if you want a lazy replication:
```py
import torch
import numpy as np
from transformers import CLIPSegProcessor, CLIPSegForImageSegmentation

# Create gray image at 512x512 with 32x32 black spot in the middle for clipseg to find
# Note: intentionally use a size that isn't the ClipSeg size just to prove that the input size is irrelevant
arr = np.ones((512, 512, 3), dtype=np.uint8) * 128
arr[240:272, 240:272] = 0

processor = CLIPSegProcessor.from_pretrained("CIDAS/clipseg-rd64-refined")
model = CLIPSegForImageSegmentation.from_pretrained("CIDAS/clipseg-rd64-refined")

with torch.no_grad():
    processed = processor(text="the black spot", images=arr, return_tensors="pt", padding=True)
    print(f"{processed.pixel_values.shape}") # prints 1, 3, 352, 352
    mask = model(**processed)[0] # ValueError: Input image size (352*352) doesn't match model (224*224).

# Now do something with the mask, maybe save a PIL image,
# doesn't matter cause you can't get to here without an error

```


### Expected behavior

Expected behavior: it works, as it did before updating transformers

But instead, it fails with `Input image size (352*352) doesn't match model (224*224).`
See my comment on the PR that broke it: https://github.com/huggingface/transformers/pull/32600/files#r1816732699

Secondarily, there's now a warning about one of the arguments (That, again, is in the docs and has been there for years):
`UserWarning: The following named arguments are not valid for 'ViTImageProcessor.preprocess' and were ignored: 'padding'`


Comments:
Note that temporary workaround for this error is just manually `pip install transformers==4.45.0` to forcibly backdate it, since the error is only present in absolute latest 4.46.0 version

https://github.com/huggingface/transformers/blob/f73f5e62e2383c1cb6975fca70082d6dc51ec6f2/tests/models/clipseg/test_modeling_clipseg.py#L833-L840

Try
```python
model(**processed, interpolate_pos_encoding=True)
```

That does make it work.

Note that that is not a solution as (A) an argument shouldn't be required just to work at all here, and (B) that is incompatible with older versions of transformers, so it can't even work as a workaround code patch.
Also offhand not sure what the original code was doing (was it always interpolating? Or is interpolating wrong to do here?)

A) The argument does appear to be required in this case because `Input image size (352*352) doesn't match model (224*224).`, the purpose of `interpolate_pos_encoding` is to enable input sizes different than the model's.
B) You have a few options for a workaround: set a minimum version of `transformers` in your `requirements.txt`, use `importlib.metadata.version`/`packaging.version` to check version installed package version number and determine whether `interpolate_pos_encoding=True` should be used, use `inspect.signature` to check whether `model.forward` accepts `interpolate_pos_encoding`.
>was it always interpolating

https://github.com/huggingface/transformers/blob/53fad641cfdb5105e2470bcf3ef17ea8e25cc300/src/transformers/models/clipseg/modeling_clipseg.py#L193-L198

In some cases.

Imo these changes might have benefited from a deprecation warning, and documentation should have been updated, but I'll leave it to maintainers to make further comment.

> A) The argument does appear to be required in this case because Input image size (352*352) doesn't match model (224*224)., the purpose of interpolate_pos_encoding is to enable input sizes different than the model's.

I think you missed the point: the input image is _not_ 352. It's 512x512 in my above example, but could be anything. ClipSeg's own code within transformers is resizing it to 352, then complaining that it's the wrong size.

`Input image size` in the `ValueError` refers to the size specified in the [processor](https://huggingface.co/CIDAS/clipseg-rd64-refined/blob/583b388deb98a04feb3e1f816dcdb8f3062ee205/preprocessor_config.json#L19-L21), 352x352. The image itself, 512x512 in your example, is processed to 352x352. Maybe the `ValueError` should specify `pixel values size` instead, but I think the meaning is pretty clear. 352x352 is larger than the model's size, 224x224, so interpolate_pos_encoding is required.

Thanks for reporting, I contributed CLIPSeg to the Transformers library some years ago. There was no `interpolate_pos_encoding` method back then.

I'm personally not a fan of just blindly adding this `interpolate_pos_encoding` method to each model without checking everything still works as expected. For CLIPSeg, the authors [already defined this themselves](https://github.com/NielsRogge/transformers/blob/6e017b5472e33c638f52191a0446a55cf0ebf058/src/transformers/models/clipseg/modeling_clipseg.py#L182). It's recommended to use `CLIPSegProcessor` to prepare the inputs for you and perform a forward pass.

Edit, yes they just introduced a breaking change: https://github.com/huggingface/transformers/blob/1d063793318b20654ebb850f48f43e0a247ab7bb/tests/models/clipseg/test_modeling_clipseg.py#L799

Anyway, I've opened a PR to revert it: https://github.com/huggingface/transformers/pull/34419

All vision models, included the CLIP family models, have been modified (or are in the process to be) with the same interpolate function and with `interpolate_pos_encoding=True` required in the signature (https://github.com/huggingface/transformers/issues/30579).

It is true that ClipSeg already had a function doing that, which works in the same way as the current function (both use bicubic interpolation with `nn.functional.interpolate` and `align_corners=False`). Also, previously there was no need to add `interpolate_pos_encoding=True` in the signature of ClipSeg.

But this has not been "blindly" substituted. The benefits of the new `interpolate_pos_encoding` versus the previous `interpolate_position_embeddings` in ClipSeg are:

1. Ensures compatibility with TorchScript and dynamic input shapes (this came from https://github.com/huggingface/transformers/pull/33226 ),
2. Includes an early return condition to avoid unnecessary interpolation when the number of patches hasn't changed and the image is square,
3. Includes docstrings explaining the method and where is adapted from.

I believe the solution is not to revert back to the previous function, but possibly to remove the need to add `interpolate_pos_encoding=True` to guarantee code compatibility with a deprecation warning, as @hlky suggested, or being explicit in the error message about the need to add `interpolate_pos_encoding=True` in the signature.

@amyeroberts what do you think?

If the behavior is identical to previous with the arg enabled, it should probably be enabled by default?

Again, I need to reiterate: Running clipseg as intended will always produce different shapes that require interpolation. It is explicitly erroneous to have that argument disabled.

@mcmonkey4eva why it is "explicitly erroneous to have the argument disabled"?

Note that the positional encoding introduced affects only the input image, not the output segmentation of ClipSeg.

I am confused why I have to keep repeating this:

If you run the code as intended, it errors. The code does not work if you do not have this interpolation enabled. The size difference that it needs to interpolate between it produced _by clipseg itself_ not by the caller code.

_Again_, the input image can be any size you want, the HF Transformers ClipSeg code is what resizes it to 352, and then complains that the 352 it output is not the 224 it expects.

@mcmonkey4eva You appear to be confused by the currently incorrect documentation, indeed running the code from the incorrect documentation produces an error, it is unfortunate the documentation was not updated to include `interpolate_pos_encoding=True`.

Please refer to the comments above for a further explanation of why this change was made and the proposed solutions i.e. updating documentation, adding a deprecation warning, setting the default `interpolate_pos_encoding` to True, etc

In the meantime, you can easily resolve this in your integration with one of the following options:
- set a minimum version of `transformers` in your `requirements.txt`
```python
transformers>=4.46.0
```
- use `importlib.metadata.version`/`packaging.version` to check version installed package version number and determine whether `interpolate_pos_encoding=True` should be used
```python
import torch
import numpy as np
from transformers import CLIPSegProcessor, CLIPSegForImageSegmentation

import importlib.metadata
import packaging

arr = np.ones((512, 512, 3), dtype=np.uint8) * 128
arr[240:272, 240:272] = 0

processor = CLIPSegProcessor.from_pretrained("CIDAS/clipseg-rd64-refined")
model = CLIPSegForImageSegmentation.from_pretrained("CIDAS/clipseg-rd64-refined")

kwargs = (
    {"interpolate_pos_encoding": True}
    if packaging.version.parse(importlib.metadata.version("transformers"))
    >= packaging.version.Version("4.46.0")
    else {}
)

with torch.no_grad():
    processed = processor(
        text="the black spot", images=arr, return_tensors="pt", padding=True
    )
    print(f"{processed.pixel_values.shape}")
    mask = model(**processed, **kwargs)[0]

```
- use `inspect.signature` to check whether `model.forward` accepts `interpolate_pos_encoding`
```python
import torch
import numpy as np
from transformers import CLIPSegProcessor, CLIPSegForImageSegmentation

import inspect

arr = np.ones((512, 512, 3), dtype=np.uint8) * 128
arr[240:272, 240:272] = 0

processor = CLIPSegProcessor.from_pretrained("CIDAS/clipseg-rd64-refined")
model = CLIPSegForImageSegmentation.from_pretrained("CIDAS/clipseg-rd64-refined")

kwargs = (
    {"interpolate_pos_encoding": True}
    if "interpolate_pos_encoding" in inspect.signature(model.forward).parameters
    else {}
)

with torch.no_grad():
    processed = processor(
        text="the black spot", images=arr, return_tensors="pt", padding=True
    )
    print(f"{processed.pixel_values.shape}")
    mask = model(**processed, **kwargs)[0]
```


Hi, @mcmonkey4eva is not confused because of that, as this has been added to the documentation too, please check https://huggingface.co/docs/transformers/v4.46.0/en/model_doc/clipseg#transformers.CLIPSegModel where the Parameter `interpolate_pos_encoding` is documented. I think in the release notes of the new version this could have been more explicit, or we could have added some deprecation warning.

@mcmonkey4eva, as mentioned before, in the new version you need to add `interpolate_pos_encoding=True` for it to run with different resolutions as the one pre trained and this will solve your problem. 

You can also refer to [here](https://github.com/huggingface/transformers/issues/34415#issuecomment-2438972495) for an explanation of the changes.

> @mcmonkey4eva, as mentioned before, in the new version you need to add interpolate_pos_encoding=True for it to run with different resolutions as the one pre trained and this will solve your problem.

Please by god read what I wrote above repeatedly. This is an incredibly frustrating attempt at communication.

Let me just lay this out nice and neatly so that everyone can get on the same page.

(I'll use "pos embed" to refer to the positional encoding/embedding just for brevity's sake)

- ClipSeg, like most ViTs, accepts 224x224px input images unless the pos embed is interpolated to account for a larger input dimension
- In the original implementation the ClipSeg authors rescaled inputs to 352x352px before feeding them into the model
  - In keeping with this, Transformers' `ImageProcessor` for ClipSeg defaults to 352x352px output resolution
- The original ClipSeg implementation implicitly interpolates the pos embed for whatever input image size it's given.
  - The original authors intended for pos embed interpolation to *always* be enabled, given the unconditional interpolation in the original code and use of a resolution that cannot otherwise be accepted by the model.
- Transformers' original implementation dutifully and faithfully replicated this behaviour: https://github.com/huggingface/transformers/blob/6e017b5472e33c638f52191a0446a55cf0ebf058/src/transformers/models/clipseg/modeling_clipseg.py#L182-L199

This was the status quo for quite some time.

- #32600  replaces the various ViT models' bespoke pos embed interpolation methods/functions with a single uniform implementation
- This implementation is functionally equivalent to the one ClipSeg (and essentially all ViTs) were already using, so unifying these implementations makes sense.
- However, the unified impl differs from the previous ClipSeg-specific impl in one major aspect: **it is disabled by default.**
- Essentially all code using this model class is likely reliant on the existing behaviour
  - as mentioned above, the model authors clearly intended for it to be enabled by default and there is no reason to *not* enable interpolation in normal usage.

---

This is a breaking user-facing API change that is, IMO, entirely unjustified. Merely updating the docs to say "you have to pass `interpolate_pos_encoding=True` now" is not a satisfactory solution; the new argument cannot be passed to the model under old transformers versions without raising an exception, placing undue burden on users who would like to write code that is compatible with a range of transformers versions.

The question is not "how do i get my code to work now"; @mcmonkey4eva certainly doesn't need help with that given the codebases he maintains. The question is "Can we fix this unnecessary breakage of existing code and unintuitive change in default behaviour?"

The simple and straightforward solution here would be to just change the default for `interpolate_pos_encoding` to `True` in ClipSeg's `forward()` definition.

A slightly more complicated one would be to make the default value of this arg a model configuration property, with the default overridden in the model class.

Thanks for the thorough recap of the fundamentals, @neggles! In that vain, I'll make some further points:

> https://github.com/huggingface/transformers/pull/32600 replaces the various ViT models' bespoke pos embed interpolation methods/functions with a single uniform implementation
> This implementation is functionally equivalent to the one ClipSeg (and essentially all ViTs) were already using, so unifying these implementations makes sense.

That PR *added* the function to various ViT models and replaced it only in ClipSeg. This was part of a greater project #30579 to enable dynamic resolution input for more vision models. Other benefits are explained [here](https://github.com/huggingface/transformers/issues/34415#issuecomment-2438972495), namely TorchScript compatability.

> This is a breaking user-facing API change

Yes, as we've said it probably should have been accompanied by a deprecation warning.

> the new argument cannot be passed to the model under old transformers versions without raising an exception, placing undue burden on users who would like to write code that is compatible with a range of transformers versions.
> The question is not "how do i get my code to work now"; @mcmonkey4eva certainly doesn't need help with that given the codebases he maintains

The burden of workarounds is simply part and parcel of being a developer, Transformers itself has [several](https://github.com/search?q=repo%3Ahuggingface%2Ftransformers%20inspect.signature&type=code). If you'd like the codebase to work in the meantime, please refer to the solutions [here](https://github.com/huggingface/transformers/issues/34415#issuecomment-2439728257)

> The question is "Can we fix this unnecessary breakage of existing code and unintuitive change in default behaviour?"

Yes, please refer to the proposed solutions [here](https://github.com/huggingface/transformers/issues/34415#issuecomment-2438395485) and [here](https://github.com/huggingface/transformers/issues/34415#issuecomment-2438972495).

> The simple and straightforward solution here would be to just change the default for interpolate_pos_encoding to True in ClipSeg's forward() definition.

Indeed, this is one of the proposed solutions.


Good take @neggles, I agree with you! Thanks also @manuelsh for clarifying the background.

I believe making `interpolate_pos_encoding` default to `True` would be the best solution here.

However @manuelsh I saw the logits were updated in the [integration test](https://github.com/huggingface/transformers/blob/1d063793318b20654ebb850f48f43e0a247ab7bb/tests/models/clipseg/test_modeling_clipseg.py#L807), that should not be the case, logits should remain the same to ensure backwards compatibility when passing `interpolate_pos_encoding=True`.

@hlky Fair point! I was on my phone so I didn't have the best view of that PR, but my comments still very much apply to the combination of #32600 and #30579 taken together 😝

I'm quite aware of the benefits of the change - and entirely in favor of the overall change, FWIW - but there appeared to be some miscommunication W.R.T. @mcmonkey4eva's purpose with opening this PR, so it seemed like a good idea to try and clear that up.

> The burden of workarounds is simply part and parcel of being a developer, Transformers itself has [several](https://github.com/search?q=repo%3Ahuggingface%2Ftransformers%20inspect.signature&type=code)

Well yeah, but a *lot* of downstream users of this library are data scientists, not developers, and just because python ML library dependency management is already a dumpster fire doesn't mean we should make it *worse* without a good reason! One of the stated goals of `transformers` is to (try to) be simple, straightforward, and user-friendly, and this change is the antithesis of that.

@NielsRogge Thanks! I agree. Seems like the lowest-effort lowest-impact most-obvious choice, and as a bonus it won't cause any problems for anyone who *does* happen to have implemented a workaround in the meantime.

FWIW, it looks like the actual logit change in [#32600](https://github.com/huggingface/transformers/pull/32600/files#diff-80df12a147715f5a77fbb18b75789ef0bcd807ee281384859c1f547f7134d3c1L807-R807) was fairly insignificant:
```diff
         expected_masks_slice = torch.tensor(
-            [[-7.4613, -7.4785, -7.3628], [-7.3268, -7.0899, -7.1333], [-6.9838, -6.7900, -6.8913]]
+            [[-7.4613, -7.4785, -7.3627], [-7.3268, -7.0898, -7.1333], [-6.9838, -6.7900, -6.8913]]
         ).to(torch_device)
```
Probably just down to rounding changes, chasing that down is maybe not entirely worth the effort 🤷‍♀️ 

@mcmonkey4eva, I understand the frustration with the breaking change in ClipSeg requiring an extra argument. I’m addressing this and appreciate everyone’s pragmatic solution to default `interpolate_pos_encoding=True`.

What’s unclear is whether we’re keeping the new `interpolate_pos_encoding` function or reverting to the previous one in ClipSeg. I believe the new one is best (consistent across ViT models, functionally equivalent and improved) yet I see in @NielsRogge's PR https://github.com/huggingface/transformers/pull/34419 we might revert. @NielsRogge, is there another reason for keeping the old function?

@manuelsh we're usually pretty strict on logits matching, up to atol=1e-4 to 1e-6.

So perhaps we could modify the method to keep the old behaviour of interpolation (which matches the [original one](https://github.com/timojl/clipseg/blob/77c4e2dc853880df46413d4a870a725e961f2f73/models/clipseg.py#L118)). 

Hey all! 🤗 
Sorry that things got heated when this is entirely on our side. 

>This is a breaking user-facing API change that is, IMO, entirely unjustified. Merely updating the docs to say "you have to pass interpolate_pos_encoding=True now" is not a satisfactory solution; the new argument cannot be passed to the model under old transformers versions without raising an exception, placing undue burden on users who would like to write code that is compatible with a range of transformers versions.

I completely agree with you, and am sorry that this escaped our reviews, we'll patch this ASAP. 

@NielsRogge I think logits depend on the hardware you are using, the one making the PR might not have the same, and the difference is acceptable! 

Hi @manuelsh the issue is fixed, apologies for my comment earlier, and thanks for the great explainer.

No worries @NielsRogge, I am glad we landed on a good solution.

I have 4.46.2 and this is still broken, is that to be expected? Which version has the fix? :) 

I have this error in Pinokio ComfyUI. I understand I need to install (downgrade) to transformers==4.45.0. I did that with anaconda and pip install transformers==4.45.0 but in E:\pinokio\api\comfy.git\app or where in which folder should it be installed? I also didn't find any test_modeling_clipseg.py, only a modeling_clipseg.py.



> Note that temporary workaround for this error is just manually `pip install transformers==4.45.0` to forcibly backdate it, since the error is only present in absolute latest 4.46.0 version

In which folder do I have to run this command in cmd or anaconda "pip install transformers==4.45.0" (I use PINOKIO ComfyUI)

> I have 4.46.2 and this is still broken, is that to be expected? Which version has the fix? :)

In my current version of CUI (v0.3.14-11-g095d8671 (2025-02-09)), due to the LTX video and built-in component "colpali-engine"
`ltx-video 0.1.0 requires transformers~=4.44.2`
`colpali-engine 0.3.4 requires transformers<4.47.0,>=4.46.1`
transformers v4.46.3 was installed and there is an error with this version. 

After upgrading to transformers==4.48.3, the error is no longer there.