Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

to_pil_image different results depending on numpy/torch input #8090

Closed
rb-synth opened this issue Nov 2, 2023 · 5 comments 路 Fixed by #8097
Closed

to_pil_image different results depending on numpy/torch input #8090

rb-synth opened this issue Nov 2, 2023 · 5 comments 路 Fixed by #8097

Comments

@rb-synth
Copy link

rb-synth commented Nov 2, 2023

馃悰 Describe the bug

to_pil_image has different behaviour depending on torch or numpy input. This is not documented as far as I can see. There is a note that numpy is expected to be HWC, whereas torch is expected to be CHW, but that's not relevant here.

import torch
from torchvision.transforms.functional import to_pil_image
a = torch.rand((100, 101))
print(to_pil_image(a).mode)
# L
print(to_pil_image(a.numpy()).mode)
# F

This is not documented, nor is there any warning, so errors due to this are hard to track down. The problematic code is this section:

    if isinstance(pic, torch.Tensor):
        if pic.is_floating_point() and mode != "F":
            pic = pic.mul(255).byte()

in which the torch.tensor is rescaled. Can we mirror functionality for numpy arrays? (pic * 255).round().astype(np.uint8)

Versions

all versions

@NicolasHug
Copy link
Member

Thanks for the report @rb-synth . @vfdev-5 @pmeier any chance you remember if there's any reason for the current behaviour? Otherwise, any concern with treating this as a bugfix and align both?

@pmeier
Copy link
Collaborator

pmeier commented Nov 2, 2023

any reason for the current behaviour?

Most likely just an oversight.

Otherwise, any concern with treating this as a bugfix and align both?

No, ok for me. TBH, I would have something like

if isinstance(input, np.ndarray):
    input = torch.from_numpy(input)

and from that point no longer bother with any differences. Our "numpy" support for images is questionable at best.

@rb-synth
Copy link
Author

rb-synth commented Nov 2, 2023

@pmeier I have no issue with your suggestion, but that would break backwards compatibility, since documentation states that numpy is expected HWC and torch is expected CHW. If we convert numpy directly to torch, then they'd both be expected to have CHW shape. I'd be in favour of unifying, but realise this would be a breaking change.

@pmeier
Copy link
Collaborator

pmeier commented Nov 2, 2023

Sorry, I missed that in your comment. In that case, let's do

if isinstance(input, np.ndarray):
    input = torch.from_numpy(input).permute((2, 0, 1)).contiguous()

My point here is that we should eliminate all other branching that happens based on numpy vs torch. If we get a numpy image we convert it to a tensor and after that, everything is the same.

@NicolasHug
Copy link
Member

LMK what you think on #8091.

It best not to convert from numpy to pytorch BTW, because the current implementation already does the opposite (convert the tensor to numpy) and a lot of the code assumes numpy arrays, not tensors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants