[BC-breaking] Unified inputs for grayscale ops and transforms #2586

vfdev-5 · 2020-08-14T08:39:04Z

Related to #2292

Description:

Unified inputs for grayscale op and transforms: GrayScale, RandomGrayscale, F.rgb_to_grayscale.
Modified F_t.rgb_to_grayscale implemention to match better PIL implementation
- Side effect: this breaks test_adjustments => we need to update this test according to how we test results in test_adjust_gamma.
Deprecated F.to_grayscale in favor of F.rgb_to_grayscale

BC-breaking: rgb_to_grayscale now returns a tensor with the same number of dimensions as the original tensor.

EDIT:

uint8 type is transformed as L = (19595 * r + 38470 * g + 7471 * b + 2 ** 15) / 2 ** 16
How to transform floating point ? -> (0.299 * r + 0.587 * g + 0.114 * b)

Unlocks #2566 #2595

…-5/issue-2292-unify-grayscale

…-6/issue-2292-unify-grayscale

- deprecated F.to_grayscale in favor of F.rgb_to_grayscale

codecov · 2020-08-18T09:16:27Z

Codecov Report

Merging #2586 into master will increase coverage by 1.36%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #2586      +/-   ##
==========================================
+ Coverage   70.28%   71.64%   +1.36%     
==========================================
  Files          95       94       -1     
  Lines        8204     8162      -42     
  Branches     1299     1298       -1     
==========================================
+ Hits         5766     5848      +82     
+ Misses       2019     1903     -116     
+ Partials      419      411       -8

Impacted Files	Coverage Δ
torchvision/transforms/functional_tensor.py	`62.29% <61.53%> (-0.46%)`	⬇️
torchvision/transforms/functional_pil.py	`66.98% <65.21%> (+1.90%)`	⬆️
torchvision/transforms/functional.py	`81.15% <81.81%> (-0.06%)`	⬇️
torchvision/transforms/transforms.py	`76.37% <100.00%> (+0.07%)`	⬆️
torchvision/datasets/vision.py	`48.27% <0.00%> (-15.52%)`	⬇️
torchvision/datasets/utils.py	`58.53% <0.00%> (-6.71%)`	⬇️
torchvision/ops/deform_conv.py	`66.66% <0.00%> (-3.34%)`	⬇️
torchvision/datasets/__init__.py	`100.00% <0.00%> (ø)`
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be8192e...c9a76fa. Read the comment docs.

fmassa

Thanks for the PR Victor!

I've made a few comments, let me know what you think

torchvision/transforms/functional_tensor.py

fmassa · 2020-08-20T11:08:58Z

