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

Argument checks incorrect because of nan. #1010

Open
ryanelandt opened this issue Jul 31, 2023 · 2 comments
Open

Argument checks incorrect because of nan. #1010

ryanelandt opened this issue Jul 31, 2023 · 2 comments

Comments

@ryanelandt
Copy link
Contributor

When checking input arguments, it's common to want min <= max. If this statement is not true, the program needs to error. One way to make sure the statement is true is to assert that it is true. E.g., with:

BOOST_MATH_ASSERT(min <= max);

A common way of checking the assertion that min <= max in the Boost codebase is to use the construction:

if (min > max) {
      return policies::raise_evaluation_error(function, "assertion min <= max failed (first arg=%1%)", min, boost::math::policies::policy<>());
}

The first approach has three advantages over the second approach:

  1. It is cleaner and clearer (imo)
  2. It makes disagreement between the error message and performed check impossible (e.g., update beta error messages and checks #1003)
  3. It gives the right answer when one of both arguments is nan. E.g.,
const auto fn = [](double x) { return std::make_pair(x, 1.0); }; 

const double inf = std::numeric_limits<double>::infinity();
const double nan = std::numeric_limits<double>::quiet_NaN();
   
const double x0 = boost::math::tools::newton_raphson_iterate(fn, -inf,  /* initial guess */
                                                                  inf,  /* lower bound */
                                                                  nan,  /* upper bound */
                                                                  52);

gives x0 = nan, even though the lower bound is inf.

NOTE:
The six comparison operators are: ==, <, <=, >, >=, and !=. For all comparison operators besides !=, if one or both of the arguments are nan, the result is false according to the IEEE standard.

@jzmaddock
Copy link
Collaborator

The issue with assertions is that they are debug only, once the program is built with -D_NDEBUG=1 they disappear.

The six comparison operators are: ==, <, <=, >, >=, and !=. For all comparison operators besides !=, if one or both of the arguments are nan, the result is false according to the IEEE standard.

True, however it is common for programs to be compiled with options (often the default for release builds) where the NaN's are assumed to simply not exist and these assumptions no longer hold. Besides which if(!(min < max)) would have the same logic as your assertion.

@ryanelandt
Copy link
Contributor Author

Besides which if(!(min < max)) would have the same logic as your assertion.

Yes that's much better. That's what I used in #1003.

The issue with assertions is that they are debug only, once the program is built with -D_NDEBUG=1 they disappear.

That's true. They also don't print out the values of the things being compared. What's really needed IMO, are macros with the following properties:

  • Don't go away in release mode
  • automatically print the values being compared on failure

So, essentially Boost Test macros, but for runtime use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants