Skip to content

Fix crash when imshow encounters longdouble data #10768

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

Merged
merged 1 commit into from
Mar 15, 2018
Merged

Fix crash when imshow encounters longdouble data #10768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions lib/matplotlib/colors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,11 +954,6 @@ def __call__(self, value, clip=None):
resdat -= vmin
resdat /= (vmax - vmin)
result = np.ma.array(resdat, mask=result.mask, copy=False)
# Agg cannot handle float128. We actually only need 32-bit of
# precision, but on Windows, `np.dtype(np.longdouble) == np.float64`,
# so casting to float32 would lose precision on float64s as well.
if result.dtype == np.longdouble:
result = result.astype(np.float64)
if is_scalar:
result = result[0]
return result
Expand Down
14 changes: 11 additions & 3 deletions lib/matplotlib/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from math import ceil
import os
import logging
import warnings

import numpy as np

Expand Down Expand Up @@ -264,8 +265,8 @@ def _make_image(self, A, in_bbox, out_bbox, clip_bbox, magnification=1.0,
and magnified by the magnification factor.

`A` may be a greyscale image (MxN) with a dtype of `float32`,
`float64`, `uint16` or `uint8`, or an RGBA image (MxNx4) with
a dtype of `float32`, `float64`, or `uint8`.
`float64`, `float128`, `uint16` or `uint8`, or an RGBA image (MxNx4)
with a dtype of `float32`, `float64`, `float128`, or `uint8`.

If `unsampled` is True, the image will not be scaled, but an
appropriate affine transformation will be returned instead.
Expand Down Expand Up @@ -361,6 +362,13 @@ def _make_image(self, A, in_bbox, out_bbox, clip_bbox, magnification=1.0,
a_min, a_max = np.int32(0), np.int32(1)
if inp_dtype.kind == 'f':
scaled_dtype = A.dtype
# Cast to float64
if A.dtype not in (np.float32, np.float16):
if A.dtype != np.float64:
Copy link
Member

Choose a reason for hiding this comment

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

might as well just make this one if-statement....

Copy link
Member

Choose a reason for hiding this comment

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

There are things happening after this one but not in it...

Copy link
Member

@jklymak jklymak Mar 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

if A.dtype not in (np.float32, np.float16, np.float64):
   warnings.warn()
   scaled_dtype = np.float64

should do it, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually did this in commit a453ba4. This led to problems in some Windows environments and failures in CI. It seems like scaled_dtype needs to be set to np.float64 instead of np.longdouble even if np.longdouble == np.float64.

Copy link
Member

Choose a reason for hiding this comment

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

not in returns something different than !=? I'm not following the logic here at all.

Copy link
Contributor Author

@br-Zhang br-Zhang Mar 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

It's very platform specific. Running on Win-64 gives:

>>> numpy.longdouble in (numpy.float16, numpy.float64)
False

Copy link
Member

Choose a reason for hiding this comment

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

But numpy.longdouble == numpy.float64 returns true?

Copy link
Contributor Author

@br-Zhang br-Zhang Mar 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Very weird.

>>> numpy.longdouble == numpy.float64
False

Copy link
Member

Choose a reason for hiding this comment

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

Again, the problem doesn't have anything to do with the check here; it's the use of scaled_dtype later that causes a problem. If you flatten the condition, then scaled_dtype=np.longdouble is not replaced by np.float64 (on Windows only!) It's then crashing in C code later (which is arguably a NumPy inconsistency) which doesn't understand np.longdouble.

Copy link
Member

Choose a reason for hiding this comment

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

OK; I get it now. everything not np.float16/32 has to be explicitly set to np.float64, even if python thinks np.longdouble is equivalent to np.float64, because the C code doesn't think its equivalent.

Sorry to be so dense, but I do think its pretty bad that the C code ends up incompatible in this way.

warnings.warn(
"Casting input data from '{0}' to 'float64'"
"for imshow".format(A.dtype))
scaled_dtype = np.float64
else:
# probably an integer of some type.
da = a_max.astype(np.float64) - a_min.astype(np.float64)
Expand All @@ -386,7 +394,7 @@ def _make_image(self, A, in_bbox, out_bbox, clip_bbox, magnification=1.0,
# of over numbers.
if self.norm.vmin is not None and self.norm.vmax is not None:
dv = (np.float64(self.norm.vmax) -
np.float64(self.norm.vmin))
np.float64(self.norm.vmin))
vmid = self.norm.vmin + dv / 2
newmin = vmid - dv * 1.e7
if newmin < a_min:
Expand Down
2 changes: 2 additions & 0 deletions lib/matplotlib/tests/test_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,8 @@ def test_empty_imshow(make_norm):
def test_imshow_float128():
fig, ax = plt.subplots()
ax.imshow(np.zeros((3, 3), dtype=np.longdouble))
# Ensure that drawing doesn't cause crash
fig.canvas.draw()


def test_imshow_bool():
Expand Down