-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Allow imshow from float16 data #15436
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
Conversation
lib/matplotlib/image.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are casting to higher bit depth isn't it increasing float precision?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have made that better, but the only casting the warning really covers is float128 -> float64. Everything else is cast automatically. From what I could tell they were already casting float128 down and ints into floats. float16 through float64 were preserved and passed along as is so I'm wondering if it isn't out of scope here to just cast float16 up.
See my comment on #15432. I do not think this is the correct fix.
If I am wrong, this still needs a test.
It looks like we were already casting quadruple floats down into doubles here. The second commit preserves that and casts float16 to it's nearer float32 and warns on any float casting on the python side.
Co-Authored-By: Elliott Sales de Andrade <quantum.analyst@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please change this so that we don't need to access float128, a series of known cases and returns with a final "if we don't know what this is, cast to float64" is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test for this? There's already test_imshow_float128, so something similar for float16 would be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps the whole thing can be written as scaled_dtype = np.float64 if A.dtype.itemsize > 4 else np.float32? (this way things will keep working even if numpy adds obscure formats like 8-bit floats)
Also, do we actually want to warn when upcasting float16 to float32 (which doesn't lose precision)? For example we already silently cast ints to floats in many places in the lib.
Uh oh!
There was an error while loading. Please reload this page.
Removes ValueError "dtype not supported" error when plotting data with dtype float16 ( Fixes #15432 ). This casts everything into dtype float64. Casting is done silently when upscaling precision and with a warning when precision will be decreased.