Fix complex sin and cos on inputs with small absolute value or large pure imaginary part #19823
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pearu
force-pushed
the
pearu/sin
branch
5 times, most recently
from
aabca27
to
82adfb4
Compare
|
@pearu It's definitely worth also fixing this upstream, although as you note that can be done in another PR (and then we can remove the custom lowering in JAX). |
jakevdp
reviewed
Feb 15, 2024
jakevdp
reviewed
Feb 15, 2024
jakevdp
requested changes
Feb 20, 2024
pearu
force-pushed
the
pearu/sin
branch
2 times, most recently
from
c97526b
to
3298dcb
Compare
jakevdp
reviewed
Feb 20, 2024
pearu
force-pushed
the
pearu/sin
branch
6 times, most recently
from
a8571b3
to
3a75d41
Compare
jakevdp
approved these changes
Feb 22, 2024
google-ml-butler
bot
added
kokoro:force-run
pull ready
jakevdp
approved these changes
Feb 22, 2024
jakevdp
reviewed
Feb 22, 2024
jakevdp
approved these changes
Feb 22, 2024
…pure imaginary part
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As in the title.
In addition, a test
testUfuncOnComplexPlaneis introduced that validates jax implementations of complex ufuncs on the whole complex plane using numpy ufuncs as references.Fixes #19753, #19754
In terms of the Validate complex functions in array libraries tool, the validity state of the sin function on complex plane is
before and after this PR.
Discussion: ideally, issues with sin/cos ought to be fixed upstream (https://github.com/openxla/stablehlo). This PR implements lowerings for complex sin and cos functions because it is the fastest way to enable the fix for the end-users.