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

Issue133 nan compare #322

Merged
merged 7 commits into from
Feb 26, 2020
Merged

Issue133 nan compare #322

merged 7 commits into from
Feb 26, 2020

Conversation

mikebentley15
Copy link
Collaborator

Fixes #133

Description:
I created a new function called flit::abs_compare(a, b) that does a little more than std::abs(a-b).

  • If a and b are both NaN with the same sign, return 0.0
  • If a and b are both inf with the same sign, return 0.0

This is to be a better default comparison function that will not fail if both test and baseline values match (even if they are inf or NaN).

This flit::abs_compare() function is used also from flit::l2norm() to take the difference between elements before taking the square and summing. Therefore, flit::l2norm() also benefits from being able to handle inf and NaN in matching spots within the vector.

I changed data/tests/Empty.cpp to use flit::abs_compare() as the default comparison between long double values.

Documentation:
Added a section in writing-test-cases.md called Default Comparison Details giving these details for both flit::abs_compare() and flit::l2norm().

Tests:
Added tests to tests/flit_src/tst_flitHelpers_h.cpp for flit::abs_compare(), and made more tests for flit::l2norm().

@mikebentley15 mikebentley15 added bug c++ Involves touching c++ code documentation Involves touching documentation tests Involves touching tests labels Feb 26, 2020
@mikebentley15
Copy link
Collaborator Author

Sorry, I need to do a documentation change showing the default comparison used in the example test code.

Was causing TravisCI to fail since its compiler only has C++11
support and not C++17 support
src/flit/flitHelpers.h Outdated Show resolved Hide resolved
Remove comment on details that have been removed

No longer return inf if expected is NaN and actual is inf.  It was not a symmetric relationship, so removed.
@mikebentley15
Copy link
Collaborator Author

It's been fixed. @IanBriggs please complete your review.

@IanBriggs IanBriggs merged commit e13f287 into devel Feb 26, 2020
@IanBriggs IanBriggs deleted the issue133-nan-compare branch February 26, 2020 22:02
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 tests Involves touching tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default comparison with NaN values
2 participants