Skip to content
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

Default comparison with NaN values #133

Closed
mikebentley15 opened this issue Apr 12, 2018 · 2 comments · Fixed by #322
Closed

Default comparison with NaN values #133

mikebentley15 opened this issue Apr 12, 2018 · 2 comments · Fixed by #322
Assignees
Labels
bug c++ Involves touching c++ code documentation Involves touching documentation question

Comments

@mikebentley15
Copy link
Collaborator

mikebentley15 commented Apr 12, 2018

If the ground-truth execution returns NaN or -NaN, then no matter what the other executables return, the default comparison function will always return either NaN or -NaN.

Not only is this value weird and unintuitive, but it breaks the bisect algorithm. If it weren't for the bisect algorithm, I would say this is not an issue, as it is kind of weird if your baseline code is returning NaN.

However, for the sake of bisect, and therefore for always and forever, the default case shall be to say that if the ground-truth is NaN or -NaN, then return 0.0 if the baseline is the same sign of NaN. That is to say, as far as FLiT is concerned in the default case, NaN == NaN and -NaN == -NaN.

@mikebentley15 mikebentley15 added the c++ Involves touching c++ code label Jun 6, 2018
@mikebentley15 mikebentley15 added the documentation Involves touching documentation label Aug 15, 2018
@mikebentley15
Copy link
Collaborator Author

I sat down with Ian and we hashed out what we want implemented. First thing to do would be to make tests checking all of these aspects.

Concerns:

  • How does the FLiT code handle NaN and inf return values? Properly, I hope
  • Can we return negative numbers?

FP categories:

  • inf
  • -inf
  • NaN
  • -NaN
  • in-between

Cases:

Baseline Trouble Return Current Pass
NaN NaN 0.0 NaN no
NaN -NaN NaN NaN
NaN inf inf NaN no
NaN -inf inf NaN no
NaN normal NaN NaN
-NaN NaN NaN NaN
-NaN -NaN 0.0 NaN no
-NaN inf inf NaN no
-NaN -inf inf NaN no
-NaN normal NaN NaN
inf NaN NaN NaN
inf -NaN NaN NaN
inf inf 0.0 NaN no
inf -inf inf inf
inf normal inf inf
-inf NaN NaN NaN
-inf -NaN NaN NaN
-inf inf inf inf
-inf -inf 0.0 NaN no
-inf normal inf inf
normal NaN NaN NaN
normal -NaN NaN NaN
normal inf inf inf
normal -inf inf inf
normal normal abs normal

Patterns:

  • If sign and (nan-ness or inf-ness are the same), then should return 0.0
  • If baseline is NaN and trouble is inf, return inf (ignore sign)

So, it looks like with these two if statements along with a fallback of the current implementation, we can get the behavior we want.

We also need to ensure this is documented properly.

@mikebentley15
Copy link
Collaborator Author

I think I want the default comparison to be symmetric. That means a different table:

Baseline Trouble Return Current Pass
NaN NaN 0.0 NaN no
NaN -NaN NaN NaN
NaN inf NaN NaN
NaN -inf NaN NaN
NaN normal NaN NaN
-NaN NaN NaN NaN
-NaN -NaN 0.0 NaN no
-NaN inf NaN NaN
-NaN -inf NaN NaN
-NaN normal NaN NaN
inf NaN NaN NaN
inf -NaN NaN NaN
inf inf 0.0 NaN no
inf -inf inf inf
inf normal inf inf
-inf NaN NaN NaN
-inf -NaN NaN NaN
-inf inf inf inf
-inf -inf 0.0 NaN no
-inf normal inf inf
normal NaN NaN NaN
normal -NaN NaN NaN
normal inf inf inf
normal -inf inf inf
normal normal abs normal

There is really only one additional pattern that needs to be implemented on top of std::abs():

  • If sign is the same and (nan-ness or inf-ness is the same), then return 0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c++ Involves touching c++ code documentation Involves touching documentation question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants