I recently encountered an odd problem while setting the diagonal of a rectangular matrix. I traced the problem to the determination of the row count for the Diagonal expression in Core/Diagonal.h, and found a FIXME comment indicating the calculation was known to be incorrect.

I believe it the corrected expression should be:

Code: Select all

RowsAtCompileTime = (int(DiagIndex) == Dynamic || int(MatrixType::SizeAtCompileTime) == Dynamic) ? Dynamic
                      : (EIGEN_SIZE_MIN_PREFER_DYNAMIC(
                            MatrixType::RowsAtCompileTime - EIGEN_PLAIN_ENUM_MAX(-DiagIndex, 0),
                            MatrixType::ColsAtCompileTime - EIGEN_PLAIN_ENUM_MAX( DiagIndex, 0)))

(this also needs to be changed 2 other places, and I would be happy to submit a patch if you'd like)

However, a more difficult problem is accessing the negative first off-diagonal. That is, when DiagIndex is -1. In this case, the index is interpreted as Dynamic. I worked around this by redefining Dynamic to a large negative value (-0x10000000), which seems to work but causes a number of annoying overflow warnings when compiling. Does anyone know of a better way to fix this?

Thanks for finding this out. The conflict between -1 and Dynamic is very annoying because changing its value is not an option. We'll think about it. In the meantime, a patch for the rectangular case would stil be welcome. Ideally, with an extension of the test/diagonal.cpp unit test.

I'm not sure if this is the right place for these, but here are patches for the implementation and unit test.

Patch for src/Core/Diagonal.h: http://pastebin.com/BLc3bSAU
Patch for test/diagonal.cpp: http://pastebin.com/iBisvmmd

Also, thanks for the great library.

Thanks for the patch. Revealing the issue in the unit test was a bit more subtle:

https://bitbucket.org/eigen/eigen/chang ... a00e8dc4f/
changeset: 04ea00e8dc4f
user: ggael
date: 2012-07-10 22:39:05
summary: fix computation of fixed size sub/super diagonal size

And FYI, here is how we fixed the conflict with Dynamic:

https://bitbucket.org/eigen/eigen/chang ... 5fd44ae41/
changeset: 61f5fd44ae41
user: ggael
date: 2012-07-10 23:04:17
summary: Add a DynamicIndex constant for signed quantities and use it to fix the conflict
between Diagonal<S,-1> (the first sub diagonal) and a runtime super/sub diagonal which is now:
Diagonal<S,DynamicIndex>