KhronosGroup / KhronosGroup/SYCL-CTS
sycl::mix testing is problematic
- Dominant language
- C++
- Stars
- 75
- Forks
- 96
- Avg merge
- 5d 18h
- Merged PRs (30d)
- 4
Description
`sycl::mix` testing is both overly strict, and insufficiently wide.
`sycl::mix` is specified as
https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#_common_functions
> *Preconditions:* If the inputs are scalars, the value of `a` must be in the range [0.0, 1.0]. If the inputs are not scalars, each element of `a` must be in the range [0.0, 1.0].
>
> *Returns:* The linear blend of `x` and `y`. When the inputs are scalars, returns `x + (y - x) * a`. Otherwise, returns `x[i] + (y[i] - x[i]) * a[i]` for each element of `x`, `y`, and `a`.
This is tested in the CTS by making sure that we are [within 1 ULP](https://github.com/KhronosGroup/SYCL-CTS/blob/bb20fb699e687d36250d627942320c9ab434be44/tests/math_builtin_api/modules/sycl_functions.py#L164) of [`x + (y - x) * a`](https://github.com/KhronosGroup/SYCL-CTS/blob/bb20fb699e687d36250d627942320c9ab434be44/util/math_reference.cpp#L255) for randomized inputs [between 0.1 and 0.9](https://github.com/KhronosGroup/SYCL-CTS/blob/bb20fb699e687d36250d627942320c9ab434be44/tests/math_builtin_api/modules/test_generator.py#L87).
The first problem here is that the SYCL specification uses a blend of mathematical expressions and C++ expressions without specifying which is which, almost certainly this is meant to be taken as a mostly-mathematical expression (see e.g. `degrees`, where the vector version is specified as returning `(180 / π) * radians[i]`, which clearly cannot be a C++ expression), the C++ expression `x + (y - x) * a` is not (in general) within 1 ULP of the mathematical result rounded suitably to the appropriate C++ floating point type, and the C++ expression `x + (y - x) * a` is not (in general) a good implementation.
`mix` was added to C++20 as `std::lerp` (except without the restriction on `0 <= a <= 1`) and this was carefully considered at that time. In [P0811R3: Well-behaved interpolation for numbers and pointers](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0811r3.html), S. Davis Herring writes:
> Both [obvious approaches](https://stackoverflow.com/questions/4353525/floating-point-linear-interpolation) used in published implementations of floating-point linear interpolation have [issues](https://math.stackexchange.com/questions/907327/accurate-floating-point-linear-interpolation):
>
> 1. `a+t*(b-a)` does not in general reproduce *b* when `t==1`, and can overflow if *a* and *b* have the largest exponent and opposite signs.
> 2. `t*b+(1-t)*a` is not monotonic in general (unless the product *ab*≤0).
It goes on to list properties that are desired for `std::lerp`, including these. By testing that `mix` is within 1 ULP of a reference implementation that is known to not generally be useful, the CTS requires an implementation of `mix` that is not generally useful.
Additionally, in OpenCL, single-precision `mix` is specified as ["absolute error tolerance of 1e-3" (full profile) or "Implementation-defined" (embedded profile)](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_C.html#relative-error-as-ulps). A stricter check for `mix` than what OpenCL specifies implies that a `sycl::mix` implementation that defers to OpenCL `mix` cannot, in general, pass the SYCL CTS, which seems undesirable.
The second problem here is that by only testing values between 0.1 and 0.9, the worst problems with this reference implementation never even come up during CTS tests and go unnoticed, both in the CTS itself and in implementations that take guidance from the CTS.
Contributor guide
Research direction
Start with tests/math_builtin_api/modules/sycl_functions.py, util/math_reference.cpp, and tests/math_builtin_api/modules/test_generator.py, then compare their mix reference and input ranges with the SYCL specification, OpenCL tolerance, and std::lerp discussion linked in the issue. Done means the CTS check has a justified conformance criterion and exercises boundary cases without requiring an unsuitable implementation strategy.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100