8000 numpy.maximum since 1.15 raises RuntimeWarning when encountering a NaN even though the docs say it should propagate NaNs · Issue #12038 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content
8000

numpy.maximum since 1.15 raises RuntimeWarning when encountering a NaN even though the docs say it should propagate NaNs #12038

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

Jump to bottom
Closed
tudorprodan opened this issue Sep 26, 2018 · 19 comments
Closed

numpy.maximum since 1.15 raises RuntimeWarning when encountering a NaN even though the docs say it should propagate NaNs #12038

tudorprodan opened this issue Sep 26, 2018 · 19 comments
Labels
06 - Regression 54 - Needs decision

Comments

@tudorprodan
Copy link

numpy.maximum now raises a RuntimeWarning when encountering a NaN, but the documentation says it propagates NaNs and even goes to the trouble of saying exactly how they are propagated, which makes it seem like it shouldn't give a warning.

Reproducing code example:

In [1]: import numpy as np

In [2]: x = np.array([1, 2, np.nan], dtype=float)

In [3]: np.maximum(x, x)
/home/tudor/anaconda3/bin/ipython:1: RuntimeWarning: invalid value encountered in maximum
  #!/home/tudor/anaconda3/bin/python
Out[3]: array([ 1.,  2., nan])

Numpy/Python version information:

In [6]: import sys, numpy; print(numpy.__version__, sys.version)
1.15.1 3.6.6 |Anaconda custom (64-bit)| (default, Jun 28 2018, 17:14:51)
[GCC 7.2.0]
@charris
Copy link
Member
charris commented Sep 26, 2018 •
edited
Loading

Hmm, this comes from #11043 and friends. @mattip looks like we need to review that work and check against the documented behavior.

@charris
Copy link
Member
charris commented Sep 26, 2018

Note that we may still want to raise a warning.

@charris
Copy link
Member
charris commented Sep 26, 2018

Might open for discussion on the mailing list.

@charris
Copy link
Member
charris commented Sep 26, 2018

If we do decide the raising a warning is the right thing, we should document that. I'm tending in that direction as the appearance of NaNs could indicate an error or that nanmax should be used.

@WarrenWeckesser
Copy link
Member

I added a "thumbs up" emoji to the bug report, but it occurs to me that the meaning of doing that is a bit vague, so here's the explicit comment: I agree that maximum should not generate a warning when given a nan. If the code is designed and documented to handle nan, then nan is not an "inval 8000 id value". The warning is wrong in that sense.

[...] nanmax should be used.

I don't think there is nan-function for maximum.

@WarrenWeckesser
Copy link
Member

Raising a warning is also inconsistent. Functions such as numpy.add and numpy.arctan2 do not raise a warning:

In [21]: np.__version__
Out[21]: '1.15.1'

In [22]: np.add(1.0, np.nan)
Out[22]: nan

In [23]: np.arctan2(1.0, np.nan)
Out[23]: nan

@tudorprodan
Copy link
Author

If we do decide the raising a warning is the right thing, we should document that. I'm tending in that direction as the appearance of NaNs could indicate an error or that nanmax should be used.

nanmax does something different, it ignores nans, whereas maximum propagates nans.

In [1]: import numpy as np

In [2]: x = np.arange(3, dtype=float)

In [3]: x[2] = np.nan

In [4]: x
Out[4]: array([ 0.,  1., nan])

In [5]: np.nanmax(np.vstack([x, x[::-1]]), axis=0)
Out[5]: array([0., 1., 0.])

In [6]: np.maximum(x, x[::-1])
/home/tudor/anaconda3/bin/ipython:1: RuntimeWarning: invalid value encountered in maximum
  #!/home/tudor/anaconda3/bin/python
Out[6]: array([nan,  1., nan])

@eric-wieser
Copy link
Member
eric-wieser commented Sep 26, 2018 •
edited
Loading

I don't think there is nan-function for maximum.

fmax is the function you're after

@eric-wieser
Copy link
Member

Some discussion about whether maximum should warn here: #9020 (comment)

