Added corrections to re-enable reciprocal test in math_brute_force suite for relaxed math mode #2221
Conversation
|
Possibly dumb question: why are we introducing a special path through binary_operator_float to test reciprocal? Reciprocal is a unary operation, not a binary operation... |
Yep, good point. I went that way just because I wasn't able to choose a better spot. There is no unary float operator, right? To create one I would have to duplicate a lot of code. Any other clues? |
Treating reciprocal as a binary operator with its first argument being the constant 1 seems the least worst solution for now. I'm in the process of reducing code duplication in the math bruteforce suite, so eventually we may be able to move it into its own place. But for now I'm okay with @shajder's chosen solution. |
| // reciprocal differs from divide only in relaxed mode, skip otherwise | ||
| if ((strcmp(f->name, "reciprocal") == 0) && !relaxedMode) return CL_SUCCESS; |
There was a problem hiding this comment.
This is true currently, but it might not be true for long, see: KhronosGroup/OpenCL-Docs#1293
I think it would be better to test "reciprocal" in all cases, even those where the required accuracy matches "divide".
There was a problem hiding this comment.
@bashbaug I can't spot any plans to change spec in reference to fp32 reciprocal precision, am I missing something ?
There was a problem hiding this comment.
Discussed in the January 21st teleconference. We would prefer to test all of the cases, even if there may be some overlap with the divide test:
- In theory, "divide" and "reciprocal" may have different accuracy requirements, though we are currently trending to keep them in sync.
- Testing "reciprocal" explicitly will give us more coverage for the "1.0 / x" cases explicitly, which we may not get through "divide".
There was a problem hiding this comment.
Thanks, reciprocal activated for all fp16/fp32/fp64 and relaxed cases
-reciprocal activated for relaxed float math -reciprocal test added for fp16 and fp64
fixes #2145
As suggested by @svenvh reciprocal has different precision requirements than divide. This PR introduces special path for reciprocal for binar_float_operator to test reciprocal with relaxed math. If this PR will get approvals, invalidate PR #2162