Incorrect segmentation results on float input in 4.31.0

[antoche]

System Info

python-3.9.10
transformers-4.31.0
pytorch-2.0.1

Who can help?

@amyeroberts

Information

• The official example scripts
• My own modified scripts

Tasks

• An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
• My own task or dataset (give details below)

Reproduction

The following example (based on the examples from the docs) gives consistent results with transformers-4.27.2 whether or not image is kept as uint8 or converted to float32. But with 4.31.0, the result is wrong when using the float32 input:

import torch
import numpy as np

from transformers import AutoImageProcessor, UperNetForSemanticSegmentation
from PIL import Image
from huggingface_hub import hf_hub_download

image_processor = AutoImageProcessor.from_pretrained("openmmlab/upernet-convnext-tiny")
model = UperNetForSemanticSegmentation.from_pretrained("openmmlab/upernet-convnext-tiny")

filepath = hf_hub_download(
    repo_id="hf-internal-testing/fixtures_ade20k", filename="ADE_val_00000001.jpg", repo_type="dataset"
)

image = Image.open(filepath).convert("RGB")
image = np.array(image)

# Comment the line below to get the right result in 4.31.0
image = image.astype(np.float32)/255.0

inputs = image_processor(images=image, return_tensors="pt").pixel_values
outputs = model(inputs)
sizes = [np.array(image).shape[:2]]
seg = torch.stack(image_processor.post_process_semantic_segmentation(outputs, target_sizes=sizes))
torch.unique(seg)

Expected behavior

Expected result (observerd behaviour in 4.27.2 regardless of whether the float conversion is commented out):

tensor([ 0,  1,  2,  4,  6,  9, 17, 25, 52, 53])

Actual result in 4.31.0 (unless the float conversion is commented out and the input image is kept as uint8):

tensor([2])

(i.e., the whole image is perceived as one class)

[NielsRogge]

cc @amyeroberts

this seems to be the same issue as discussed in #24857. Basically there's no need to rescale the images yourself before passing them to the image processor. The image processor already handles the rescaling for you.

Alternatively, if you want to handle the rescaling yourself, just instantiate the image processor as follows:

from transformers import AutoImageProcessor

image_processor = AutoImageProcessor.from_pretrained("openmmlab/upernet-convnext-tiny", do_rescale=False)

I'm curious to know why you expected to handle the rescaling yourself :) maybe we can improve documentation on this.

[antoche]

Thanks for the quick reply.

I'm not sure I 100% understand what you mean with "rescaling" here. Do you mean dividing the values by 255 to go from a (0,255) range to a (0,1) range?

If that is what you mean, then the answer to your question is we never do "rescaling", because we never use 8-bit representations in the first place. The example I posted here converts from uint8 to float32 and "rescales" because I wanted to start from a basic example that was as close to the docs as possible. But in our actual applications, we always work in floating point representations, and therefore in the (0,1) range.

I don't know of any rationale to have floating point representations use an arbitrary range like (0, 255), and as far as I have seen, any code dealing with floating-point image representations treat the usable range as (0,1). Images in uint8 use the (0, 255) range simply because that's the only values this finite integer can represent. If an image was in, say, a uint16 representation, they I'd expect it to use the (0, 65536) range. So from my point of view, it seems odd to have to "rescale" my values by multiplying them by 255 in order for them to be processed correctly.

From #24857, I'm also under the impression that transformers might be resizing my inputs by converting them into uint8 in order to let PIL do the resizing. If that is the case, it is also very problematic for us. The whole reason we are using floating point image representations is so we can work on high-precision, high-dynamic-range images. Converting floating point values to uint8 completely destroys this ability. Given that both pytorch and tensorflow can resize image tensors natively, I don't really understand the need to go through a third library that will decimate my data.

Hope it helps, happy to provide more information if that was unclear.

[amyeroberts]

Hi @antoche, thanks for raising this issue and providing so much detail.

