Cactus math work-arounds break CarpetX reduction operators

[Erik Schnetter]

Years ago we introduced work-arounds for C++ compilers that didn’t properly support the std:: namespace (probably PGI, IBM, Cray). It turns out these work-arounds violate the C++ standard since the re-define global identifiers such as “isnan”. I found out today that they actually break the reduction operators in CarpetX when “-O2” is used (“-O0” is fine).

These versions of these compilers are not relevant any more, and these work-arounds can now be removed. This they cause problems and since they violate the C++ standard, I suggest that we do so.

[Roland Haas]

Pull request is here: https://bitbucket.org/cactuscode/cactus/pull-requests/95/remove-c-math-work-arounds/diff

[Erik Schnetter]

Carpet also needs to be updated; changes here https://bitbucket.org/eschnett/carpet/pull-requests/35/carpet-adapt-math-functions-to-recent/diff

[Roland Haas]

The patch as of git hash 62bc14ab "Remove C++ math work-arounds" of cactus breaks compilation on QueenBee with

COMPILING EinsteinEvolve/NewRad/src/extrap.cc
In file included from /home/rhaas/ET_trunk/configs/sim/bindings/include/GenericFD.h(4),
                 from /home/rhaas/ET_trunk/arrangements/EinsteinEvolve/NewRad/src/extrap.cc(8):
/home/rhaas/ET_trunk/arrangements/KrancNumericalTools/GenericFD/src/GenericFD.h(78): error: more than one instance of overloaded function "signbit" matches the argument list:
In file included from /home/rhaas/ET_trunk/configs/sim/bindings/include/GenericFD.h(4),
                 from /home/rhaas/ET_trunk/arrangements/EinsteinEvolve/NewRad/src/extrap.cc(8):
            function "signbit(double)"
In file included from /home/rhaas/ET_trunk/configs/sim/bindings/include/GenericFD.h(4),
                 from /home/rhaas/ET_trunk/arrangements/EinsteinEvolve/NewRad/src/extrap.cc(8):
            function "std::signbit(double)"
In file included from /home/rhaas/ET_trunk/configs/sim/bindings/include/GenericFD.h(4),
                 from /home/rhaas/ET_trunk/arrangements/EinsteinEvolve/NewRad/src/extrap.cc(8):
            argument types are: (CCTK_REAL8)
In file included from /home/rhaas/ET_trunk/configs/sim/bindings/include/GenericFD.h(4),
                 from /home/rhaas/ET_trunk/arrangements/EinsteinEvolve/NewRad/src/extrap.cc(8):
    int s = signbit(x);

The current choice being between breaking the existing ET on QueenBee (old but in the official list of supported machines) and breaking CarpetX I would rather keep CarpetX broke until QueenBee’s option list etc has been updated.

[Roland Haas]

Also breaks on these with the same error:

• comet
• mike
• shelob
• wheeler

‌

[Roland Haas]

The earliest g++ version (which is used for its stdc++ library by icpc) that works is g++ version 6.X (I do not remember the correct “X”). This is true even when using the newest Intel compiler since the issue is the C++ standard library and not the compiler.

[Roland Haas]

A (partial) workaround would be to test for the faulty behaviour in configure, set a #define and use that with an #ifdef to enable the workarounds only when and if needed. This would let Cactus (but not CarpetX) be used on old systems just like now but those who want to try at the new CarpetX code will have to make sure to use modern compilers (have to any since it requires C++17). Also see

#2409 about affected systems being retired.

[Roland Haas]

Here’s a configure test fragment to trigger the issue:

#include <cmath>
void badmath(void)
{
  using namespace std;
  isnan(0.);
}

to be compiled with icpc -std=c++11 -gxx-name=g++ -O0 -c badmath.cc or g++ -std=c++11 -O0 -c badmath.cc.

[Erik Schnetter]

Note that #include <math.h> should not be used in this test. Another test would be the same, but with #include <math.h>. Yet another test would be the same as above, but using a single-precision argument to isnan.

[Roland Haas]

Comet had been retired so is no longer an issue.

[Roland Haas]

The workarounds (indirectly) also cause issues on new M1 macOS machines since Apple seems to have removed the finite function call that is used in some places instead of std::isfininite (and just isfinite does not work).

[Roland Haas]

Applied as git hash 0721ab29 "Remove C++ math work-arounds" of cactus

Applied as git hash f85f1350 "Carpet: Adapt math functions to recent flesh changes" of carpet

Let’s see if anything breaks until the next release. In that case we can add a configure time check (similar to the one outlined above) and re-activate the workarounds when needed (which won’t be on any system where CarpetX can be compiled).

[Roland Haas]

Resolved for now. Reopen if things break.