Skip to content

BUG: Fix error with 180 degree rotation in Rotation.align_vectors() with an infinite weight#20573

Merged
nmayorov merged 4 commits intoscipy:mainfrom
scottshambaugh:align_vectors_bug
Apr 28, 2024
Merged

BUG: Fix error with 180 degree rotation in Rotation.align_vectors() with an infinite weight#20573
nmayorov merged 4 commits intoscipy:mainfrom
scottshambaugh:align_vectors_bug

Conversation

@scottshambaugh
Copy link
Copy Markdown
Contributor

@scottshambaugh scottshambaugh commented Apr 24, 2024

Reference issue

Closes #20555

What does this implement/fix?

There are two related issues with exact rotations near 180 degrees:

  • A perfect 180 deg rotation results in r = cross = [0, 0, 0], and is thus cast to a 0 degree rotation (critical defect)
  • Numerical instabilities near a 180 deg rotation (non-critical defect)

This fixes both. For the numerical instability part, near 180 degrees for the test case I'm seeing a pointing error of 2e-6 drop to 5e-9 (and now matches the non-exact code path to 1e-11). Tried to make the code path as compact as possible, but improvements welcome!

@github-actions github-actions bot added scipy.spatial Cython Issues with the internal Cython code base defect A clear bug or issue that prevents SciPy from being installed or used as expected labels Apr 24, 2024
@j-bowhay j-bowhay requested a review from nmayorov April 24, 2024 15:19
@scottshambaugh scottshambaugh marked this pull request as draft April 24, 2024 15:55
@scottshambaugh scottshambaugh marked this pull request as ready for review April 24, 2024 19:30
@scottshambaugh scottshambaugh force-pushed the align_vectors_bug branch 6 times, most recently from e6fd5c0 to 05bd338 Compare April 25, 2024 18:35
@nmayorov nmayorov merged commit 701d8da into scipy:main Apr 28, 2024
@nmayorov
Copy link
Copy Markdown
Contributor

Solid fix @scottshambaugh!

Is it necessary to mention bug fixes in the release notes? I don't see a dedicated section for that here.

@tylerjereddy
Copy link
Copy Markdown
Contributor

Is it necessary to mention bug fixes in the release notes?

nope

@tylerjereddy tylerjereddy added this to the 1.14.0 milestone Apr 28, 2024
@tylerjereddy tylerjereddy added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Apr 28, 2024
@scottshambaugh scottshambaugh deleted the align_vectors_bug branch April 29, 2024 01:41
@tylerjereddy tylerjereddy modified the milestones: 1.14.0, 1.13.1 May 2, 2024
tylerjereddy pushed a commit to tylerjereddy/scipy that referenced this pull request May 2, 2024
…ith an infinite weight (scipy#20573)

* Fix exact rotations at 180 deg, improve near 180 deg

Comments

* Tests for exact near 180 deg rotations

* Fix tests

* Code review updates

---------

Co-authored-by: Scott Shambaugh <scottshambaugh@users.noreply.github.com>
@tylerjereddy tylerjereddy mentioned this pull request May 2, 2024
10 tasks
@scottshambaugh
Copy link
Copy Markdown
Contributor Author

scottshambaugh commented May 14, 2024

Note that this also affected antiparallel single vector pairs as in #20660, since those use the infinite weight code path, but that should be covered under this fix.

# For the special case of a single vector pair, we use the infinite
# weight code path
if N == 1:
weight_is_inf = np.array([True])

@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cython Issues with the internal Cython code base defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.spatial

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: spatial: error in Rotation.align_vectors() with an infinite weight

3 participants