RFC: Use of min/max, fmin/fmax, etc.

If you have two doubles x and y and you'd like to take their maximum, you have quite a few options.

fmax(x, y); // calls the C function
std::max(x, y); // resolves to std::max<double>(x, y)
std::fmax(x, y); // resolves to std::fmax<double>(x, y)

I think we should not be using std::max at all (except for integer types), because it does not handle NaNs gracefully (unfortunately, it's what we use almost everywhere). E.g. the following programme

#include <cmath>
#include <iostream>

int
main() {
  double const mynan = std::nan("");
  std::cout << std::max(1.0, mynan) << std::endl;
  std::cout << std::max(mynan, 1.0) << std::endl;
}

prints

1
nan

for me, which is useless and also not reliable. std::fmax exists for this purpose.

But I think we also should not be writing std::fmax directly instead. Rather, I'd like to advocate

using std::fmax;
fmax(x, y);

thereby enabling ADL for types like adouble. The fact that we're currently not doing that has forced someone to put

namespace std
{
   adouble max(adouble a, adouble b) {
     ...
   }
}

in dune/fufem/utilities/adolcnamespaceinjections.hh which from my reading of the standard (quoted here) is not allowed.

To be precise, I think

namespace std
{
   template<>
   adouble const &max(adouble const &a, adouble const &b) {
     ...
   }
}

would be allowed in this case but I'd rather not see std extended unless absolutely necessary.

So to summarise, I would like us to use

using std::fmax;
fmax(x, y);

in place of fmax, std::max, and std::fmax for floating point types everywhere because it handles NaN properly and works in conjunction with ADL. If we can get a consensus on this matter (for dune-fufem, dune-solvers, etc.) I'll happily go ahead and do the actual work. I'd first like to collect comments and opinions, though.

NB: DenseVector::infinity_norm has a using clause just like this to get ADL to work properly but then uses std::max instead of std::fmax. That's intentional here, because a NaN should propagate!

Assignee Loading
Time tracking Loading