On Sun, Jun 14, 2015 at 7:48 PM, Aaron Watry <[email protected]
<mailto:[email protected]>> wrote:
On Sun, Jun 14, 2015 at 4:19 PM, Jan Vesely <[email protected]
<mailto:[email protected]>> wrote:
On Sat, 2015-06-13 at 21:22 -0500, Aaron Watry wrote:
> Meh, this still feels broken. Give me a bit longer.
and it is :). I don't think this can work based on abs(expected
- real),
since ULP depends on the magnitude of the numbers. This
information is
lost after subtraction.
Yeah, that's essentially the conclusion that I came to last night as
well. Currently, we're saying that 3 ULP is 3x the smallest
representable floating point number, not 3x the interval between
adjacent float values for the given magnitude.
I was thinking that we might need to take the expected value, cast
to unsigned, add the ulp value, then convert back to float. From
there, find the difference between expected and the expected+ulp,
and use that as a tolerance. We could alternatively call
nextafterf/nextafter the required number of times from the expected
value in either direction and get a min/max allowed value and then
check that the result value is in that range.
I considered casting to ints too. However, it runs into problems in some
corner cases (like neg zero, crossing exponent boundaries, ...),
nextafter offered an easy way out. although it's still not perfect. When
the number sits on exponent boundary, going up or down gives different
steps. one example is 2^24:
16777214: 16777214.000000
16777215: 16777215.000000
16777216: 16777216.000000
16777217: 16777216.000000
16777218: 16777218.000000
16777219: 16777220.000000
we could take something like unit = nextafterf(expected) - expected;
and then compare against ulp * unit, that way we can even subtract 0.5
unit for python/piglit rounding.
Although nextafterf(expected) - expected hits representability issues
for very large numbers.
however the problem whether to take nextaftef(x, 0.0f) or nextafterf(x,
+/- inf) persits.
I'm not sure which one we want. if python always rounded to/away from zero,
we could always take the opposite, but it would cause problems for
non-integer ulp (which we don't support at the moment anyway).
Jan
Yes, we still run into cases where the tests themselves are expected
an incorrectly rounded value, but at least the ULP checking code
might be functioning correctly then.
Thoughts?
--Aaron
I had an idea some time back to implement this using nextafterf
in both
directions and checking whether the result falls in that interval.
However, there is still a problem. Some of the expected values are
already rounded (I'm not sure what rounding mode is used by
python by
default), but unless it always rounds in one direction, we'll
still get
slight differences based on whether the actual value was rounded
up or
down.
I have attached a small test program that shows the deficiencies of
fabsf based approaches.
regards,
Jan
>
> --Aaron
>
> On Sat, Jun 13, 2015 at 2:28 PM, Aaron Watry
<[email protected] <mailto:[email protected]>> wrote:
>
> > We need to actually check against the float value from the
union,
> > instead of just doing (diff > ulp), which seems to cast the
diff to
> > an int before checking against ulp.
> >
> > Signed-off-by: Aaron Watry <[email protected]
<mailto:[email protected]>>
> > CC: Tom Stellard <[email protected]
<mailto:[email protected]>>
> > CC: Jan Vesely <[email protected]
<mailto:[email protected]>>
> > ---
> > tests/util/piglit-util-cl.c | 28 ++++++++++++++--------------
> > 1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/tests/util/piglit-util-cl.c
b/tests/util/piglit-util-cl.c
> > index 47e0c7a..6cdd718 100644
> > --- a/tests/util/piglit-util-cl.c
> > +++ b/tests/util/piglit-util-cl.c
> > @@ -80,7 +80,7 @@ piglit_cl_probe_floating(float value,
float expect,
> > uint32_t ulp)
> >
> > diff = fabsf(value - expect);
> >
> > - if(diff > ulp || isnan(value)) {
> > + if (diff > t.f || isnan(value)) {
> > printf("Expecting %f (0x%x) with tolerance
%f (%u ulps),
> > but got %f (0x%x)\n",
> > e.f, e.u, t.f, t.u, v.f, v.u);
> > return false;
> > @@ -108,7 +108,7 @@ piglit_cl_probe_double(double value,
double expect,
> > uint64_t ulp)
> >
> > diff = fabsl(value - expect);
> >
> > - if(diff > ulp || isnan(value)) {
> > + if (diff > t.f || isnan(value)) {
> > printf("Expecting %f (0x%lx) with tolerance
%f (%lu ulps),
> > but got %f (0x%lx)\n",
> > e.f, e.u, t.f, t.u, v.f, v.u);
> > return false;
> > @@ -162,7 +162,7 @@
piglit_cl_get_platform_version(cl_platform_id platform)
> > int scanf_count;
> > int major;
> > int minor;
> > -
> > +
> > /*
> > * Returned format:
> > *
> >
OpenCL<space><major_version.minor_version><space><platform-specific
> > information>
> > @@ -353,7 +353,7 @@ piglit_cl_get_info(void* fn_ptr, void*
obj, cl_uint
> > param)
> >
> > if(errNo == CL_SUCCESS) {
> > param_ptr = calloc(param_size, sizeof(char));
> > -
> > +
> > /* retrieve param */
> > if(fn_ptr == clGetPlatformInfo) {
> > errNo =
clGetPlatformInfo(*(cl_platform_id*)obj,
> > param,
> > @@ -463,7 +463,7 @@
piglit_cl_get_program_build_info(cl_program program,
> > cl_device_id device,
> > .program = program,
> > .device = device
> > };
> > -
> > +
> > return piglit_cl_get_info(clGetProgramBuildInfo,
&args, param);
> > }
> >
> > @@ -479,7 +479,7 @@
piglit_cl_get_kernel_work_group_info(cl_kernel kernel,
> > cl_device_id device,
> > .kernel = kernel,
> > .device = device
> > };
> > -
> > +
> > return piglit_cl_get_info(clGetKernelWorkGroupInfo,
&args, param);
> > }
> >
> > @@ -620,7 +620,7 @@ piglit_cl_get_device_ids(cl_platform_id
platform_id,
> > cl_device_type device_type,
> >
piglit_cl_get_error_name(errNo));
> > return 0;
> > }
> > -
> > +
> > /* get device list */
> > if(device_ids != NULL &&
num_device_ids > 0) {
> > *device_ids =
malloc(num_device_ids *
> > sizeof(cl_device_id));
> > @@ -761,7 +761,7 @@
> >
piglit_cl_build_program_with_source_extended(piglit_cl_context
context,
> > piglit_cl_get_error_name(errNo));
> > return NULL;
> > }
> > -
> > +
> > errNo = clBuildProgram(program,
> > context->num_devices,
> > context->device_ids,
> > @@ -788,7 +788,7 @@
> >
piglit_cl_build_program_with_source_extended(piglit_cl_context
context,
> > char* log =
> > piglit_cl_get_program_build_info(program,
> >
> > context->device_ids[i],
> >
> > CL_PROGRAM_BUILD_LOG);
> > -
> > +
> > printf("Build log for device %s:\n
-------- \n%s\n
> > -------- \n",
> > device_name,
> > log);
> > @@ -848,11 +848,11 @@
> >
piglit_cl_build_program_with_binary_extended(piglit_cl_context
context,
> > for(i = 0; i < context->num_devices; i++) {
> > char* device_name =
> > piglit_cl_get_device_info(context->device_ids[i],
> >
> > CL_DEVICE_NAME);
> > -
> > +
> > printf("Error for %s: %s\n",
> > device_name,
> >
piglit_cl_get_error_name(binary_status[i]));
> > -
> > +
> > free(device_name);
> > }
> >
> > @@ -860,7 +860,7 @@
> >
piglit_cl_build_program_with_binary_extended(piglit_cl_context
context,
> > return NULL;
> > }
> > free(binary_status);
> > -
> > +
> > errNo = clBuildProgram(program,
> > context->num_devices,
> > context->device_ids,
> > @@ -884,11 +884,11 @@
> >
piglit_cl_build_program_with_binary_extended(piglit_cl_context
context,
> > char* log =
> > piglit_cl_get_program_build_info(program,
> >
> > context->device_ids[i],
> >
> > CL_PROGRAM_BUILD_LOG);
> > -
> > +
> > printf("Build log for device %s:\n
-------- \n%s\n
> > -------- \n",
> > device_name,
> > log);
> > -
> > +
> > free(device_name);
> > free(log);
> > }
> > --
> > 2.1.4
> >
> >
--
Jan Vesely <[email protected] <mailto:[email protected]>>
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit