KhronosGroup / SYCL-CTS

SYCL Conformance Tests
Apache License 2.0
62 stars 80 forks source link

sycl::mix testing is problematic #835

Open hvdijk opened 11 months ago

hvdijk commented 11 months ago

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 of x + (y - x) * a for randomized inputs between 0.1 and 0.9.

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, S. Davis Herring writes:

Both obvious approaches used in published implementations of floating-point linear interpolation have issues:

  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). 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.

tomdeakin commented 11 months ago

WG discussed this, and are working on the resolution.