yt
yt copied to clipboard
Updating the linthresh guessing
This is an attempt at updating how the linthresh value is calculated for SymLogNorms when not provided. Note that I based it off of #3849 , so I'm submitting in draft mode until that one is cleared. The changes here are simple enough that it should be easy to update this PR after #3849 is fully reviewed.
IMO the linthresh calculation here nicely handles all of the concerns of #3944 without too much complexity. I need to update/add some tests and the docs would need a little updating, but it'd be great to get some feedback on the approach.
Conceptually, the calculation here uses the minimum absolute nonzero value as a starting guess at linthresh (as in #3295) but then limits the dynamic range of the symlog scale when necessary to avoid the floating point precision issues that caused the errors in #3858. The limiting is sensitive to the data so it does a decent job for a range of data distributions (and should not error for any).
Some images:
fn = 'FIRE_M12i_ref11/snapshot_600.hdf5'
ds = yt.load(fn)
p = yt.ProjectionPlot(ds, 'x', ('gas', 'density'), width=(30, 'Mpc'))
p.show()
And using the sample data from #3858
import yt
ds = yt.load('symlogsets/yt_bug_reproducer/plt04000')
slc = yt.SlicePlot(ds, "z", ("boxlib", "scalar"))
slc.show()
Note that I experimented with the new imshow kwarg (interpolation_stage="rgba"
) but still got some rounding error crashes related to the normalization objects (SymLogNorm
, LogNorm
) and so gave up on that route...
So the issue with using interpolation_stage
is that it is only an imshow
kwarg. When using the Norm
objects directly, it's pretty easy to get it to error with the edge cases we're trying to handle here. e.g., using a linthresh close to the underflow limit will error:
from matplotlib.colors import SymLogNorm
v = SymLogNorm(1e-323, vmin=-1e-1, vmax=1e-1)
v(1e-2) # errors with the familiar `ValueError: Invalid vmin or vmax`.
There's some discussion of adding an interpolation stage argument to the Norm
objects (e.g, the end of this issue), but even if that happens soon we'd then be handling multiple dependency conditions for the same underlying problem. And so I feel like it's a lot simpler to just try to avoid the conditions that give rise to the precision issues.
I mentioned this in one of my replies to a review comment here, but in addition to adding to the existing yt tests, I'm planning to do a more thorough matplotlib-dependent suite of tests to check all these edge cases before switching from draft.
Seems reasonable, thank you for your research on this !
Ok, so I rewrote all of _guess_linthresh
. It felt clearer to avoid the whole abs(min,max) vs min(abs) confusion and treat the positive and negative ranges sequentially. It's maybe a little more verbose but I think it's easier to understand now.
interesting... that failed test passed locally. Will have to dig in further...
#3849 is in, now this should be rebased so we can see only the relevant bits in the diff
I went for it since the rebase turned out a no op.
I like this solution overall. The only thing I see missing from the PR is a brief mention of this in the colorbar linthresh narrative docs, like a sentence describing the linthresh guess (e.g., "linthresh uses the minimum absolute nonzero value but limits the dynamic range of the symlog scale when necessary to avoid floating point precision issues (10**15)"). Thanks for coding this up, Chris!
Thanks for taking care of the rebase, @neutrinoceros ! I'll look through your comments ASAP.
@chrishavlin there's a missing import reported by flake8
Latest batch updates the docs, bumps the answers and incorporates the remaining suggestions. I'll go through and close out obviously outdated threads (done)...