Inclusion of VDT is problematic

[dpiparo]

The inclusion of part of the VDT library (https://github.com/dpiparo/vdt) is problematic.
For example in include/blond/vdtcore_common.h, constants present in a system header, math.h, are redefined. Namely:

#define M_PI 3.14159265358979323846
#define M_PI_2 1.57079632679489661923

This is less than optimal practice. If inclusion of parts of other libraries cannot be avoided or proper treatment as external packages has to be postponed, it may be useful to include in the BLonD repo the proper headers ;-) (https://github.com/dpiparo/vdt/blob/master/include/vdtcore_common.h)

Cheers,
D

[kiliakis]

Hi,
Old headers have been replaced with proper headers in commit 95a74c7 .
We are currently including only a part of the library in order to minimize dependencies.

@dpiparo I saw that there is a version of sin that operates on arrays (sinv()), do you think that we could profit from using it instead of the scalar one?

Cheers

[dpiparo]

Hi,

in general it is bad to snapshot externals: one unavoidably falls in the trap of not including fixes and improvements (it happened in other projects in the past exactly for vdt).
If this is a temporary workaround, it is acceptable though.
Vectorisation is to be explored. Perhaps vectorising the full set of operations and not just the sin is a better solution? What needs to be vectorised?

Cheers,
D

[kiliakis]

I will leave this open to remind me that this is only a temporary workaround ;)

As for the vector version of sin, I mainly had in mind kick function, where fast_sin is called in a loop for every particle. Perhaps we could use fast_sinv there.

This particular loop is vectorized anyway.

[OlegJakushkin]

@dpiparo: #include "inttypes.h" is not Visual Studio compatible... Have you tried to compile VDT on Windows?

[dpiparo]

No, VDT is not intended for windows yet. You can propose a patch if you have one in mind.
Nevertheless, I would not recommend to define constants which are available in *nix system headers within the package. Maybe an ifdef block can be used?