This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 1521 - SIGFPE encountered when hypot_impl::run is called with two zero arguments.
Summary: SIGFPE encountered when hypot_impl::run is called with two zero arguments.
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.3 (current stable)
Hardware: x86 - 64-bit Linux
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-14 15:34 UTC by David Robinson
Modified: 2019-12-04 17:29 UTC (History)
3 users (show)



Attachments

Description David Robinson 2018-02-14 15:34:33 UTC
Summary: 

SIGFPE encountered in `hypot_impl::run` with two zero arguments.

Overview:

An arithmetic error occurs in Eigen's `hypot_impl::run`, during the use of Spectra to solve an eigenvalue problem. This is caused by a call to  `hypot_impl::run` with two zero arguments .


Steps to Reproduce:

The bug is reproduceable when `hypot_impl::run` is called with two zero values.

Obviously the bug can be avoided by disabling FE_INVALID but this is an unattractive option in a large simulation. 

Example Code:

```
#include <Eigen/Core>                                                            
#include <fenv.h>                                                                
                                                                                 
                                                                                 
int main()                                                                       
                                                                                 
{                                                                                
feenableexcept(FE_INVALID);                                                      
double a=0,b=0;                                                                  
                                                                                 
using namespace Eigen::internal;                                                 
double c =  hypot_impl<double>::run(a,b);                                       
}                    
```

Results:

The program throws a SIGFPE, crashing. If the FE_INVALID is disabled the exception is avoided and the expected result is obtained.
Comment 1 Christoph Hertzberg 2018-02-15 17:27:34 UTC
I would change the implementation to something like this (requires re-ordering of methods in MathFunctions.h):

  typedef typename NumTraits<Scalar>::Real RealScalar;
  static inline RealScalar run(const Scalar& x, const Scalar& y)
  {
    EIGEN_USING_STD_MATH(abs);
    EIGEN_USING_STD_MATH(sqrt);
    RealScalar _x = abs(x);
    RealScalar _y = abs(y);
    RealScalar p = numext::maxi(_x, _y);
    if(p==RealScalar(0)) return RealScalar(0);
    RealScalar q = numext::mini(_x, _y);
    RealScalar qp = q / p;
    return p * sqrt(RealScalar(1) + qp*qp);
  }

Another advantage: Only one branch required (which most of the time is false).

Open question: should infinities and NaNs be handled properly? hypot(inf, inf) would result in NaN, and either hypot(0, NaN) or hypot(NaN, 0) could result in 0, depending on how `maxi` and `mini` are implemented

Also: This implementation does not make sense for complex scalars, since `abs` is not necessarily stable.
Comment 2 Gael Guennebaud 2018-04-04 13:22:06 UTC
I applied a slightly different patch to propagate NaN, together with dedicated unit tests and code factorization with scalar_hypot_op<>.


https://bitbucket.org/eigen/eigen/commits/222c4a60a2365c
https://bitbucket.org/eigen/eigen/commits/cb8b7de2f7297b
https://bitbucket.org/eigen/eigen/commits/2ffd0a74a6566d

3.3 backports:

https://bitbucket.org/eigen/eigen/commits/a9c06b854991be
https://bitbucket.org/eigen/eigen/commits/91d1ac3061f35b
Comment 3 Gael Guennebaud 2018-04-04 13:23:07 UTC
Regarding complexes, only pretty old versions of clang's stdlib had numerical issues with abs(complex), so we should be fine.
Comment 4 Nobody 2019-12-04 17:29:38 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/1521.

Note You need to log in before you can comment on or make changes to this bug.