Skip to content
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

Fix failing unit test in Python 3.13 #288

Merged
merged 7 commits into from
Jan 31, 2025
Merged

Fix failing unit test in Python 3.13 #288

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e1d446d
Fix failing unit test in Python 3.13
mcara Jan 30, 2025
550a3ed
add change log entry
mcara Jan 30, 2025
194219f
further fixes
mcara Jan 30, 2025
c4454f7
ignore fp errors in dot_qd
mcara Jan 30, 2025
0438ec3
restore old test check
mcara Jan 30, 2025
30ae26b
cleanup
mcara Jan 30, 2025
c46e72b
add a code comment for PyUFunc_clearfperr
mcara Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
Release Notes
=============

1.3.3 (30-January-2025)
=======================

- Fixed a bug in the C implementation of ``dot_qd`` that may result in
"invalid value encountered in length" runtime warning when input vectors
contain NaN values. [#288]


1.3.2 (12-June-2024)
====================

Expand All @@ -16,6 +24,7 @@ Release Notes

- Build wheel with Numpy 2.0 release candidate ahead of Numpy 2.0 release [#279]


1.3.1 (5-November-2023)
=======================

Expand Down
24 changes: 22 additions & 2 deletions spherical_geometry/tests/test_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,28 @@ def test_math_util_angle_domain():


def test_math_util_length_domain():
with pytest.raises(ValueError):
great_circle_arc.length([[np.nan, 0, 0]], [[0, 0, np.inf]])
a = [
[np.nan, 0, 0],
[np.nan, 0, 0],
[np.nan, np.nan, np.nan],
[0, 0, 0],
[0, 0, 0],
[0, 0, np.nan],
[0, 0, 0],
]
b = [
[0, 0, np.inf],
[0, 0, np.inf],
[0, 0, 0],
[np.inf, np.inf, np.inf],
[0, 0, np.inf],
[0, 0, 0],
[0, 0, 0],
]

v = great_circle_arc.length(a, b)

assert np.all(np.logical_not(np.isfinite(v)))
Copy link
Contributor

Choose a reason for hiding this comment

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

To check I'm understanding. Previously providing np.nan to length resulted in ValueError: Out of domain for acos and with this PR it doesn't raise an exception and instead returns np.nan?

Where is length used and will this change in behavior have consequences?



def test_math_util_angle_nearly_coplanar_vec():
Expand Down
21 changes: 17 additions & 4 deletions src/math_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ normalize_qd(const qd *A, qd *B) {
double l[4];

for (i = 0; i < 3; ++i) {
if (ISNAN_QD(A[i])) {
c_qd_copy_d(NPY_NAN, B->x);
return 1;
}

c_qd_sqr(A[i].x, T[i]);
}

Expand Down Expand Up @@ -176,6 +181,10 @@ dot_qd(const qd *A, const qd *B, qd *C) {
double tmp[4][4];

for (i = 0; i < 3; ++i) {
if (ISNAN_QD(A[i]) || ISNAN_QD(B[i])) {
c_qd_copy_d(NPY_NAN, C->x);
return;
}
c_qd_mul(A[i].x, B[i].x, tmp[i]);
}

Expand Down Expand Up @@ -646,10 +655,14 @@ DOUBLE_length(char **args, const intp *dimensions, const intp *steps, void *NPY_
load_point_qd(ip1, is1, A);
load_point_qd(ip2, is2, B);

if (normalize_qd(A, A)) return;
if (normalize_qd(B, B)) return;

if (length_qd(A, B, &s)) return;
if (normalize_qd(A, A) || normalize_qd(B, B) || length_qd(A, B, &s)) {
#if defined(NAN)
*((double *)op) = NAN;
#else
*((double *)op) = strtod("NaN", NULL);
#endif
continue;
}

*((double *)op) = s.x[0];
END_OUTER_LOOP
Expand Down
Loading