Bug 526 - Incorrect result of setLinSpaced
Incorrect result of setLinSpaced
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Core - general
3.1
x86 - 64-bit Windows
: Normal major
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-23 08:52 UTC by Vladimir
Modified: 2013-02-18 18:34 UTC (History)
4 users (show)



Attachments
output of setLinSpaced (12.01 KB, text/plain)
2012-10-23 08:52 UTC, Vladimir
no flags Details

Description Vladimir 2012-10-23 08:52:00 UTC
Created attachment 303 [details]
output of setLinSpaced

Filling the array with numbers produces incorrect results

I want an array of 50 rows and 49 columns, filled with increasing numbers in columns, like this:

0  50 ...  2400
1  51      2401
2  52
3  53
.  .
.  .
.  .
49 99 ...  2449


Here is the code:

ArrayXXi indsM(50, 49); 
for(int i = 0; i < 49; ++i) {
    indsM.col(i).setLinSpaced(50, i * 50, (i + 1) * 50 - 1);
}

cout << indsM << endl;

The complete output is in the attached file, here is the excerpt:
0  50 100 150 ... 2400
1  51 101 151 ... 2401
2  50 102 150
3  51 103 151
4  52 104 152
5  53 105 153
.
.
.
.
40 97 149 197 ... 2449

Please, note that all columns, where numbers end with fifty-something, are filled incorrectly.

I also tried overloaded setLinSpaced(low, high), the result is the same.
Comment 1 Vladimir 2012-10-23 09:08:56 UTC
Oups, there's a typo, the lower left number must be 49.
0  50 100 150 ... 2400
1  51 101 151 ... 2401
2  50 102 150
3  51 103 151
4  52 104 152
5  53 105 153
.
.
.
.
49 97 149 197 ... 2449
Comment 2 Christoph Hertzberg 2012-12-20 15:59:38 UTC
This is only loosely related, but did we specify how setLinSpaced() shall behave for integer types, e.g. should setLinspaced(3, 0, 1) result in [0 0 1] or [0 1 1]?
The current implementation results in [0 0 0] which I guess is not intended.

I think a general definition such as result(i) = (low*(n-1-i)+high*i)/(n-1) would make sense. Of course, more efficient implementations shall be used where possible (e.g. the current implementation for floats and something Bresenham-style for integers).

Another side-note: should we maybe also provide methods which accepts a start value, a step size and total length or upper limit (under different names of course), i.e. sth similar to Matlab's (low:step:high) expression?
Comment 3 Jitse Niesen 2013-02-18 18:34:24 UTC
Perhaps an easier test example is:

  VectorXf v(8);
  v.setZero();
  v.tail(6).setLinSpaced(0, 1);
  cout << v.transpose() << endl;

which produces (assuming that vectorization is enabled)

  0   0   0 0.2   0 0.2 0.4 0.6

instead of 

  0   0   0 0.2 0.4 0.6 0.8   1

The problem is linspaced_op_impl in the case RandomAccess = false, as implemented in Functors.h:538. In the test example, and also in the original example, the assignment uses LinearVectorizedTransversal. This means that first operator() in linspaced_op_impl is called a number of times, then packetOp(), then operator(). Unfortunately, operator() in linspaced_op_impl does not advance m_base.

I fixed this, but I am not sure my fix is the most efficient as it adds madd() calls which are unnecessary in some situations. Revision 8a36e91732b7 (dev branch) and 6baa53efd8da (version 3.1)

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