ianhinder / Kranc

A Mathematica package for generating code for solving time dependent partial differential equations
http://kranccode.org
GNU General Public License v2.0
29 stars 10 forks source link

Kranc generates code which does not compile with IBM compilers #65

Open barrywardell opened 12 years ago

barrywardell commented 12 years ago

In the KerrSchild thorn of the EinsteinExact code, there is a term in the equations involving 'sqrt(2)'. Note that this appears in the generated code as sqrt(2), not sqrt(2.0). For most compilers this is fine, but Peter reports that the IBM compiler on P7 fails to compile the code:

"/mnt/gpfs.work/diener/ET-Release-May/Cactus/configs/test/build/KerrSchild/KerrSchild_always.cc", line 185.31: 1540-1229 (I) Argument number 1 is an rvalue of type "int". "/usr/include/math.h", line 987.20: 1540-1202 (I) No candidate is better than "sqrt(long double)". "/mnt/gpfs.work/diener/ET-Release-May/Cactus/configs/test/build/KerrSchild/KerrSchild_always.cc", line 185.31: 1540-1231 (I) The conversion from argument number 1 to "long double" uses "a floating point-integral conversion".

Is there a way to either modify Kranc to fix this, or use some specific compiler options with the IBM compiler?

rhaas80 commented 12 years ago

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

Hello Barry,

Is there a way to either modify Kranc to fix this, or use some specific compiler options with the IBM compiler? I'd blame the compiler for being too picky :-)

On a more serious note: I am surprised that the best match suggested is "long double" and not "double". It might be sufficient to have Kranc include "cmath" instead of "math" which gives you the C math functions instead of the C++ ones.

Actually: do we need the Kranc sources to be C++ anymore? Maybe just C99 would be sufficient (we use c99 features in other basic thorns as well so we would not risk more failures). That should get rid of the error and also might make compilation faster since C is a smaller and simpler language than C++.

Yours, Roland


My email is as private as my paper mail. I therefore support encrypting and signing email messages. Get my PGP key from http://keys.gnupg.net. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk+poVwACgkQTiFSTN7SboWmDQCdGLTJsI/BdPk5KE/6rAZu8jc/ 3gMAoLVmVX0q8IAXwphdCZ/zcMtNLLx5 =oNI5 -----END PGP SIGNATURE-----

eschnett commented 12 years ago

This is probably a confusion between including , , and/or using "using namespace std;", together with Carpet's attempts to fix this for all sorts of weird C++ implementations.

One solution to add (CCTK_REAL) in front of 2, or writing ToReal(2) (this is one reason why I introduced this function).

Similar problems exist with pow(x,y) when x and y have different types (e.g. pow(var,2)).

Vectorisation uses C++.

ianhinder commented 12 years ago

In C, the type of sqrt is double -> double. In C++, there are several overloaded functions called sqrt, with signatures double -> double, float -> float and long double -> long double (http://www.cplusplus.com/reference/clibrary/cmath/sqrt/). My understanding is that if an int is used where a floating point type is expected, the int will be converted automatically. In this case, the compiler doesn't know which of the sqrt functions to choose, so it chooses the "best". [Shouldn't it be able to infer this from the environment in which the function is used? Maybe it's not allowed to be so clever.] This is all fine. I don't understand why "The conversion from argument number 1 to "long double" uses "a floating point-integral conversion" is a problem. What is wrong with such a conversion?

I found page 242 of http://www.redbooks.ibm.com/redbooks/pdfs/sg247853.pdf -- is this helpful? That seems to be talking about using C99 features in C++.

Incidentally, my version of that thorn has line 185 showing

      SQR(Z) - SQR(ToReal(a)) + sqrt(SQR(pow(X,2) + pow(Y,2) + pow(Z,2) - 

which brings up another problem: Kranc should be converting pow(X,2) into SQR(X) (which is X*X).

barrywardell commented 12 years ago

So, would a simple solution be to just enable vectorisation? That would then ensure that ToReal() is inserted as appropriate.

barrywardell commented 12 years ago

Yes, with vectorisation enabled the problematic term becomes ksqrt(ToReal(2)), which should be OK for any compiler. This is probably a sufficient workaround for the moment, but I suggest keeping this open until the original problem can be solved.

With regards tot the SQR problem Ian mentioned, I think this is related to some interaction with CSE. When I turn CSE off, the SQR appears as it should.

ianhinder commented 12 years ago

But does anyone understand why the original problem appears in the first place? Is it a fault of Kranc, a fault of the compiler, or an ambiguity in the C++ language specification?

eschnett commented 12 years ago

I believe this depends on which include files you use. By default, sqrt is not visible. With , probably only sqrt(double) is visible. With , all sqrts are visible, and you have an ambiguity. If you include both math.h and cmath, then it may depend on the order... But all of this is just a guess.

No, C++ compilers are not allowed to look at the context of where sqrt is used. Function overloading looks only at the arguments.

ianhinder commented 12 years ago

This page suggests that including cmath or math.h is equivalent, but that is not said explicitly and this is obviously not an authoritative source. Do you know a good place to look to learn more about this?

eschnett commented 12 years ago

For these kinds of questions I would either search Google (without much hope), read the standard (costs money), or look/ask on stackoverflow.com. See e.g. http://stackoverflow.com/questions/6215467/cmath-header-confusion. It seems the answer is "it depends".

In Carpet I define a namespace "good", and try to put good declarations there (i.e. declarations that work everywhere), for example good::isnan.

eschnett commented 12 years ago

When vectorising, we explicitly generate the equivalent of sqrt((CCTK_REAL)2). (If this is not the case, this is a separate issue.) We could do the same when not vectorising.