xla
xla copied to clipboard
openxla
Improve the accuracy of asin(x) and asinh(x) for complex x using modified Hull et al algorithm.
As in the title.
~Fixes https://github.com/openxla/xla/issues/8553~ - PR https://github.com/openxla/xla/pull/9802 disabled the fix.
Update: the fix to https://github.com/openxla/xla/issues/8553 will be available via https://github.com/openxla/stablehlo/pull/2357
Hi @pearu , any update on this PR, can you provide the information requested ? Thanks.
Hi @pearu , gentle reminder! Thanks.
@kamaljeeti , thanks for the reminder! The reviewers request for the regression test has been addressed via https://github.com/google/jax/pull/19823 . This PR can be updated after https://github.com/google/jax/pull/20144 has landed.
@pearu https://github.com/google/jax/pull/20144 has landed. Could you please help update the PR? :)
@pearu google/jax#20144 has landed. Could you please help update the PR? :)
@penpornk, certainly :) Meanwhile, https://github.com/openxla/xla/pull/10503 introduced a testing framework for functions with complex inputs in the XLA side. We should use this framework in this PR for asinh as well.
Thank you for the heads up! https://github.com/openxla/xla/pull/10503 is now merged. (Many thanks to @reedwm!)
Hi @ddunl , there are a couple of checks failed on the Cl, can you have a look at them once? Thanks.
While using asin from asinh for complex inputs is the right way to go (as implemented in this PR), asin itself on complex inputs has very poor accuracy. I am currently implementing complex asin based on https://dl.acm.org/doi/10.1145/275323.275324 and then update this PR accordingly. Meanwhile, I'll mark the PR as a draft.
As it turns out, asin/asinh improvements in xla/client/lib/math.cc will not be available for JAX. Therefore, I am currently working on submitting the modified Hull algorithm to StableHLO so that asin/asinh fixes here will be eventually effective also for JAX.
So, this PR should be considered as an improvement to asin/asinh in xla/client math lib that has no effect to the asin/asinh state in JAX.
https://github.com/openxla/stablehlo/pull/2357 implements the modified Hull et al algorithm for StableHLO.
The asin implementations for both XLA and StableHLO are now generated using https://github.com/pearu/functional_algorithms tool that includes the validation of the algorithm against mpmath with the correctness of ULP difference <= 3. While in this PR we are still using tolerance conditions for measuring the difference of asin results and its reference values, using ULP difference is better measure because it does not depend on the magnitude of compared values. https://github.com/openxla/stablehlo/pull/2357 uses ULP difference, doing the same in XLA is planned as a follow-up.
@hawkinsp @jakevdp Could you please have a look at this PR and approve if it looks good to you?
@jakevdp Could you please help take another look at this PR? @pearu has addressed your initial comments.
Also, @GleasonK, would it make sense to request reviews from people who have reviewed https://github.com/openxla/stablehlo/pull/2357 ?
I see that the function "static XlaOp Hypot(XlaOp x, XlaOp y)". Is it intended? if it is not, could you please remove it?
