- changed component to Cactus
-
assigned issue to
Cactus math work-arounds break CarpetX reduction operators
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.
Comments (13)
-
-
reporter Carpet also needs to be updated; changes here https://bitbucket.org/eschnett/carpet/pull-requests/35/carpet-adapt-math-functions-to-recent/diff
-
- changed status to open
-
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.
-
Also breaks on these with the same error:
- comet
- mike
- shelob
- wheeler
-
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.
-
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#2409about affected systems being retired. -
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
org++ -std=c++11 -O0 -c badmath.cc
. -
reporter 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 toisnan
. -
Comet had been retired so is no longer an issue.
-
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 ofstd::isfininite
(and justisfinite
does not work). -
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).
-
- changed status to resolved
Resolved for now. Reopen if things break.
- Log in to comment
Pull request is here: https://bitbucket.org/cactuscode/cactus/pull-requests/95/remove-c-math-work-arounds/diff