Skip to content

Conversation

mdhaber
Copy link
Contributor

@mdhaber mdhaber commented Sep 24, 2024

Reference issue

Closes gh-21610

What does this implement/fix?

gh-21610 noted that logsumexp could return imaginary components outside the range (-pi, pi] after the changes in gh-21597. This PR enforces the original branch cut convention and documents it.

@mdhaber mdhaber added scipy.special maintenance Items related to regular maintenance tasks labels Sep 24, 2024
@github-actions github-actions bot added maintenance Items related to regular maintenance tasks and removed maintenance Items related to regular maintenance tasks labels Sep 24, 2024
# outside the range (-pi, pi]. Check that this is resolved.
# While working on this, I noticed that all other tests passed even
# when the imaginary component of the result was zero. This suggested
# the need of a stronger test with imaginary dtype.
Copy link
Contributor Author

@mdhaber mdhaber Sep 24, 2024

Choose a reason for hiding this comment

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

Would be good to include a test with b, or maybe just a separate property-based test of the basic behavior for all input types and options. If this looks good as-is, though, let's make that a separate PR since it's not urgent.

Copy link
Contributor

@steppi steppi left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @mdhaber

@steppi steppi merged commit 2ee2fc5 into scipy:main Sep 30, 2024
40 checks passed
@lucascolley lucascolley added this to the 1.15.0 milestone Sep 30, 2024

def _wrap_radians(x):
# Wrap radians to -pi, pi interval
return (x + math.pi) % (2 * math.pi) - math.pi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@steppi it occurs to me that this would lose precision near $x = 0$. The alternative, of course, is lazywhere. Do we want to do that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.special
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: special.logsumexp: imaginary component exceeds (-pi, pi]
3 participants