-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
PowerNorm: do not clip negative values #10234
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
Ummm.... I don't know who wrote I'd expect 0.5 to map to |
In #7631: They do Unless I'm being stupid, I don't see that anyone looking at #1204 caught this or this was discussed, but I may have missed it. |
Yes, it is equivalent to ((a - vmin) / (vmax - vmin))**gamma, so a plain Normalize is the case of gamma=1. This seems reasonable to me; we are not dealing with logs. It is consistent with the description of Gamma correction: https://en.wikipedia.org/wiki/Gamma_correction. First the data are linearly scaled to the 0-1 range based on specified vmin and vmax, and second, that 0-1 range is mapped to a new 0-1 range by raising it to a power. The puzzle to me is why the clipping was put there in the first place; it looks like confusion between the need for the second stage to be in the 0-1 range and the implementation, which put part of the clipping in the first stage--where it doesn't belong. I think this PR, removing that odd restriction of the original data to positive values, is doing the right thing. |
Of course @anntzer picked up on it in #7294 (comment) |
OK, but thats saying the scale should be the same if vmin=0, vmax=1, or if vmin = 10000 and vmax = 10001. A parabola x**2 has a very different slope at those two values. If the colorbar ticks go 0-1 I'd be fine with that, but they say 10000 to 10001, with the ticks quadratically spaced as if it were from 0 to 1. Thats seems pretty un-numerate to me. If the user wants to normalize between 0 and 1, they should do so and make the colorbar plotted as such. Not sure I get the analogy to gamma correction - thats mapping 0-1 to 0-1 isn't it? But hey, I never use |
Sorry, parting shot: If I had two axes that were side-by-side and each was power-norm 2, and one went from 0-10, and the other from 5-10, with the second one half as long as the first, I'd expect the spacing between 7 and 8 to be the same in each axis. |
@jklymak Indeed, The code
expectedly outputs
after the fix. The value of 0 is in no way special for the rest |
My point is that |
@jklymak you are talking about a different sort of transform than what is implemented in PowerNorm. What is implemented is not going to be changed, with the likely exception of this PR. If you see a compelling use case for the version you are describing, then it will need a different PR and a different name. |
Fair enough. Suggest docs change to make the order of operations explicit: |
Sorry for inaccuracy: |
By all means, making it clear in the docstring exactly what PowerNorm does would be an important addition to this PR. |
ping! Is this still active? I think this normalization is mathematically wrong-headed, but at least lets document it properly! |
Are there any show-stoppers left to get the pull request accepted? (are any additional changes expected from my part?) |
It needs to pass the tests. |
I am confused by this, it looks like it does still clip negative values to 0... |
Yes, after the norm has been applied. Thats desirable (vmin maps to zero) |
I was wondering about that also--instead of clipping, should the values be "bad" (out of range?) so they would be colored as such? |
@efiring I always forget how the out of range stuff works. What gets put in if data is out of range? I didn't quickly find an answer (ahem Documentation issue). Note that I don't think LogNorm puts data out of range, it just clips.... |
PR Summary
Do not clip negative values in PowerNorm, consistently with LogNorm.
PowerNorm can now be used with offset data.