Fix huge imshow range #18458
Fix huge imshow range #18458
Conversation
closes matplotlib#18415 We need to special case when rescaling the user input for interpolation with LogNorm. The issue is that due to the inherent precision limits of floating point math we can end up with errors on the order if 1e-18 of the overall data range when rescaling. If the bottom vlim is close to 0 and the top is order 10e20 than this error can result in the rescaled vmin being negative which then fails when we (temporarily) change the vmin/vmax on the norm. We started adjusting the vmin/vmax to work around the same issue with float precision in matplotlib#17636 / 3dea5c7 to make sure that values exactly equal to the limits did not move across the boundary and get erroneously mapped is over/under. Long term we may want to add the ability for the norms to suggest to their clients what the constraints on the vmin/vmax are, but hard-coding a special case for LogNorm is the pragmatic solution.
|
This is probably fine for 3.3.2 but this really needs a proper rethink with tolerances for floating point error built in consistently and in a way we can explain to end users. |
I'm really not sure there is a "right" way to do this because no matter what you do it is wrong in some situation. |
|
I agree its frustrating! But I feel we've been playing whack-a-mole - I think a bit of research and carefully laying out what we do and can't do would go a long way to fixing things. FWIW, I think the current bug is pretty bad. I think that the "bug" we were trying to fix should really have been the user's domain. If you have vmin=1 and vmax=1e20, you cannot expect x=1e20 to be guaranteed to not be "over", and the user should know enough to put a bit of slop into the |
|
@jklymak I'm not sure this is the right venue to hash this out, but it's not even obvious to me why you'd need any margin on vmax. |
|
I think #16910 (comment) describes the issue relatively well, though I guess we eventually came to the wrong conclusion. |
|
I stand by the conclusion we landed on, we should not expect users to be aware of our internal mechanizations / representations of the data / transforms and we should not silently re-order values. All of this difficulty is driven by us using the Agg re-sampler for our up/down sampling of the images and it's [0, 1] + hard clipping constraints (which was in turn driven by changing the default colormap, seeing red in a viridis colored image, and realizing the colormapping -> resampling in RGBA space was not the right thing to do). If we replaced the resampler with something without those constraints we would at least be able to move all of these issues to the other side of the function call or avoid all together. |
|
The two py38 failures are collisions with nbconvert. Not sure why it is windows only, but more recent jobs run with nbconvert 6.0.2 and these failed with 6.0.1, restarted the jobs to hopefully pick up the new versions. |
|
I'm just suggesting that for 3.4 someone sit down and write it out carefully and prove that its mathematically correct, and that we clearly delineate any floating point limitations we can't get around. |
|
Wouldn't it be simpler at some point to just rewrite the resampling code ourselves? It can't be that hard(*)... (* conditions apply) |
2-D convolution is pretty slow so I'd think it would have to be done in C? How quick is skimage and would we want that as a dependency? OTOH I don't see that it does filtering as it resamples, so its different than Agg. Same with |
|
I don't mind rewriting the resampling code in C myself (some day). |
|
... and all the filter/interp method support etc? It just seems like the kind of thing that people have done 100 times. Its just the Agg algorithm doesn't track over/under, whereas that is a "feature" we have. |
|
The filters are likely not that hard to implement themselves, as they are probably just one formula each... |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free to suggest an improvement. |
Merge pull request matplotlib#18458 from tacaswell/fix_huge_imshow_range Fix huge imshow range Conflicts: lib/matplotlib/colors.py - had spurious commit in the initial PR which re-factored code not on the 3.3.x branch lib/matplotlib/tests/test_image.py - conflicts from many tests added to bottom of test_image.py on default branch
…-v3.3.x Backport PR #18458: Fix huge imshow range
PR Summary
closes #18415
We need to special case when rescaling the user input for
interpolation with LogNorm. The issue is that due to the inherent
precision limits of floating point math we can end up with errors on
the order if 1e-18 of the overall data range when rescaling. If the
bottom vlim is close to 0 and the top is order 10e20 than this error
can result in the rescaled vmin being negative which then fails when
we (temporarily) change the vmin/vmax on the norm.
We started adjusting the vmin/vmax to work around the same issue with
float precision in #17636 / 3dea5c7 to make sure that values exactly
equal to the limits did not move across the boundary and get
erroneously mapped is over/under.
Long term we may want to add the ability for the norms to suggest to
their clients what the constraints on the vmin/vmax are, but
hard-coding a special case for LogNorm is the pragmatic solution.
There are still some issues with this sort of extreme data range. With the default interpretation (anti-aliased) when zoomed out we will get stippling that disappears when you zoom in (because the interpolation kernels are not perfect and the constant background at -1 picks up noise on a scale big enough to get above 100 (this is because we shrink and re-scale the data to the [0, 1] range before interpolation and then scale back out to the full range at the end). It is extra visible on this example because things are flipping between "bad" (defaults to white) and "good but small".
PR Checklist
pytestpasses).flake8on changed files to check).