Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use std::fmin instead of std::min in HcalTimeSlew::delay #12088

Merged
merged 1 commit into from Oct 30, 2015

Commits on Oct 25, 2015

  1. Use std::fmin instead of std::min in HcalTimeSlew::delay

    SIGSEGV (Segfault) was detected in `HcalDeterministicFit::getLandauFrac`
    with ICC 16.0.0 20150815.
    
    Segfault happens in RecoLocalCalo/HcalRecAlgos/src/HcalDeterministicFit.cc:42
    
        42        sum= landauFrac[int(ceil(tStart+tsWidth))];
    
    The function is called here in
    RecoLocalCalo/HcalRecAlgos/interface/HcalDeterministicFit.h:112
    
        112       getLandauFrac(-tsShift4,-tsShift4+tsWidth,i4);
    
    But `tsShift4` ends up being **NAN**:
    
        112       getLandauFrac(-tsShift4,-tsShift4+tsWidth,i4);
        (gdb) p tsShift4
        $3 = nan(0x400000)
        (gdb) p tsWidth
        $4 = 25
        (gdb) p i4
        $5 = 0
    
    `HcalTimeSlew::delay` use a `log` function and a negative argument in
    passed.
    
        (gdb) p fTimeSlew
        $9 = HcalTimeSlew::InputPars
        (gdb) p fTimeSlewBias
        $10 = HcalTimeSlew::Medium
        (gdb) p fpar0
        $11 = 12.299899999999999
        (gdb) p fpar1
        $12 = -2.1914199999999999
        (gdb) p fpar2 // par2
        $13 = 0
        (gdb) p inputCharge[4] // fC
        $14 = (__gnu_cxx::__alloc_traits<std::allocator<double> >::value_type &)
        @0x7fff38cf7e20: -0.08074517548084259
    
    CalibCalorimetry/HcalAlgos/src/HcalTimeSlew.cc:23
    
        return std::min(cap, par0 + par1*log(fC+par2));
    
    Thus we end up with `return std::min(6.0, NAN);`
    
    `std::log` has direct description of how it works if implementation
    supports IEEE:
    
        If the argument is negative, NaN is returned and FE_INVALID is
        raised.
    
    What `std::min` should return in case one of arguments is `NAN` is not directly
    defined in C++ standard. Standard requires `std::min` to return the
    first argumnet (6.0 in our case) if arguments are equivalent. All
    comparison with `NAN` will return `false`. Becasue of this `std::min`
    implementation with GCC and Clang will return the first argument. `NAN`
    is returned in ICC case.
    
    According to IEEE 754:
    
        min(x,NaN) = min(NaN,x) = x
    
    C++11 also introduced `std::fmin` from C standard, which provides a
    direct instructions how `NAN` is handled:
    
        .. between a NaN and a numeric value, the numeric value is chosen)
    
    `std::min` is generic and does not direclty talk about IEEE
    floating-points.
    
    `std::fmin` works fine in correctly between GCC, Clang and ICC.
    
    ICC issue will be reported for Intel for further discussions.
    
    Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47706
    
    Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
    David Abdurachmanov authored and David Abdurachmanov committed Oct 25, 2015
    Copy the full SHA
    bf26bcd View commit details
    Browse the repository at this point in the history