New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 1383 - LinSpaced not backwards compatible w.r.t. num_steps=0
LinSpaced not backwards compatible w.r.t. num_steps=0
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: General
3.3 (current stable)
All All
: Normal Crash
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-24 20:45 UTC by Alec Jacobson
Modified: 2017-01-25 17:15 UTC (History)
3 users (show)



Attachments

Description Alec Jacobson 2017-01-24 20:45:43 UTC
I'm not sure if LinSpaced has well documented behavior for size=0, but in version 3.2.9 this 


    Eigen::VectorXi::LinSpaced(0,0,-1);

would produce an empty vector. (This comes up rather regularly for dynamic code where you have Eigen::VectorXi::LinSpaced(n,0,n-1);)

In version 3.3.1 and 3.3.90, the code above causes a divide by zero:


Process 48952 stopped
* thread #1: tid = 0x25fd2c, 0x0000000100000f64 .main`Eigen::internal::linspaced_op_impl<int, long long vector[2], true>::linspaced_op_impl(this=0x00007fff5fbfecc8, low=0x00007fff5fbfed2c, high=0x00007fff5fbfed28, num_steps=0) + 196 at NullaryFunctors.h:96, queue = 'com.apple.main-thread', stop reason = EXC_ARITHMETIC (code=EXC_I386_DIV, subcode=0x0)
    frame #0: 0x0000000100000f64 .main`Eigen::internal::linspaced_op_impl<int, long long vector[2], true>::linspaced_op_impl(this=0x00007fff5fbfecc8, low=0x00007fff5fbfed2c, high=0x00007fff5fbfed28, num_steps=0) + 196 at NullaryFunctors.h:96
   93  	  linspaced_op_impl(const Scalar& low, const Scalar& high, Index num_steps) :
   94  	    m_low(low),
   95  	    m_multiplier((high-low)/convert_index<Scalar>(num_steps<=1 ? 1 : num_steps-1)),
-> 96  	    m_divisor(convert_index<Scalar>(num_steps+high-low)/(high-low+1)),
   97  	    m_use_divisor((high+1)<(low+num_steps))
   98  	  {}
   99
Comment 1 Alec Jacobson 2017-01-24 21:25:31 UTC
This bug also applies to the .setLinSpaced member function. Analogously triggered by:

     Eigen::VectorXi a;
     a.setLinSpaced(0,0,-1);
Comment 2 Gael Guennebaud 2017-01-25 14:28:57 UTC
Since it used to work:

https://bitbucket.org/eigen/eigen/commits/f9e15e6f77e8/
https://bitbucket.org/eigen/eigen/commits/d8141d508293/ (3.2)
Summary:     Bug 1383: Fix regression from 3.2 with LinSpaced(n,0,n-1) with n==0.
Comment 3 Alec Jacobson 2017-01-25 15:45:07 UTC
Relatedly, 3.2.9 used to allow "reverse" LinSpaced results: high < low:

      Eigen::VectorXi a = Eigen::VectorXi::LinSpaced(10,9,0);

With 3.3.1 and 3.3.90 I get:

* thread #1: tid = 0x35183f, 0x0000000100001d29 .main`int const Eigen::internal::linspaced_op_impl<int, long long vector[2], true>::operator(this=0x00007fff5fbfeb70, i=0)<long>(long) const + 57 at NullaryFunctors.h:104, queue = 'com.apple.main-thread', stop reason = EXC_ARITHMETIC (code=EXC_I386_DIV, subcode=0x0)
    frame #0: 0x0000000100001d29 .main`int const Eigen::internal::linspaced_op_impl<int, long long vector[2], true>::operator(this=0x00007fff5fbfeb70, i=0)<long>(long) const + 57 at NullaryFunctors.h:104
   101 	  EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE
   102 	  const Scalar operator() (IndexType i) const
   103 	  {
-> 104 	    if(m_use_divisor) return m_low + convert_index<Scalar>(i)/m_divisor;
   105 	    else              return m_low + convert_index<Scalar>(i)*m_multiplier;
   106 	  }
   107 	


(haven't tested your most recent commit to see if it also fixes this)
Comment 4 Gael Guennebaud 2017-01-25 16:45:44 UTC
Right, I've thought about the high<low case when fixing the previous issue, and yes in this case m_divisor can become zero. Actually, the current version does not work at all for integer types with high<low.
Comment 5 Gael Guennebaud 2017-01-25 17:15:47 UTC
OK, this time high<low should be fine, at least for cases for which an even spacing is possible. 

https://bitbucket.org/eigen/eigen/commits/0129ada50a19/
https://bitbucket.org/eigen/eigen/commits/251f2c07153e/ (3.3)
Summary:     Bug 1383: fix regression in LinSpaced for integers and high<low

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