Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Mask values < vmin in PowerNorm#25256

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
dstansby wants to merge1 commit intomatplotlib:mainfromdstansby:powernorm-clip

Conversation

dstansby
Copy link
Member

PR Summary

Fixes#25239 (thanks@jklymak for the hint there!). I also took the oppurtunity to tidy up the code to make it clearer what's going on.

Given#25239 is a pretty bad bug (the tick labels on a colorbar are wrong) I think it's worth backporting this.

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (andpytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a.. versionadded:: directive in the docstring and documented indoc/users/next_whats_new/
  • API changes are marked with a.. versionchanged:: directive in the docstring and documented indoc/api/next_api_changes/
  • Release notes conform with instructions innext_whats_new/README.rst ornext_api_changes/README.rst

@dstansbydstansby added this to thev3.7.1 milestoneFeb 19, 2023
@dstansbydstansby marked this pull request as ready for reviewFebruary 19, 2023 10:56
Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

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

I don't think these should be masked. You should just set to -1 or something less than zero. Otherwise the "under" logic won't work.

@dstansby
Copy link
MemberAuthor

I've just copied whatLogNorm does for values < 0 here:

importmatplotlib.colorsasmcolornorm=mcolor.LogNorm(vmin=1,vmax=10)x=norm(-1)print(type(x))# <class 'numpy.ma.core.MaskedConstant'>print(x)# --print(x.data)# 0.0print(x.mask)# True

@jklymak
Copy link
Member

jklymak commentedFeb 19, 2023
edited
Loading

But these are not values less than zero - these are values less than vmin. If you set vmin =10 in log norm and pass it a value of 0.1 it returns -2 and the colormapping will give the under colour. In particular in the above
print(norm(0.1)) returns-1.0

@dstansby
Copy link
MemberAuthor

But the transformation is only well defined for inputs that are >= vmin - any input < vmin will end up with(input - vmin)**gamma being undefined.

resdat = resdat ** gamma

mask = result.mask | under_mask
result = np.ma.array(resdat, mask=mask, copy=False)
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be

good=resdat>=vminresdat= (resdat-vmin)/ (vmax-vmin)resdat[good]=resdat[good]**gamma

This will make the negative part of the scale linear, but it's better than setting it to be masked.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Why do we want to make the negative part of the scale linear? In the same way a log scale is valid forx > 0, and masks the output where any values havex <= 0, thePowerNorm as implemented is valid forx >= vmin, so it is consistent to mask forx < vmin.

Copy link
Member

Choose a reason for hiding this comment

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

Because the way you are doing it here, x < vmin is always masked.

For LogNorm x<vmin isonly masked if x<0. However, 0<x<vmin isnot masked, but set to a number less than 0, and hence shows up as the under-color in the colormapping.

This proposal will allow x<vmin, and always map it to a number less than zero, and hence show up as the under-color in the color mapping.

Again, the reason is that the order of operations is different for PowerNorm. For LogNorm we do

norm = (log10(x) - log10(vmin))/ (log10(vmax)-log10(vmin)

for PowerNorm we do

norm = ((x-vmin)/(vmax-vmin))**gamma

so whether we apply the gamma outside the range [vmin, vmax] doesn't matter too much.

@jklymak
Copy link
Member

But the transformation is only well defined for inputs that are >= vmin - any input < vmin will end up with(input - vmin)**gamma being undefined.

Yes, but thats only because the norm is being done with an order of operations opposite of our other norms. If it were consistent with sayLogNorm, it would return(x**g - vmin**g)/(vmax**g - vmin**g), and erroring vmin, vmax <0 would be fine, and x would mask less than zero.

But the argument for PowerNorm, as put forward by its proponents, is that it is a gamma correction to the colormap, not the data values. In that case, values less than vmin should map to ourunder color.

@dstansby
Copy link
MemberAuthor

Ah, reading through_make_norm_from_scale I think I understand the difference you're describing betweenLogNorm andPowerNorm, thanks for being patient!

I'm still not sure I buy the argument that we should map to the under color though - while the implementation ofPowerNorm might be a bit odd, at the end of the day it's just a function that has a valid input domain, and I think we should be consistent in either mapping values outside that domain to the under/over colours, or masking them.

It seems likeLogNorm already takes the approach of masking values outside the valid input domain, so that's the approach I've taken here. While I'd be happy to discuss this and potentially change it, I still think for this bugfix it's worth copying this behaviour withPowerNorm to avoid confusion for now.

@greglucas
Copy link
Contributor

xref creating a powerscale:#20532

Maybe worth trying to resurrect that PR to help here?

@jklymak
Copy link
Member

jklymak commentedFeb 19, 2023
edited
Loading

@greglucas I think thats orthogonal. The problem here is that all ticks < vmin are being mapped to zero.

I'm suggesting we map linearly x<vmin, and then use the power norm for x>vmin.@dstansby is proposing we just mask x<vmin.

I still believe mapping to something <0 for x<vmin is what we want because it allows the under-color to show.

data=np.arange(25.).reshape((5,5))data[0,0]=np.NaNfig,ax=plt.subplots(figsize=(3,2.5),layout='constrained')norm=mcolors.PowerNorm(gamma=0.5,vmin=2,vmax=23)cmap=cm.get_cmap('RdBu_r')cmap.set_over('m')cmap.set_under('g')cmap.set_bad('y')pc=ax.pcolormesh(data,norm=norm,cmap=cmap)fig.savefig('/Users/jklymak/Downloads/New.png')plt.show()

New

If we go the other way then the green square will be blue in the example above (current behaviour) or yellow (@dstansby suggestion).

@greglucas
Copy link
Contributor

I did not look at that PR closely, I just assumed it would help to handle the values below/above vmin/vmax due to the extended "scale", but perhaps not.

I agree with@jklymak, I don't think masking is the right fix here.

@dstansby
Copy link
MemberAuthor

I agree with@jklymak, I don't think masking is the right fix here.

Why not? Do you not buy my argument in#25256 (comment)?

(sorry to dig my heels in here, but I'm still convinced that if we are going to be consistent withLogNorm, values outside the valid input range should be masked)

@greglucas
Copy link
Contributor

No, I don't buy that argument. I don't think vmin/vmax defines the valid domain.

>>> from matplotlib.colors import LogNorm>>> norm = LogNorm(vmin=100, vmax=1000)>>> norm(1)-2.0

under/over colors are valuable and shouldn't be ignored IMO. I don't want the "bad" value if I provide something less than or greater than vmin/vmax.

On the other hand, if you want to mask divide-by-zero values that seems reasonable.

jklymak reacted with thumbs up emoji

@jklymak
Copy link
Member

Just to re-iterate: LogNorm does a non-linear transform first, followed by a linear normalization:

$$ n = \frac{log_{10}(x) - log_{10}(v_{min})}{log_{10}(v_{max}) - log_{10}(v_{min})} $$

whereas PowerNorm does the linear normalization first, followed by a non-linear transformation of the normalization:

$$ n = \left(\frac{x - v_{min}}{v_{max} - v_{min}}\right)^{\gamma} $$

I think x<v_{min} is just an under value, not a bad data value for PowerNorm, whereas x<0 for LogNorm is a bad data value, so we can't do anything about that in terms of over/under.

greglucas reacted with thumbs up emoji

@dstansby
Copy link
MemberAuthor

If powernorm was impelmented as

$$ n = \frac{x^{\gamma} - v_{min}^{\gamma}}{v_{max}^{\gamma} - v_{min}^{\gamma}} $$

I would agree with everything above - for0 <= x < v_min the transform still returns a valid value, that can be mapped to the under value, and forx < 0 that transform doesn't map to a valid value, so has to be masked.

For the current implementation:

$$ n = \left(\frac{x - v_{min}}{v_{max} - v_{min}}\right)^{\gamma} $$

any valuesx < v_min do not return a valid value, and therefore I think they should be masked.

I agree that

  1. IdeallyPowerNorm should be implemented as the first equation above
  2. If it was,0 <= x < v_min would map to the under color

But, that's not how it's implemented at the moment, and as implementedx < v_min should map to an invalid value.

Perhaps this is worth a 4th(!) opinion, or discussing on a weekly call?

@jklymak
Copy link
Member

I'll just re-iterate the argument that PowerNorm is a linear normalization with a gamma-corrected color scale. That was the justification for the current implementation (the discussion of which I cannot seem to find). If being applied as a color correction, I still feel the under/over colors should behave the same as for a linear norm.

Perhaps@tacaswell should just choose one behaviour or the other - he was the last one to change the clipping behaviour. Perhaps he will just keep it the same clip it has always been? To summarize the choices:

The Norm does

$$ n = \left(\frac{x - v_{min}}{v_{max} - v_{min}}\right)^{\gamma} $$

which can be complex if the argument is <0. Choice for x < vmin are

  1. return 0
  2. return masked
  3. return a negative number (x-vmin)/(vmax-vmin)

@dstansby
Copy link
MemberAuthor

Okay, I think I'm coming round now, apologies again for being stubborn till now!

Forx < vmin we definitely don't want to return 0, that's current behaviour that's causing the issue. I'd propose just returning-1, because we can't extend the function to values belowx < vmin. Thoughts?

I still feel the under/over colors should behave the same as for a linear norm.

If this is the case (and I think it is too), does that meanx < 0 should be mapped in the same way that0 < x < vmin is mapped? Ithink it does, but not 100% sure there...

@jklymak
Copy link
Member

I don't think it matters - if we want to map to -1 that is fine. I was just thinking about when inverting the Norm it is nice to have a unique inverse when possible. Obviously it can't have the same scale as inside [vmin, vmax], so I was proposing just making it linear outside that region.

@jklymak
Copy link
Member

BTW, we can discuss on today's call if you still feel it needs discussion?

@QuLogicQuLogic modified the milestones:v3.7.1,v3.7.2Mar 4, 2023
@anntzer
Copy link
Contributor

I agree that values below vmin should be mapped to a negative value (i.e. the under color). I think@jklymak's point that we may as well make it the transform invertible is good; this can be either by linear extrapolation on the negative side, or perhaps$sign(v-v_\min) \times \left|\frac{v-v_\min}{v_\max-v_\min}\right|^\gamma$?

@jklymak
Copy link
Member

@dstansby, any further opinion on this?

@dstansby
Copy link
MemberAuthor

Sorry, slipped off my radar and not getting back on it any time soon. Anyone else feel free to replace or finish this PR.

jklymak reacted with thumbs up emoji

@jklymakjklymak marked this pull request as draftJune 14, 2023 15:53
@QuLogicQuLogic removed this from thev3.7.2 milestoneJul 5, 2023
@QuLogicQuLogic added this to thev3.7.3 milestoneJul 5, 2023
@QuLogicQuLogic modified the milestones:v3.7.3,future releasesSep 6, 2023
@dstansbydstansby mentioned this pull requestDec 31, 2023
4 tasks
@dstansby
Copy link
MemberAuthor

New PR at#27589

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak left review comments

Assignees
No one assigned
Projects
None yet
Milestone
future releases
Development

Successfully merging this pull request may close these issues.

[Bug]: colors.PowerNorm results in incorrect colorbar
5 participants
@dstansby@jklymak@greglucas@anntzer@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp