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.

Sign up for GitHub

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

Jump to bottom

Fix acos op decomposition for non complex numbers to be more accurate #2452

Open
GleasonK opened this issue Jul 22, 2024 · 0 comments
Open

Fix acos op decomposition for non complex numbers to be more accurate #2452

GleasonK opened this issue Jul 22, 2024 · 0 comments
Assignees
@pearu
Labels
CHLO

Comments

@GleasonK
Copy link
Member

What happened?

An initial fix was made for accuracy but was reverted due to a bug in the pattern generator:
#2449

This ticket tracks fixing that work and submitting a more accurate decomposition for acos op.

Current:
// Expand acos for non-complex arguments to MHLO dialect as follows:
//   acos(x) = 2 * atan2(sqrt(1 - x^2), (1 + x))  if x != -1

Using 1-x^2 will be inaccurate when abs(x) is close to 1. The corresponding cancellation errors are avoided when using (1-x)*(1+x).

Steps to reproduce your issue

No response

Version information

No response
@GleasonK GleasonK added the CHLO label Jul 22, 2024
@GleasonK GleasonK mentioned this issue Jul 22, 2024
Merged
@pearu pearu mentioned this issue Jul 30, 2024
Merged
@pearu pearu mentioned this issue Aug 17, 2024
Merged
GleasonK pushed a commit that referenced this issue Aug 21, 2024
@pearu
Refactor complex asin/acos/asinh/acosh to use asin_acos_kernel. Impro…
f8fed84 
…ve real acos accuracy. (#2496)

As in the title.

This PR introduces asin_acos_kernel operation that implements modified
Hull et al algorithm for evaluating complex asin, acos, asinh, and acosh
operations. This task corresponds to the refactor request in
#2411 (comment)
and it resolves
pearu/functional_algorithms#15.

In addition, the PR also improves the real acos accuracy as a fix to
#2452 . As a result, the
accuracy of single-precision real acos improved as follows (using 1
million samples):
```
                             main       this PR
ULP difference == 0 count is 971713  -> 999045
ULP difference == 1 count is 28158   ->    962
ULP difference == 2 count is 54      ->      0
ULP difference == 3 count is 20      ->      0
ULP difference >= 4 count is 62      ->      0
```


The cause of the revert by PR
#2449 (`acos(-1) -> 0` while
expecting `pi`) is fixed in functional_algorithms which now also
generates extra samples that correspond to limiting values of real acos.
For instance, `acos(-1)->pi`, `acos(nextafter(-1, -inf))->nan`, and
`acos(nextafter(-1, int)-> approx pi` are now included in the tests.


The required functional_algorithms version is now 0.9 which includes a
fix to `fa.utils.real_samples` to return samples that are distributed
uniformly with respect to ULP differences of neighboring samples. This
fix lead to updates of `stablehlo/tests/math/*.mlir`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pearu pearu

Labels
CHLO
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pearu @GleasonK