search

SBNSoftware / sbnanaobj

4 stars 10 forks source link

Centralize definitions of NaN and -5 #24

Open cjbacchus opened 3 years ago

cjbacchus commented 3 years ago

It was mentioned in a couple of reviews that we should have some standard place where we defined the magic constants -5 and numeric_limits<float>::signaling_NaN().

We may also need to review the use of these values. The original intention in NOvA, which I think is not documented anywhere in SBN, is that a NaN value surviving to the final CAF indicates a bug in CAFMaker, where it has failed to initialize a field. Whereas a field that is somehow unfillable gets -5. Of course that magic number doesn't work for all variables, and we have some -999s etc as well.

wesketchum commented 2 years ago

@FernandaPsihas @PetrilloAtWork can you coordinate this? If it can be done by 2021C, great -- if not, let's move it to general Infrastructure/Utilities.

cjbacchus commented 2 years ago

kSignalingNaN is being introduced in https://github.com/SBNSoftware/sbnanaobj/pull/44 There still needs to be a bulk rewrite to make use of it (sed -i is our friend here)

brucehoward-physics commented 2 years ago

kUninitializedInt being introduced in same PR now.

PetrilloAtWork commented 2 years ago

There was the question whether float kSignalingNaN could be also used to initialize double. From a ROOT session:

$ float const f = std::numeric_limits<float>::signaling_NaN()
(const float) nanf
$ double d = std::numeric_limits<float>::signaling_NaN()
(double) nan
$ std::isnan(d)
(bool) true

So in short: a double variable assigned a float NaN is indeed given a NaN value. Note however that all of the following: d == d, f == f, and d == f will return false, which is a weird feature of NaN. So testing must be done with std::isnan(), in all cases.

Just mentioning here that we could in principle have a specific NaN for uninitialised variables. It would behave as any other NaN value, but on careful inspection it may be identified as the "uninitialised variable" NaN.