torchvision/transforms/functional_tensor.py

    """Convert the given RGB Image Tensor to Grayscale.
    For RGB to Grayscale conversion, ITU-R 601-2 luma transform is performed which
    is L = R * 0.2989 + G * 0.5870 + B * 0.1140

    Args:
        img (Tensor): Image to be converted to Grayscale in the form [C, H, W].
+        num_output_channels (int): number of channels of the output image. Value can be 1 or 3. Default, 1.


I wonder if we need this at all or if we should move this logic to the transform itself?

Currently, Grayscale transform is using num_output_channels as argument and RandomGrayscale outputs the same number of channels as in the input.
If you are OK with BC change, we can remove it. It is a bit difficult to estimate how many users apply F.to_grayscale with this option.

I would not say to remove it from Grayscale, but only from the functional implementation.

torchvision/transforms/functional_tensor.py

…-5/issue-2292-unify-grayscale

…-6/issue-2292-unify-grayscale

…-5/issue-2292-unify-grayscale

vfdev-5 · 2020-08-26T21:01:37Z

@fmassa I updated the PR. Changes:

F.to_grayscale works on PIL Image and goes directly to F_pil.to_grayscale.
F.rgb_to_grayscale works on both: PIL images and Tensors and dispatch to F_pil.to_grayscale or F_t.rgb_to_grayscale
no more deprecations
reverted to unique manner to compute F_t.rgb_to_grayscale on float/uint8 tensors and added other review comments
kept num_output_channels in F.to_grayscale, F.rgb_to_grayscale. We can iterate on that.

Let me know if it corresponds to your ideas.

fmassa

Thanks!

fmassa · 2020-08-28T08:43:34Z

torchvision/transforms/functional_tensor.py

+    # This implementation closely follows the TF one:
+    # https://github.com/tensorflow/tensorflow/blob/v2.3.0/tensorflow/python/ops/image_ops_impl.py#L2105-L2138
+    l_img = (0.2989 * r + 0.587 * g + 0.114 * b).to(img.dtype)
+    l_img = l_img.unsqueeze(dim=-3)


this is a BC-breaking change, as before the tensor returned would have 2 dimensions. I'll change the title of the PR to account for this

* [WIP] Unify ops Grayscale and RandomGrayscale * Unified inputs for grayscale op and transforms - deprecated F.to_grayscale in favor of F.rgb_to_grayscale * Fixes bug with fp input * [WIP] Updated code according to review * Removed unused import

vfdev-5 added 5 commits July 2, 2020 19:49

[WIP] Unify ops Grayscale and RandomGrayscale

50a223b

Merge branch 'master' of https://github.com/pytorch/vision into vfdev…

d630b2c

…-5/issue-2292-unify-grayscale

Merge branch 'master' of https://github.com/pytorch/vision into vfdev…

05c3a0a

…-6/issue-2292-unify-grayscale

Unified inputs for grayscale op and transforms

50da7be

- deprecated F.to_grayscale in favor of F.rgb_to_grayscale

Fixes bug with fp input

1078e1d

vfdev-5 mentioned this pull request Aug 18, 2020

Refactor adjust ops tests #2595

Merged

vfdev-5 marked this pull request as ready for review August 18, 2020 08:28

CristianManta mentioned this pull request Aug 18, 2020

adjust_hue now supports inputs of type Tensor #2566

Merged

fmassa reviewed Aug 20, 2020

View reviewed changes

vfdev-5 added 5 commits August 21, 2020 11:58

Merge branch 'master' of https://github.com/pytorch/vision into vfdev…

ba166ec

…-5/issue-2292-unify-grayscale

Merge branch 'master' of https://github.com/pytorch/vision into vfdev…

eef7833

…-6/issue-2292-unify-grayscale

[WIP] Updated code according to review

f228dc8

Merge branch 'master' of https://github.com/pytorch/vision into vfdev…

bc3717e

…-5/issue-2292-unify-grayscale

Removed unused import

c9a76fa

vfdev-5 requested a review from fmassa August 26, 2020 21:01

fmassa approved these changes Aug 28, 2020

View reviewed changes

fmassa merged commit 2eba1f0 into pytorch:master Aug 28, 2020

fmassa changed the title ~~Unified inputs for grayscale ops and transforms~~ [BC-breaking] Unified inputs for grayscale ops and transforms Aug 28, 2020

pmeier mentioned this pull request Mar 10, 2022

RandomGrayscale fails for grayscale tensor inputs #5581

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BC-breaking] Unified inputs for grayscale ops and transforms #2586

[BC-breaking] Unified inputs for grayscale ops and transforms #2586

vfdev-5 commented Aug 14, 2020 •

edited by fmassa

Loading

codecov bot commented Aug 18, 2020 •

edited

Loading

fmassa left a comment

fmassa Aug 20, 2020

vfdev-5 Aug 21, 2020 •

edited

Loading

fmassa Aug 21, 2020

vfdev-5 commented Aug 26, 2020

fmassa left a comment

fmassa Aug 28, 2020

[BC-breaking] Unified inputs for grayscale ops and transforms #2586

[BC-breaking] Unified inputs for grayscale ops and transforms #2586

Conversation

vfdev-5 commented Aug 14, 2020 • edited by fmassa Loading

codecov bot commented Aug 18, 2020 • edited Loading

Codecov Report

fmassa left a comment

Choose a reason for hiding this comment

fmassa Aug 20, 2020

Choose a reason for hiding this comment

vfdev-5 Aug 21, 2020 • edited Loading

Choose a reason for hiding this comment

fmassa Aug 21, 2020

Choose a reason for hiding this comment

vfdev-5 commented Aug 26, 2020

fmassa left a comment

Choose a reason for hiding this comment

fmassa Aug 28, 2020

Choose a reason for hiding this comment

vfdev-5 commented Aug 14, 2020 •

edited by fmassa

Loading

codecov bot commented Aug 18, 2020 •

edited

Loading

vfdev-5 Aug 21, 2020 •

edited

Loading