@WarrenWeckesser
Copy link
Member

Some discussion about whether maximum should warn here: #9020 (comment)

In that discussion, @juliantaylor said

maximum/minimum should warn, nan's may lead to unexpected results.

The maximum and minimum docstrings explain how nan is handled, so I don't know what "unexpected results" refers to. If the idea is that typically, a nan indicates that a calculation went wrong somewhere, and the user should be warned that they passed a nan to maximum, then the same argument applies to all numpy functions (e.g. add, arctan2, sin, etc.). But most functions don't warn with nan. What is special about maximum and minimum that they should raise a warning where most other numpy functions do not?

@tudorprodan
Copy link
Author

What needs to happen for this to be classified as either a) feature that works as expected or b) regression? What's the process?

I'm asking because I am using code that's throwing a lot of warnings because of this behaviour change. If it's a feature, than I need to work around it, if it's a regression then I just need to temporarily silence that one warning.

@charris
Copy link
Member
charris commented Oct 8, 2018

My own feeling on this as that we should do what is most useful to the users, but I don't know what that is :) I find the arguments for not warning a bit stronger than those in favor, although I'd like to know more about the use case in which the warning is a problem. It would be good to have more opinions, maybe a posting on the mailing list would help. Also @pv, @rgommers Thoughts?

@tudorprodan
Copy link
Author

although I'd like to know more about the use case in which the warning is a problem

The warning is a problem for me because I try to keep my stuff tidy with no warnings.
Also, if you share code with someone else, it's preferable for the other person if the code doesn't raise warnings.

The function has NaN handling very well documented: it explicitly says what happens when NaNs are found and the behaviour is well defined: NaNs are propagated using a well defined rule.
So it's my personal opinion that this is a regression and should be fixed unless the intent is to change the function behaviour. I think if it were the intent to change it in that way, someone would have said so.

But I'm not sure at what point and who decides on that.

@WarrenWeckesser
Copy link
Member

Is anyone currently arguing for keeping the warnings?

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Oct 20, 2018
@charris
Copy link
Member
charris commented Oct 20, 2018

NumPy 1.15.3 is coming up, it would be good to settle this...

@charris charris added this to the 1.15.3 release milestone Oct 20, 2018
@mattip
Copy link
Member
mattip commented Oct 21, 2018

Also maybe related to #8945 where warnings are possibly emitted when comparing (sign, abs, gt, lt ...)

@mattip mattip mentioned this issue Oct 21, 2018
Merged
@wholmgren wholmgren mentioned this issue Oct 24, 2018
Closed
4 tasks
@wholmgren
Copy link

I will chime in as a strong proponent of not emitting these warnings. The reasoning is carefully explained by @WarrenWeckesser and @tudorprodan.

I arrived here because the pvlib python library uses maximum in a way that expects valid nan inputs and desires that they be passed along in the output. We'd like to avoid writing work around functions like this one pvlib/pvlib-python#610

@charris
Copy link
Member
charris commented Oct 24, 2018

I think this is trending strongly in the direction of no warnings.

@eric-wieser
Copy link
Member
eric-wieser commented Oct 25, 2018 •
edited
Loading

Taking a step back here - as far as I'm aware all of the floating point ufuncs have well-defined behavior on nans, as specified by ieee754.

So should we just change the default to ignore this class of fperr, rather than choosing not to emit it?

Edit: nevermind, I wasn't aware we raised this explicitly

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Oct 31, 2018
@charris charris removed this from the 1.15.4 release milestone Oct 31, 2018
@mattip mattip mentioned this issue Dec 17, 2018
Closed
moustakas 749E added a commit to legacysurvey/legacypipe that referenced this issue Apr 12, 2019
@moustakas
use fmax to deal with numpy/numpy#12038
e0ce41a
@wholmgren wholmgren mentioned this issue May 2, 2019
Closed
@mattip mattip mentioned this issue Dec 19, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
06 - Regression 54 - Needs decision
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@charris @WarrenWeckesser @eric-wieser @mattip @tudorprodan @wholmgren
0