search

idefix-code / idefix

A fast finite volume code designed to run on many architectures, such as GPU, CPU and manycores, using Kokkos.
https://idefix.readthedocs.io/
Other
28 stars 19 forks source link

avoid implicit conversion to double in VTK I/O routines #255

Closed glesur closed 3 months ago

glesur commented 3 months ago

Some compilers (e.g. CLang 17.0) do an implicit conversion to double when using the trigonometric functions. This results in corrupted vtk files since the BigEndian conversion and the VTK format all expect single precision coordinates in VTK files.

This PR fixes this issue encountered recently with Rocm 5.7

glesur commented 3 months ago

The only fix possible with bigEndian only is to restrict the input type to float only.

Incidentally, I don't recall why this function was generalised as a template function... (which is bugged for any type that is not coded on 4 bytes). Clearly, this mess should be cleaned up.

neutrinoceros commented 3 months ago

I don't recall why this function was generalised as a template function

I don't recall either, but it seems fishy. Anyway, no big deal if you don't want to take the risk to undo the templating.

glesur commented 3 months ago

templating is needed because we need big endian conversion for both floats and integer (for the geometry and periodicity flags you introduced back in the days)

Just pushed a new version that implements bigendian conversion in an agnostic manner (i.e. doesn't assume anything regarding the size of the input type). While this is now full-proof, it doesn't prevent the original bug which was an implicit conversion to double precision before calling BigEndian, because the sin and cos functions in C are normally returning double precision.

In other words, if some sin and cos functions are used in the VTK for particles, these should be replaced by their c++ equivalent std::sin and std::cos, that are properly overloaded for single precision inputs.