search

lesgourg / class_public

Public repository of the Cosmic Linear Anisotropy Solving System (master for the most recent version of the standard code; GW_CLASS to include Cosmic Gravitational Wave Background anisotropies; classnet branch for acceleration with neutral networks; ExoCLASS branch for exotic energy injection; class_matter branch for FFTlog)
230 stars 285 forks source link

Fix seg fault from ndf15 evolver in #419 #423

Open fruzsinaagocs opened 3 years ago

fruzsinaagocs commented 3 years ago

This PR fixes a possible segmentation fault arising from NaN values being passed to the numjac function of the ndf15 numerical solver when asking for the relevant derivatives (i.e. the right-hand-side of the differential equation being solved). I perform a class_test whenever the relevant function calculating the derivatives, *derivs, is called, so CLASS throws a more informative error, and doesn't break when called in a loop as part of an MCMC chain.

This fixes #419, where a shooting error is delayed until the background module, but is never actually thrown because the segmentation error occurs first. It also fixes an issue encountered in GAMBIT (https://github.com/GambitBSM), where an unphysical point in the MCMC parameter space causes one or more of the differential equations involved to be ill-defined.

fruzsinaagocs commented 3 years ago

Hi Nils,

Thanks for the quick response! That's completely true, unfortunately the --fast-math option generally makes it hard to catch NaNs with a generic origin... Let's see what Thomas suggests.

pstoecker commented 3 years ago

Thanks for the heads-up on this Nils. This could really lead to problems.

How about using isnan() from math.h or is this also effectively optimised away with --fast-math?

fruzsinaagocs commented 3 years ago

Unfortunately isnan() is optimised away as well. Basically, --fast-math assumes NaNs and infs never occur.

pstoecker commented 3 years ago

OK I see. if --ffast-math is really such a beasty flag one should really refrain from using it, unless one is 100% sure that the code will never lead to infs or NaNs (which is a very ambitious promise in a code a big as CLASS).

I guess that this is actually the reason why this particular flag is never included when we compile C/C++ codes in gambit with gcc. Seeing as --ffast-math is a shortcut for fno-math-errno -funsafe-math-optimizations -ffinite-math-only -fno-rounding-math -fno-signaling-nans -fcx-limited-range one could instead use this list and strip -ffinite-math-only (and other potentially poisonous flags) from it.

The question, however, is whether we will have to give up speed in doing so.

ThomasTram commented 3 years ago

Hi @fruzsinaagocs

Thanks for looking into this! I don't like --fast-math either, we should test how much of a speed difference it really makes. (Also perhaps for a subset of them, lige suggested by @pstoecker ) This seems like something that could be done with a little bit of bash-scripting. However, perhaps the fix could be made more robust: there must be some index calculation that fails with NaNs, but that can be fixed in such a way that it works regardless of --fast-math. Do you happen to have an .ini file that generates a segmentation-fault?

Cheers, Thomas