[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Function used in many augmentations (convert_image_dtype) has an issue #48701

Closed
shkarupa-alex opened this issue Apr 22, 2021 · 9 comments · Fixed by #49868
Closed

Function used in many augmentations (convert_image_dtype) has an issue #48701

shkarupa-alex opened this issue Apr 22, 2021 · 9 comments · Fixed by #49868
Labels
comp:ops OPs related issues stat:contribution welcome Status - Contributions welcome TF 2.4 for issues related to TF 2.4 type:bug Bug

Comments

@shkarupa-alex
Copy link
Contributor
shkarupa-alex commented Apr 22, 2021 •
edited
Loading

Please make sure that this is a bug. As per our
GitHub Policy,
we only address code/doc bugs, performance issues, feature requests and
build/installation issues on GitHub. tag:bug_template

System information

  • Have I written custom code (as opposed to using a stock example script provided in TensorFlow): No
  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Colab default
  • Mobile device (e.g. iPhone 8, Pixel 2, Samsung Galaxy) if the issue happens on mobile device: No
  • TensorFlow installed from (source or binary): Colab default
  • TensorFlow version (use command below): v2.4.1-0-g85c8b2a817f 2.4.1
  • Python version: 3
  • Bazel version (if compiling from source): No
  • GCC/Compiler version (if compiling from source): No
  • CUDA/cuDNN version: No
  • GPU model and memory: No

Describe the current behavior
There is a function convert_image_dtype that used in many augmentation ops like tf.image.stateless_random_brightness and etc.
That function has a rounding issue here

tensorflow/tensorflow/python/ops/image_ops_impl.py

Line 2290 in 85c8b2a

scale = dtype.max + 0.5 # avoid rounding problems in the cast

Maybe other cases also have this bug (not checked).

When we casting float to int, we should use rounding. In convert_image_dtype rounding implemented as shift by 0.5 which is correct for max value, but is not correct for 0.
See example by link below.

Describe the expected behavior
Every time convert_image_dtype changes value scale following casting to int, it should use rounding before casting.

Standalone code to reproduce the issue
https://colab.research.google.com/drive/1xZqyuAlZu_xkDZZHNsr7K8g6MyapcNPi?usp=sharing
@saikumarchalla
Copy link

Was able to reproduce the issue in TF 2.4 and Nightly version as well. Please find the gist here.

@saikumarchalla saikumarchalla added TF 2.4 for issues related to TF 2.4 comp:ops OPs related issues labels Apr 23, 2021
@amahendrakar amahendrakar assigned ymodak and unassigned amahendrakar Apr 23, 2021
@ymodak ymodak added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label May 3, 2021
@kkimdev kkimdev added stat:contribution welcome Status - Contributions welcome and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels May 6, 2021
@kkimdev
Copy link
Contributor
kkimdev commented May 6, 2021

Thanks for reporting the issue!

@ymodak ymodak removed their assignment May 6, 2021
@pointhex
Copy link
Contributor

Hi, Can I take this issue?

@kkimdev
Copy link
Contributor
kkimdev commented May 25, 2021

Yes, please feel free to do so.

@pointhex
Copy link
Contributor
pointhex commented May 28, 2021 •
edited
Loading

@kkimdev thank you. I did changes and try to check it by unit test, but have one small problem. Could you please tell me or send the link where I can find how to start the exact unit test? e.g ConvertImageTest.testNoConvert.
Could you also tell me please how to check what tests failed because of me? And do you have something like Jenkins or whatever where I can check that I don't break something?

@kkimdev
Copy link
Contributor
kkimdev commented May 28, 2021

@pointhex I think you can put unit test here https://github.com/tensorflow/tensorflow/blob/85c8b2a817f95a3e979ecd1ed95bff1dc1335cff/tensorflow/python/ops/image_ops_test.py and once you open a Github PR, it will kick off pre-submit testings. Thanks!

pointhex pushed a commit to pointhex/tensorflow that referenced this issue May 28, 2021
Made rounding in convert_image_dtype for numbers close to zero
1633b4d 
Fixes tensorflow#48701
@pointhex pointhex mentioned this issue May 28, 2021
Merged
pointhex pushed a commit to pointhex/tensorflow that referenced this issue May 28, 2021
Made rounding in convert_image_dtype for numbers close to zero
dc8272f 
Fixes tensorflow#48701
pointhex pushed a commit to pointhex/tensorflow that referenced this issue May 28, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
7a27594 
Fixes tensorflow#48701
pointhex added a commit to pointhex/tensorflow that referenced this issue May 28, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
29e196f 
Fixes tensorflow#48701
pointhex added a commit to pointhex/tensorflow that referenced this issue Jun 15, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
a79a922 
Fixes tensorflow#48701
pointhex added a commit to pointhex/tensorflow that referenced this issue Jun 15, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
34ab2eb 
Fixes tensorflow#48701
pointhex added a commit to pointhex/tensorflow that referenced this issue Jun 15, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
83c5328 
Fixes tensorflow#48701
pointhex added a commit to pointhex/tensorflow that referenced this issue Jun 23, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
1195859 
Fixes tensorflow#48701
pointhex added a commit to pointhex/tensorflow that referenced this issue Aug 3, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
ed9c059 
Fixes tensorflow#48701
pointhex added a commit to pointhex/tensorflow that referenced this issue Aug 3, 2021
@pointhex
Made rounding in convert_image_dtype for numbers close to zero
08ed485 
Fixes tensorflow#48701
@google-ml-butler
Copy link

Are you satisfied with the resolution of your issue?
Yes
No

@shkarupa-alex
Copy link
Contributor Author

The issue is still not resolved in TF master branch. It seems that PR #49868 was not merged.

@shkarupa-alex
Copy link
Contributor Author
shkarupa-alex commented Dec 6, 2021 •
edited
Loading

Here is a commit where this issue fix was removed 56ab230#diff-a5a22434f0c18768fc2e10c0e0420ac6f111a5802f67e3df54f155bfefc7094f

@shkarupa-alex shkarupa-alex mentioned this issue Feb 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
comp:ops OPs related issues stat:contribution welcome Status - Contributions welcome TF 2.4 for issues related to TF 2.4 type:bug Bug
Projects
None yet
Milestone
No milestone
6 participants
@kkimdev @shkarupa-alex @pointhex @ymodak @amahendrakar @saikumarchalla