Deprecated custom defined constants in favor of cmath definitions

Turns out that there is no use for Constants.h at all since defined constants were either not used or can be imported from cmath.

Do you approve @tol ?

Closes #4 (closed)

A few remarks

• The constants M_PI etc are not part of the C++ standard, strictly speaking whether they are available is compiler dependent.
• This changes the type of the constants from real_t to double. In LOFARBeam it was possible to switch between double and single precision by changing the definition of real_t
• BeamFormer.cc defines a speed_of_light, while Constants.h defines c as the speed of light. I think BeamFormer.cc should have used Contants.h

Hi Bas,

Good points. M_PI et al are defined by the Posix standard, so I wouldn't worry about it being unavailable. I use it in WSClean and it compiles on all relevant machines. I rather use that than having extra definitions.

Considering the type; is that an issue? Most of the time use of a double literal number doesn't affect anything, although some computations might result a double instead of a real_t, which then might cause a warning and/or unnecessary use of a double. For that having a separate real_t definition of the constant might make it a bit easier. If defining real_t as float is actually used somewhere than I would keep the constants... But is that the case @tol ? I'm not a fan of making things more generic than we need ;).

No, not really. We inherited this from the LOFAR respository. But even there real_t was hard coded. As far as I know there was no provision to configure the type in cmake.

Ok. So I think the conclusions are:

• Constants.h can't be removed because of c / speed of light
• BeamFormer.cc should use that instead of defining its own
• Up to @jmmaljaars how to resolve the real_t / M_PI_.. things :). Options are: assume real_t is double and use M_PI_.. constants instead; or define them in Constants.h, either by using M_PI vars or independently.

Do you want to change the MR @jmmaljaars ? ( I think all this is a bit nitpicking, sorry about that ).

added 1 commit

• a2aa7b5f - Re-introduce Constants.h for speed-of-light constant

Compare with previous version

Done (the funny thing was that M_PI in fact was already used in BeamFormer.cc)

approved this merge request

mentioned in commit cefe46bd

merged