-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change reference_relaxed_exp to use float type to calculate reference #2160
base: main
Are you sure you want to change the base?
Conversation
3c470fd
to
71e418c
Compare
double reference_relaxed_exp(double x) { return reference_exp(x); } | ||
double reference_relaxed_exp(double x) | ||
{ | ||
return reference_exp2(((float)x) * HEX_FLT(+, 1, 715476, +, 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, could you please explain why this test should use a float to compute the reference value in this case?
In case it helps other reviewers, HEX_FLT(+, 1, 715476, +, 0)
expands to + 0x1.715476p+0
, which is equal to 1.44269f
, or M_LOG2E_F
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ulp error for exp according to the specs is ≤ 3 ulp for the full profile and ≤ 4 ulp for embedded profile. However, with -cl-unsafe-math-optimizations flag turned on the ulp requirements become ≤ 3 + floor(fabs(2 * x)) ulp for the full profile, and ≤ 4 ulp for the embedded profile.
As the ulp error is a bit relaxed with the unsafe math operations is there a reason for the same reference values being used for both the relaxed and normal implementation of the exp built-in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Altering the reference function doesn't feel like the right solution. unary_float.cpp
already has logic to select the ULP threshold for exp
and exp2
in relaxed mode:
if (strcmp(fname, "exp") == 0 || strcmp(fname, "exp2") == 0)
{
// For full profile, ULP depends on input value.
// For embedded profile, ULP comes from functionList.
if (!gIsEmbedded)
{
ulps = 3.0f + floor(fabs(2 * s[j]));
}
fail = !(fabsf(err) <= ulps);
}
Are you able to share any details on the values that are failing for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few failures but one example is the following
ERROR: exp: -4.058329 ulp error at 0x1.6005p+2 (0x40b00280): *0x1.e98882p+7 vs. 0x1.e9887ap+7
Our current understanding is that the use of the double input values makes the result of the reference calculation a bit 'overly' precise. There is a number of other reference_relaxed
functions in this file where the input is truncated to float
use lower precision for reference values used for the relaxed implementation of exp math built-in.