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

sinpi: insufficiently precise reference result #860

Open
hvdijk opened this issue Feb 2, 2024 · 2 comments
Open

sinpi: insufficiently precise reference result #860

hvdijk opened this issue Feb 2, 2024 · 2 comments

Comments

@hvdijk
Copy link
Contributor

hvdijk commented Feb 2, 2024

SYCL-CTS shows an error when using DPC++ with our OpenCL implementation.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The SYCL 2020 Conformance Test Suite is a Catch2 v3.2.1 host application.
Run with -? for options

-------------------------------------------------------------------------------
math_builtin_float_double
-------------------------------------------------------------------------------
/home/harald/SYCL-CTS/tests/math_builtin_api/../common/../../util/proxy.h:47
...............................................................................

/home/harald/SYCL-CTS/util/logger.cpp:41: warning:
  value: 0.372822586521810384052599829374230466783046722412109375
  [3fd7dc5344149047], reference: 0.
  37282258652181010649684367308509536087512969970703125 [3fd7dc5344149042]

/home/harald/SYCL-CTS/util/logger.cpp:41: warning:
  Expected accuracy in ULP: 4

/home/harald/SYCL-CTS/tests/math_builtin_api/math_builtin.h:194: FAILED:
  CHECK( verify(log, hostRes, ref, accuracy, comment) )
with expansion:
  false
with message:
  tests case: 4601023. Correctness check failed on host.

===============================================================================
test cases:   14 |   13 passed | 1 failed
assertions: 7721 | 7720 passed | 1 failed

The value that causes this is the randomly generated input 0.8783900046.

Evaluation with an arbitrary precision tool (calc, https://github.com/lcn2/calc) tells us that the correct result is

$ calc 'epsilon(1e-100), sin(pi() * 0.8783900046)'
	~0.37282258652181026821

That is, the correct result is just under 0x1.7dc5344149045p-2, SYCL-CTS generates a reference result of 0x1.7dc5344149042p-2 (~-2.9 ULP), we generate a result of 0x1.7dc5344149047p-2 (~+2.1 ULP). We are within 4 ULP of the mathematically correct result, which is what the CTS aims to check, so the sinpi reference result would need to be calculated with more accuracy to not reject this.

I could attempt to create a PR fixing sinpi specifically if so desired, but I suspect this is a bigger problem that applies to other functions as well. I am wondering if rather than re-implementing all the functions, an existing arbitrary-precision tool or library could be reused?

@bader bader added the Agenda To be discussed during a SYCL committee meeting label Feb 2, 2024
@hvdijk
Copy link
Contributor Author

hvdijk commented Feb 6, 2024

Actually, I was wrong here, apologies. I neglected to take into account that 0.8783900046 is not exactly representable in double and redoing the calculation with the exact value, 0.87839000460000005165994707567733712494373321533203125, tells us that SYCL-CTS's reference result is right after all. We are in a range here where a 0.47 ULP difference in input causes a 2.7 ULP difference in output. I will make sure that our implementation gets fixed.

That said, I do still think the suggestion to use an existing arbitrary-precision library is a good one, because we do actually already know we have accuracy issues with some functions (sycl::mix, #835), and in general, we have no way of checking the correctness of the reference implementations.

@tomdeakin
Copy link
Contributor

The SYCL WG discussed this today. The discussion in ongoing.

@tomdeakin tomdeakin removed the Agenda To be discussed during a SYCL committee meeting label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants