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.

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

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

Merged
merged 4 commits into from Apr 28, 2024

Conversation

scottshambaugh
Copy link
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 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
29 of 30 checks passed
@nmayorov
Copy link
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
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-candidate This fix should be ported by a maintainer to previous SciPy versions. 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