Yes, by rescaling we're referring to scaling the pixel values between [0, 1] (or sometimes [-1, 1].

it seems odd to have to "rescale" my values by multiplying them by 255 in order for them to be processed correctly.

I think we're on the same page: you shouldn't have to rescale them. You're right in understanding that we do convert any input images into uint8 when resizing and so your input images would be rescaled by 255, resized and then rescaled back again. This is for historical reasons as Pillow was first used for resizing images and we've kept it mainly for backwards compatibility. Part of the reason Pillow was used is that the processing classes should (as much as possible) be framework independent i.e. a TensorFlow user and PyTorch user should be able to use the same class.

That all being said, this issue has cropped up a few times. For the linked issues e.g. #24857, we're thinking about adding a warning when we detect images with float values being passed in to prevent double rescaling.

For this particular issue, the change to remove rescaling to convert to PIL.Image.Image is more involved and not something I have immediate bandwidth for unfortunately, but will add to my longer term to-do list. For preprocessing the images, I would suggest using torchvision's transforms (this will also likely be a lot faster!). Their recent transforms v2 is v. good for simultaneously handling images and masks.

If you have any other feedback please do let us know.

[antoche]

Ok, I think I understand what transformers is doing now.

It looks like passing do_rescale=False should work around this issue for now at the moment.

[antoche]

I just ran into another instance of this issue in diffusers' StableDiffusionDepth2ImgPipeline.

The StableDiffusionDepth2ImgPipeline passes its input image (which can be either a PIL image containing 8-bit values, a numpy array containing floating point values between 0 and 1, or a torch float tensor in the range 0 and 1) to transformers' DPTFeatureExtractor. With transformers-4.31.0, DPTFeatureExtractor rescales the input by dividing it by 255 no matter what representation it's in, which mangles the tensors and results in an invalid, unusable output.

Because it is StableDiffusionDepth2ImgPipeline doing the call, I can't just change my code to pass do_rescale=False to work around it, the call has to be changed in the diffusers codebase.

I strongly advise against applying such arbitrary blind rescaling of the input data. Rescaling should only be required when converting from an integer type to a floating point type, and depends on the bit depth of the integer type (e.g., if the input values were int16, and the model expects floating point values, the input should be divided by 65535, not 255).

[amyeroberts]

@antoche We've added a check which will output a warning to the user if we detect that they're passing in images which have already had their pixel values scaled between [0-1].

We unfortunately can't control how other libraries call our code. It is however possible for you to modify the code in StableDiffusionDepth2ImgPipeline - that's the beauty of open source! You can either open a PR or fork and change the code as you wish.

The rescaling isn't arbitrary - it's based on an assumption about the inputs. Admittedly this wasn't clear and we've added the warning and updated the doc strings. We try to avoid any "magic" e.g. changing the rescaling value based on the type as this tends to lead to buggy and unexpected behaviour. The user can modify this directly using rescale_factor if they need.

[antoche]

Thanks, that's an improvement. I've reported the diffusers issue as part of huggingface/diffusers#4429 (I'm unfortunately not very keen on opening PRs for projects like diffusers because of the sheer amount of code duplication in it. This makes it really tedious for a contributor to try and improve the codebase).

Regarding "magic" code, I would argue that the default behaviour ought to be not to do any automatic transformation on the input, so the user get exactly what they asked for, without any unexpected slowdowns or data degradation. I think part of the issue here is that the processors seem to default to always scale the input by an arbitrary 1/255 no matter what. If no rescaling was done by default, there would not by any need for "magic" code either.

Conversely, if a library accepts inputs in multiple formats (e.g., int8 tensor, float tensor, numpy array, pil image, etc), then it is reasonable to expect it to massage the inputs in different ways depending on their type.

My impression is that transformers falls in the latter category, i.e., it handles multiple input types and therefore will handle/transform the inputs differently depending on their type (see, e.g., to_numpy_array), but considers that int8 tensors/arrays are the same type as floating-point ones (I would argue they are not).

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.