From 1823b55e389fd991fee0d1661d68404defa18ab8 Mon Sep 17 00:00:00 2001 From: rubiales Date: Tue, 22 Jun 2021 19:51:59 +0200 Subject: [PATCH 1/7] solve the problems of mod operator except for infinite values #18170 --- .../core/src/npymath/npy_math_internal.h.src | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/numpy/core/src/npymath/npy_math_internal.h.src b/numpy/core/src/npymath/npy_math_internal.h.src index ff4663dc3e50..ee22d72ea543 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)) { - 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; From 963caf0bf1d2b23efb60109b8c909b3078359cf6 Mon Sep 17 00:00:00 2001 From: rubiales Date: Thu, 24 Jun 2021 00:01:02 +0200 Subject: [PATCH 2/7] Solved test fails due to the warnings changed #18170 --- numpy/core/src/npymath/npy_math_internal.h.src | 1 - numpy/core/tests/test_umath.py | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/numpy/core/src/npymath/npy_math_internal.h.src b/numpy/core/src/npymath/npy_math_internal.h.src index ee22d72ea543..c22ac5186afd 100644 --- a/numpy/core/src/npymath/npy_math_internal.h.src +++ b/numpy/core/src/npymath/npy_math_internal.h.src @@ -508,7 +508,6 @@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x, @type@ y) NPY_INPLACE @type@ npy_@kind@@c@(@type@ x, @type@ y) { - return @kind@@c@(x, y); } #endif diff --git a/numpy/core/tests/test_umath.py b/numpy/core/tests/test_umath.py index 6fd4b4659314..6d97c640f578 100644 --- a/numpy/core/tests/test_umath.py +++ b/numpy/core/tests/test_umath.py @@ -459,8 +459,8 @@ def test_floor_division_errors(self, dtype): with np.errstate(divide='raise', invalid='ignore'): assert_raises(FloatingPointError, np.floor_divide, fone, fzer) with np.errstate(invalid='raise'): - assert_raises(FloatingPointError, np.floor_divide, fnan, fone) - assert_raises(FloatingPointError, np.floor_divide, fone, fnan) + assert_no_warnings(FloatingPointError, np.floor_divide, fnan, fone) + assert_no_warnings(FloatingPointError, np.floor_divide, fone, fnan) assert_raises(FloatingPointError, np.floor_divide, fnan, fzer) @pytest.mark.parametrize('dtype', np.typecodes['Float']) @@ -589,9 +589,9 @@ def test_float_remainder_errors(self, dtype, fn): fnan = np.array(np.nan, dtype=dtype) with np.errstate(invalid='raise'): assert_raises(FloatingPointError, fn, fone, fzero) - assert_raises(FloatingPointError, fn, fnan, fzero) - assert_raises(FloatingPointError, fn, fone, fnan) - assert_raises(FloatingPointError, fn, fnan, fone) + assert_no_warnings(FloatingPointError, fn, fnan, fzero) + assert_no_warnings(FloatingPointError, fn, fone, fnan) + assert_no_warnings(FloatingPointError, fn, fnan, fone) def test_float_remainder_overflow(self): a = np.finfo(np.float64).tiny From 8c67e22bd9b6152d1bddeb5db9ac2ea562581297 Mon Sep 17 00:00:00 2001 From: rubiales Date: Fri, 25 Jun 2021 19:53:48 +0200 Subject: [PATCH 3/7] Fix 1 test_umath and npy_math_internal --- numpy/core/src/npymath/npy_math_internal.h.src | 14 ++++++++++++++ numpy/core/tests/test_umath.py | 4 +++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/numpy/core/src/npymath/npy_math_internal.h.src b/numpy/core/src/npymath/npy_math_internal.h.src index c22ac5186afd..14892562c1a6 100644 --- a/numpy/core/src/npymath/npy_math_internal.h.src +++ b/numpy/core/src/npymath/npy_math_internal.h.src @@ -423,6 +423,13 @@ 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)); + + if (are_inputs_inf || !y) { + if (!npy_isnan(x)) { + npy_set_floatstatus_invalid(); + } + } return (@type@) npy_@kind@((double)x, (double) y); } #endif @@ -508,6 +515,13 @@ 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)); + + if (are_inputs_inf || !y) { + if (!npy_isnan(x)) { + npy_set_floatstatus_invalid(); + } + } return @kind@@c@(x, y); } #endif diff --git a/numpy/core/tests/test_umath.py b/numpy/core/tests/test_umath.py index 6d97c640f578..64357974b483 100644 --- a/numpy/core/tests/test_umath.py +++ b/numpy/core/tests/test_umath.py @@ -461,7 +461,8 @@ def test_floor_division_errors(self, dtype): with np.errstate(invalid='raise'): assert_no_warnings(FloatingPointError, np.floor_divide, fnan, fone) assert_no_warnings(FloatingPointError, np.floor_divide, fone, fnan) - assert_raises(FloatingPointError, np.floor_divide, fnan, fzer) + assert_no_warnings(FloatingPointError, np.floor_divide, fnan, fzer) + assert_no_warnings(FloatingPointError, np.floor_divide, fzer, fnan) @pytest.mark.parametrize('dtype', np.typecodes['Float']) def test_floor_division_corner_cases(self, dtype): @@ -590,6 +591,7 @@ def test_float_remainder_errors(self, dtype, fn): with np.errstate(invalid='raise'): assert_raises(FloatingPointError, fn, fone, fzero) assert_no_warnings(FloatingPointError, fn, fnan, fzero) + assert_no_warnings(FloatingPointError, fn, fzero, fnan) assert_no_warnings(FloatingPointError, fn, fone, fnan) assert_no_warnings(FloatingPointError, fn, fnan, fone) From 8547c5cafae7f2d8bb9209a7ebf2dc508e6f9b62 Mon Sep 17 00:00:00 2001 From: rubiales Date: Sun, 4 Jul 2021 20:32:05 +0200 Subject: [PATCH 4/7] delete comment about warning --- numpy/core/src/npymath/npy_math_internal.h.src | 1 - 1 file changed, 1 deletion(-) diff --git a/numpy/core/src/npymath/npy_math_internal.h.src b/numpy/core/src/npymath/npy_math_internal.h.src index 14892562c1a6..c4b8af350ba0 100644 --- a/numpy/core/src/npymath/npy_math_internal.h.src +++ b/numpy/core/src/npymath/npy_math_internal.h.src @@ -709,7 +709,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 */ mod = npy_fmod@c@(a, b); if (NPY_UNLIKELY(!b)) { div = a / b; From 6d25b0efc6db2b13b9b682029be3b880202ae06a Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Tue, 6 Jul 2021 11:18:28 -0500 Subject: [PATCH 5/7] TST: Change tests to more strictly (correctly) check for errors The old tests seems not to have noticed all types of errors here. It seems the `assert_no_warnings` context does not care about exceptions. --- numpy/core/tests/test_umath.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/numpy/core/tests/test_umath.py b/numpy/core/tests/test_umath.py index 64357974b483..d22bf54febdf 100644 --- a/numpy/core/tests/test_umath.py +++ b/numpy/core/tests/test_umath.py @@ -458,11 +458,13 @@ def test_floor_division_errors(self, dtype): # divide by zero error check with np.errstate(divide='raise', invalid='ignore'): assert_raises(FloatingPointError, np.floor_divide, fone, fzer) - with np.errstate(invalid='raise'): - assert_no_warnings(FloatingPointError, np.floor_divide, fnan, fone) - assert_no_warnings(FloatingPointError, np.floor_divide, fone, fnan) - assert_no_warnings(FloatingPointError, np.floor_divide, fnan, fzer) - assert_no_warnings(FloatingPointError, np.floor_divide, fzer, fnan) + + # The following already contain a NaN and should not warn + with np.errstate(all='raise'): + np.floor_divide(fnan, fone) + np.floor_divide(fone, fnan) + np.floor_divide(fnan, fzer) + np.floor_divide(fzer, fnan) @pytest.mark.parametrize('dtype', np.typecodes['Float']) def test_floor_division_corner_cases(self, dtype): @@ -588,12 +590,16 @@ def test_float_remainder_errors(self, dtype, fn): fone = np.array(1.0, dtype=dtype) finf = np.array(np.inf, dtype=dtype) fnan = np.array(np.nan, dtype=dtype) - with np.errstate(invalid='raise'): - assert_raises(FloatingPointError, fn, fone, fzero) - assert_no_warnings(FloatingPointError, fn, fnan, fzero) - assert_no_warnings(FloatingPointError, fn, fzero, fnan) - assert_no_warnings(FloatingPointError, fn, fone, fnan) - assert_no_warnings(FloatingPointError, fn, fnan, fone) + + # The following already contain a NaN and should not warn. + with np.errstate(all='raise'): + with pytest.raises(FloatingPointError, + match="invalid value"): + fn(fone, fzero) + fn(fnan, fzero) + fn(fzero, fnan) + fn(fone, fnan) + fn(fnan, fone) def test_float_remainder_overflow(self): a = np.finfo(np.float64).tiny From e67c9c9f2897312344e98a3d76754d543eaaebde Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Tue, 6 Jul 2021 12:51:05 -0500 Subject: [PATCH 6/7] BUG: Fix floating point error flags in division related ops --- .../core/src/npymath/npy_math_internal.h.src | 84 +++++-------------- numpy/core/src/umath/scalarmath.c.src | 10 +-- numpy/core/tests/test_umath.py | 5 +- 3 files changed, 28 insertions(+), 71 deletions(-) diff --git a/numpy/core/src/npymath/npy_math_internal.h.src b/numpy/core/src/npymath/npy_math_internal.h.src index c4b8af350ba0..1e46a23031fe 100644 --- a/numpy/core/src/npymath/npy_math_internal.h.src +++ b/numpy/core/src/npymath/npy_math_internal.h.src @@ -398,8 +398,8 @@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x) /**end repeat1**/ /**begin repeat1 - * #kind = atan2,hypot,pow,copysign# - * #KIND = ATAN2,HYPOT,POW,COPYSIGN# + * #kind = atan2,hypot,pow,fmod,copysign# + * #KIND = ATAN2,HYPOT,POW,FMOD,COPYSIGN# */ #ifdef @kind@@c@ #undef @kind@@c@ @@ -412,29 +412,6 @@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x, @type@ y) #endif /**end repeat1**/ -/**begin repeat1 - * #kind = fmod# - * #KIND = FMOD# - */ -#ifdef @kind@@c@ -#undef @kind@@c@ -#endif -#ifndef HAVE_MODF@C@ -NPY_INPLACE @type@ -npy_@kind@@c@(@type@ x, @type@ y) -{ - int are_inputs_inf = (npy_isinf(x) && npy_isinf(y)); - - if (are_inputs_inf || !y) { - if (!npy_isnan(x)) { - npy_set_floatstatus_invalid(); - } - } - return (@type@) npy_@kind@((double)x, (double) y); -} -#endif -/**end repeat1**/ - #ifdef modf@c@ #undef modf@c@ #endif @@ -496,8 +473,8 @@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x) /**end repeat1**/ /**begin repeat1 - * #kind = atan2,hypot,pow,copysign# - * #KIND = ATAN2,HYPOT,POW,COPYSIGN# + * #kind = atan2,hypot,pow,fmod,copysign# + * #KIND = ATAN2,HYPOT,POW,FMOD,COPYSIGN# */ #ifdef HAVE_@KIND@@C@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x, @type@ y) @@ -507,26 +484,6 @@ NPY_INPLACE @type@ npy_@kind@@c@(@type@ x, @type@ y) #endif /**end repeat1**/ -/**begin repeat1 - * #kind = fmod# - * #KIND = FMOD# - */ -#ifdef HAVE_FMOD@C@ -NPY_INPLACE @type@ -npy_@kind@@c@(@type@ x, @type@ y) -{ - int are_inputs_inf = (npy_isinf(x) && npy_isinf(y)); - - if (are_inputs_inf || !y) { - if (!npy_isnan(x)) { - npy_set_floatstatus_invalid(); - } - } - return @kind@@c@(x, y); -} -#endif -/**end repeat1**/ - #ifdef HAVE_MODF@C@ NPY_INPLACE @type@ npy_modf@c@(@type@ x, @type@ *iptr) { @@ -676,8 +633,14 @@ npy_remainder@c@(@type@ a, @type@ b) { @type@ mod; if (NPY_UNLIKELY(!b)) { + /* + * in2 == 0 (and not NaN): normal fmod will give the correct + * result (always NaN). `divmod` may set additional FPE for the + * division by zero creating an inf. + */ mod = npy_fmod@c@(a, b); - } else { + } + else { npy_divmod@c@(a, b, &mod); } return mod; @@ -687,13 +650,14 @@ NPY_INPLACE @type@ npy_floor_divide@c@(@type@ a, @type@ b) { @type@ div, mod; if (NPY_UNLIKELY(!b)) { + /* + * in2 == 0 (and not NaN): normal division will give the correct + * result (Inf or NaN). `divmod` may set additional FPE for the modulo + * evaluating to NaN. + */ div = a / b; - if (!a || npy_isnan(a)) { - npy_set_floatstatus_invalid(); - } else { - npy_set_floatstatus_divbyzero(); - } - } else { + } + else { div = npy_divmod@c@(a, b, &mod); } return div; @@ -711,13 +675,9 @@ npy_divmod@c@(@type@ a, @type@ b, @type@ *modulus) mod = npy_fmod@c@(a, b); if (NPY_UNLIKELY(!b)) { - div = a / b; - if (a && !npy_isnan(a)) { - npy_set_floatstatus_divbyzero(); - } - /* If b == 0, return result of fmod. For IEEE is nan */ + /* b == 0 (not NaN): return result of fmod. For IEEE is nan */ *modulus = mod; - return div; + return a / b; } /* a - mod should be very nearly an integer multiple of b */ @@ -725,7 +685,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 (isless(b, 0) != isless(mod, 0)) { mod += b; div -= 1.0@c@; } @@ -738,7 +698,7 @@ npy_divmod@c@(@type@ a, @type@ b, @type@ *modulus) /* snap quotient to nearest integral value */ if (div) { floordiv = npy_floor@c@(div); - if (div - floordiv > 0.5@c@) + if (isgreater(div - floordiv, 0.5@c@)) floordiv += 1.0@c@; } else { diff --git a/numpy/core/src/umath/scalarmath.c.src b/numpy/core/src/umath/scalarmath.c.src index 66f97a831431..5836545f8942 100644 --- a/numpy/core/src/umath/scalarmath.c.src +++ b/numpy/core/src/umath/scalarmath.c.src @@ -283,19 +283,13 @@ static void static void @name@_ctype_floor_divide(@type@ a, @type@ b, @type@ *out) { - @type@ mod; - - if (!b) { - *out = a / b; - } else { - *out = npy_divmod@c@(a, b, &mod); - } + *out = npy_floor_divide@c@(a, b); } static void @name@_ctype_remainder(@type@ a, @type@ b, @type@ *out) { - npy_divmod@c@(a, b, out); + *out = npy_remainder@c@(a, b); } diff --git a/numpy/core/tests/test_umath.py b/numpy/core/tests/test_umath.py index d22bf54febdf..de4784051704 100644 --- a/numpy/core/tests/test_umath.py +++ b/numpy/core/tests/test_umath.py @@ -458,6 +458,8 @@ def test_floor_division_errors(self, dtype): # divide by zero error check with np.errstate(divide='raise', invalid='ignore'): assert_raises(FloatingPointError, np.floor_divide, fone, fzer) + with np.errstate(divide='ignore', invalid='raise'): + np.floor_divide(fone, fzer) # The following already contain a NaN and should not warn with np.errstate(all='raise'): @@ -581,7 +583,8 @@ def test_float_divmod_errors(self, dtype): with np.errstate(divide='ignore', invalid='raise'): assert_raises(FloatingPointError, np.divmod, finf, fzero) with np.errstate(divide='raise', invalid='ignore'): - assert_raises(FloatingPointError, np.divmod, finf, fzero) + # inf / 0 does not set any flags, only the modulo creates a NaN + np.divmod(finf, fzero) @pytest.mark.parametrize('dtype', np.typecodes['Float']) @pytest.mark.parametrize('fn', [np.fmod, np.remainder]) From e502de26b8c84802e224e0bad66f9d179b51eab9 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Mon, 12 Jul 2021 15:40:33 -0500 Subject: [PATCH 7/7] TST: Ignore floating point warning tests on MacOS It is very much possible that this will also fail on other systems and should rather only run, e.g. on newer glibc versions. In principle, we could also split this into "no warnings given" and "warning given". MacOS seems very sloppy with floating point warnings, but hopefully it at least does not give spurious ones. --- numpy/core/tests/test_umath.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/numpy/core/tests/test_umath.py b/numpy/core/tests/test_umath.py index de4784051704..ebd61d199612 100644 --- a/numpy/core/tests/test_umath.py +++ b/numpy/core/tests/test_umath.py @@ -563,6 +563,9 @@ def test_float_remainder_roundoff(self): else: assert_(b > rem >= 0, msg) + @pytest.mark.xfail(sys.platform.startswith("darwin"), + reason="MacOS seems to not give the correct 'invalid' warning for " + "`fmod`. Hopefully, others always do.") @pytest.mark.parametrize('dtype', np.typecodes['Float']) def test_float_divmod_errors(self, dtype): # Check valid errors raised for divmod and remainder @@ -586,6 +589,9 @@ def test_float_divmod_errors(self, dtype): # inf / 0 does not set any flags, only the modulo creates a NaN np.divmod(finf, fzero) + @pytest.mark.xfail(sys.platform.startswith("darwin"), + reason="MacOS seems to not give the correct 'invalid' warning for " + "`fmod`. Hopefully, others always do.") @pytest.mark.parametrize('dtype', np.typecodes['Float']) @pytest.mark.parametrize('fn', [np.fmod, np.remainder]) def test_float_remainder_errors(self, dtype, fn):