[Piglit] [PATCH] CL: Fix check of ULP when probing float/double results

[Aaron Watry]

On Tue, Dec 29, 2015 at 12:16 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:

> On Tue, Dec 29, 2015 at 1:01 PM, Aaron Watry <awatry at gmail.com> wrote:
> > Hi Jan/Tom,
> >
> > Sorry to resurrect an ancient thread, but I was poking at the piglit CL
> ULP
> > issue last night, and thought I'd try to get us closer to a solution
> after
> > dropping the matter for too long.
> >
> > I've modified your test program with what I was thinking of trying, and I
> > wouldn't mind your feedback.
> >
> > I realize that we still have issues with discrepancies between
> > python-generated expected results based on cpu/rounding mode and the ,
> but
> > I'd at least like for us to be able to nail down the C portion before we
> > start redefining all of our expected test results.  In the long term, we
> > probably want to hand-select our inputs/outputs instead of trusting
> python,
> > but I don't necessarily think that we should block fixing our ULP
> > calculations on getting that done.
> >
> > For now, I'm still ignoring the half-ULP possibility, and just
> generating a
> > minimum/maximum allowed value based on running nextafterf(expected,
> POS/NEG
> > INFINITY) in a loop for ULP iterations. Does that sound like a tenable
> > solution?
>
> I'm not 100% sure what you're trying to do, but Micah Fedke (and to a
> lesser extent I) spent a ton of time trying to make
> GL_ARB_shader_precision piglits in an automated manner (the ext
> specifies allowable error for a bunch of functions). We ended up with
> a solution which can test some very basic things on its own (single
> operations), but any complex function basically can only be tested
> with manually-selected values.
>
> Take a look at generated_tests/gen_shader_precision_tests.py .
>
> The basic issue that you have to contend with is that if you ever end
> up with a mul + add anywhere in the intermediate calculation of
> anything (e.g. dot product), it might get collapsed into a fma(),
> which will mess things up greatly. For example imagine you have the
> following code:
>
> x = uniform value which == a * b;
> y = a * b - x
>
> One way y == 0, another way y == some small but not insignificant
> number due to the added precision in the a*b calculation. No amount of
> ULP cleverness will save you here.
>
> If this has nothing to do with what you're trying to achieve, feel
> free to ignore :)
>
>
The issue that we have in CL piglits right now is that the program-tester
is treating the ULP value for a given test as an absolute tolerance, not as
floating units of least precision.

Example (generated_tests/cl/builtin/math/builtin-float-cos-1.0.generated.cl
):
[test]
name: cos float1
kernel_name: test_1_cos_float
global_size: 12 0 0

arg_out: 0 buffer float[12] 1.0 0.0 -1.0 0.0 1.0 0.432574513059
0.753902254343 -0.145500033809 0.943808393901 0.626322983292
-0.925879022855 nan  tolerance 4 ulp
arg_in: 1 buffer float[12] 0.0 1.57079632679 3.14159265359 4.71238898038
6.28318530718 1.12345 7 8 1048576.0 16777216.0 1.32922799578e+36 nan

Let's take the second test:
cos(1.57079632679) [essentially cos(pi/2)] should be roughly 0.0. The test
is specified with a tolerance of 4 ULP. The current piglit CL code is
accepting any value in the range [-4.0, ..., 4.0], because it is treating
the ULP as an absolute tolerance, not units of least precision.

Regardless of whether 0.0 is the absolutely correct answer for
cos(1.57079632679), we can hopefully all agree that -4.0 is NOT a valid
answer for cosine of anything

The functions that I'm trying to fix here (piglit-util-cl.c:
piglit_cl_probe_[floating|double]), take:
1) The actual result from the kernel/shader.
2) The expected result defined in the test.
3) A ULP value to use to calculate the acceptable tolerances.

But it does it wrong. It was copy/pasted from the integer checking code
which takes a tolerance...

--Aaron




>   -ilia
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151229/4b4a5df3/attachment.html>