Skip to content

Fix bug on autocontrast when min==max #4999

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 29, 2021

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Nov 28, 2021

Fixes issue reported by @ain-soph at #4995 (comment)

cc @vfdev-5 @datumbox

@facebook-github-bot
Copy link

facebook-github-bot commented Nov 28, 2021

💊 CI failures summary and remediations

As of commit f00737a (more details on the Dr. CI page):


  • 2/2 failures introduced in this PR

2 failures not recognized by patterns:

Job Step Action
CircleCI binary_linux_wheel_py3.7_rocm4.2 packaging/build_wheel.sh 🔁 rerun
CircleCI binary_linux_conda_py3.9_cu111 packaging/build_conda.sh 🔁 rerun

1 job timed out:

  • binary_linux_conda_py3.9_cu111

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@ain-soph
Copy link
Contributor

Thanks for your response, but this fix doesn't seem to be correct?

You are replacing NAN with 1.0, which is not expected.

Constant across channel should still keep the value after autocontrast, rather than replacing them to 1.0.

Am I understanding autocontrast correctly?

@datumbox datumbox force-pushed the bugfix/autocontrast_equal_minmax branch from 421f593 to 367deb2 Compare November 28, 2021 10:37
@datumbox datumbox force-pushed the bugfix/autocontrast_equal_minmax branch from 367deb2 to cb7d27a Compare November 28, 2021 10:38
@ain-soph
Copy link
Contributor

ain-soph commented Nov 28, 2021

Just to confirm, current fix seems to implement (channel-wise) normalization as I illustrated in #4995 (comment). Is this expected? Or shall we use (image-wise)?

I'm not sure which behavior is we expect... Not quite familiar with this operation.

@datumbox datumbox force-pushed the bugfix/autocontrast_equal_minmax branch from 2ab06ac to 35de04c Compare November 28, 2021 11:13
@datumbox
Copy link
Contributor Author

@ain-soph I believe the correct is to do channel-wise.

I've added one more test that shows/confirms this. Below we are comparing the output of PIL vs TorchVision implementation for an image that has constant values across channels:

def test_autocontrast_equal_minmax():
img_tensor = torch.tensor([[[10]], [[128]], [[245]]], dtype=torch.uint8).expand(3, 32, 32)
img_pil = F.to_pil_image(img_tensor)
img_tensor = F.autocontrast(img_tensor)
img_pil = F.autocontrast(img_pil)
torch.testing.assert_close(img_tensor, F.pil_to_tensor(img_pil))

@ain-soph
Copy link
Contributor

That sounds nice.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@datumbox datumbox merged commit 48b82c1 into pytorch:main Nov 29, 2021
@datumbox datumbox deleted the bugfix/autocontrast_equal_minmax branch November 29, 2021 15:41
facebook-github-bot pushed a commit that referenced this pull request Dec 2, 2021
Summary:
* Fix bug on autocontrast when `min==max`

* Adding PIL vs TorchVision test for min==max

Reviewed By: NicolasHug

Differential Revision: D32759202

fbshipit-source-id: 609b591236fce73ef74fa9f00707af111f8154e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants