np.float64(np.nan) % 1 gives warning
#18170
Comments
|
Hi! I'm new. Can I work on this issue? |
|
Unless there is a comment that someone is already busy on it (fairly recently), everything is always up for grabs. We don't really assign issues. So of course, go ahead. |
|
Thank you. I'm working on it! If you give me time I can solve it. I think the problem is with The warning is raised by the
Now I'm trying to find the connection between Any guidance is accepted! meanwhile I will continue working on it. This task is helping me to understand the library internally! |
|
@arubiales if the warning is actually manually set, it would be something like |
|
@seberg I tried to find where the error is originated without success. I have looked for in many files and change many function some of them are:
After try this things and more, I fell (please confirm if you know) that the error come from |
|
Hmmm, I think we may have made a mistake here recently, it might even be a regression (although that would probably 1.20 not 1.19, but I am not sure); and yeah, I did look at those changes also :). We have the code in Which smells wrong, but I might be forgetting something. And as the comment suggests, there was a reason for that, namely that it algined behaviour between different Assuming that removing those lines does not change this, I am not sure what causes the flag to be set, but it has to happen in the Now I am a bit worried that we got a regression in 1.20 related to this. Was the original report on NumPy master or the pre-release @nschloe? EDIT: Sorry behaviour is present in 1.19.x, so removing "regression" again (leaving the milestone for now, but just as a remainder for myself to look at it again at some point). |
|
A bit of digging, this helps a little: https://www.gnu.org/software/libc/manual/html_node/FP-Comparison-Functions.html The comparison functions are another case where the invalid flag is set (that and functions that create a new NaN), which makes sense, since these functions throw away the information that the input was NaN. After some annoying gdb stepping through, I found the culprit (the above comparisons basically), so this removes it: diff --git a/numpy/core/src/npymath/npy_math_internal.h.src b/numpy/core/src/npymath/npy_math_internal.h.src
index ff4663dc3..b8dc0e77a 100644
--- a/numpy/core/src/npymath/npy_math_internal.h.src
+++ b/numpy/core/src/npymath/npy_math_internal.h.src
@@ -519,10 +519,7 @@ NPY_INPLACE @type@
npy_@kind@@c@(@type@ x, @type@ y)
{
int are_inputs_inf = (npy_isinf(x) && npy_isinf(y));
- /* force set invalid flag, doesnt raise by default on gcc < 8 */
- if (npy_isnan(x) || npy_isnan(y)) {
- npy_set_floatstatus_invalid();
- }
+
if (are_inputs_inf || !y) {
if (!npy_isnan(x)) {
npy_set_floatstatus_invalid();
@@ -715,10 +712,6 @@ npy_divmod@c@(@type@ a, @type@ b, @type@ *modulus)
{
@type@ div, mod, floordiv;
- /* force set invalid flag, doesnt raise by default on gcc < 8 */
- if (npy_isnan(a) || npy_isnan(b)) {
- npy_set_floatstatus_invalid();
- }
mod = npy_fmod@c@(a, b);
if (NPY_UNLIKELY(!b)) {
div = a / b;
@@ -735,7 +728,7 @@ npy_divmod@c@(@type@ a, @type@ b, @type@ *modulus)
/* adjust fmod result to conform to Python convention of remainder */
if (mod) {
- if ((b < 0) != (mod < 0)) {
+ if ((b < 0) != (isless(mod, 0))) {
mod += b;
div -= 1.0@c@;
}@arubiales are you still up for digging into this more? That would be awesome. I think it needs some more thought and diligence , e.g. with respect to |
|
@seberg Yes of course. It's my first contribution to Numpy and after all the hours I want to finish the job properly. I didn't enter in the I have done similar changes to the file to avoid the warning and I get to avoid with NaN as a second input. Right now I will do:
Thank you very much for your help. |
|
Those are different bugs in clang and apple M1 and unrelated to this issue, there are open issues and even PRs for it. |
Got it. Thanks for pointing it out. Let me pull down my previous comment that diverted from the original discussion. |
|
Hi @seberg sorry for the delay but I have been very busy. Is just to inform that I'm working on it. Here are the changes I did: diff --git a/numpy/core/src/npymath/npy_math_internal.h.src b/numpy/core/src/npymath/npy_math_internal.h.src
index ff4663dc3..ee22d72ea 100644
--- a/numpy/core/src/npymath/npy_math_internal.h.src
+++ b/numpy/core/src/npymath/npy_math_internal.h.src
@@ -423,16 +423,6 @@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x, @type@ y)
NPY_INPLACE @type@
npy_@kind@@c@(@type@ x, @type@ y)
{
- int are_inputs_inf = (npy_isinf(x) && npy_isinf(y));
- /* force set invalid flag, doesnt raise by default on gcc < 8 */
- if (npy_isnan(x) || npy_isnan(y)) {
- npy_set_floatstatus_invalid();
- }
- if (are_inputs_inf || !y) {
- if (!npy_isnan(x)) {
- npy_set_floatstatus_invalid();
- }
- }
return (@type@) npy_@kind@((double)x, (double) y);
}
#endif
@@ -518,16 +508,7 @@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x, @type@ y)
NPY_INPLACE @type@
npy_@kind@@c@(@type@ x, @type@ y)
{
- int are_inputs_inf = (npy_isinf(x) && npy_isinf(y));
- /* force set invalid flag, doesnt raise by default on gcc < 8 */
- if (npy_isnan(x) || npy_isnan(y)) {
- npy_set_floatstatus_invalid();
- }
- if (are_inputs_inf || !y) {
- if (!npy_isnan(x)) {- if ((b < 0) != (mod < 0)) {
+ if ((b < 0) != (isless(mod, 0))) {
- npy_set_floatstatus_invalid();
- }
- }
+
return @kind@@c@(x, y);
}
#endif
@@ -716,9 +697,6 @@ npy_divmod@c@(@type@ a, @type@ b, @type@ *modulus)
@type@ div, mod, floordiv;
/* force set invalid flag, doesnt raise by default on gcc < 8 */
- if (npy_isnan(a) || npy_isnan(b)) {
- npy_set_floatstatus_invalid();
- }
mod = npy_fmod@c@(a, b);
if (NPY_UNLIKELY(!b)) {
div = a / b;I did't make your last change (this one below) because in my knowledge both lines are the same: - if ((b < 0) != (mod < 0)) {
+ if ((b < 0) != (isless(mod, 0))) {After this changes I run a very simple file to test if warnings appears And the warnings only appears in the last two sections, when the infinite is in the left and when both numbers are infinite. Right now I'm a bit stuck so any help is welcome. Thanks. |
|
@arubiales I am not sure what your question is? The last two sections should definitely give a warning, since a new NaN is created (inf doesn't count in that regard). So that seems all fine. For example |
|
@seberg Yes a new NaN is created but if you do
Only gives the warning when you use So we solved the initial issue that was the warning with If you think is not necessary to change the |
If you try The behaviour you listed seems all correct to me, yes, so going with PR is probably best to continue discussion. |
|
Thanks @arubiales and @seberg! |
Uh oh!
There was an error while loading. Please reload this page.
The following operations correctly return
nanwithout warning:So do
Taking
%, however, does give a warning:This inconsistency extends to all numpy float types.
The text was updated successfully, but these errors were encountered: