-
Notifications
You must be signed in to change notification settings - Fork 6
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
Issue143 litmus nans #195
Issue143 litmus nans #195
Conversation
For |
Actually, none of the values used in the |
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.
I am glad you made changes to get rid of NaN
values. For the Inlining test, your values are good and show variability, same for the reciprocalmath test. However for Distributivity, you provided 10 inputs where 2 of those produce NaN comparison values (because the tru value is inf)
I think the best thing to do would be to find 10 more inputs in long double
that show variability. However, I don't think that's reasonable to make you do since inputGen
is not working anymore (that's how I found these inputs).
I think the next best thing is to remove these data values and put in a comment saying that we need to find values to show variability for long double.
convert(0x6703, 0x6455d50eb2825cf7), // 3.818039e+3006 | ||
convert(0x1f77, 0x70b75c7169817349), // 9.267715e-2508 | ||
convert(0x4ab4, 0x27faa40914dad6a6), // 4.148019e+824 | ||
// 0x663B40322A41D3AE972B |
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.
What does this comment mean?
|
||
convert(0x2355, 0xB32CA7EC5E694CA3), // 1.541551e-2209 | ||
convert(0x7336, 0x90ABC4405309DE21), // 7.201858e+3946 | ||
convert(0x736A, 0xAC4F344252296368), // 3.863064e+3962 |
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.
This input at idx2 is interesting as it shows with bisect that the assertion of independent elements triggers. I would like to understand this a little better. It was identifying files as culprits that have nothing to do with this... I was comparing "g++ -O0" with "g++ -O3 -funsafe-math-optimizations". My guess is that it involves an inline function where it is using the optimized version no matter what (because of the order in which I include the object files, maybe).
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.
It turns out it was because idx2 still outputs a NaN
for a good comparison. It is because the test returns inf
under the ground truth compilation. This is a bad set of values.
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.
Note, this applies to the one above this comment, not below. i.e. the one with the first value starting with 0x2355.
|
||
convert(0x4492, 0x870E87461590FB53), // 3.384007e+352 | ||
convert(0x71DA, 0xAC9981EE164A3F4A), // 1.498527e+3842 | ||
convert(0x586A, 0xFC38E006060C3819), // 1.079136e+1882 |
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.
This idx7 above results in an inf
value for the ground truth, and therefore results in a NaN
after the comparison. Please fix.
None of them provided variability anyway. We will want another issue to track to find and add values back in for long double for DistributivityOfMultiplication.
I went ahead and deleted the values for long double since they didn't show any variability cases. I put in a TODO statement and created issue #196 to find new values and put them in. So with these changes, I'm fine with accepting this change set. |
No description provided.