When using Eigen on Mac OS X in Xcode with default build settings, Clang gives the following warning for Block.h for row 340 and 341: Implicit conversion loses integer precision: 'Index' (aka 'long') to 'int' The obvious solution is to change m_outerStride to be of type Index, just as the return type of m_xpr.outerStride() and m_xpr.innerStride().
Actually I don't understand why we store it, and why we don't simply implement Index outerStride() as: return internal::traits<Block>::HasSameStorageOrderAsXprType ? m_xpr.outerStride() : m_xpr.innerStride(); just like innerStride()
After setting the C++ dialect in Xcode to C++11 I am receiving similar warnings in the following places (Clang is specific to the actual variable, I have marked this with ** **: Assign.h: Line 356 ::run(dst, src, **outer**); PermutationMatrix.h: Line 114: inline Index size() const { return **indices().size()**; }; Line 549: inline int rows() const { return **m_matrix.rows()**; }; Line 550: inline int rows() const { return **m_matrix.cols()**; }; Transpositions.h: Line 81: inline Index size() const {return **indices.size(); }; PartialPivLU.h: Line 104: m_p(**matrix.rows()**), Line 105: m_rowsTranspositions(**matrix.rows()**); Line 256: row_transpositions[k] = **row_of_biggest_in_col**; Line 274: first_zero_pivot = **k**; Line 345: first_zero_pivot = **k+ret**; Line 394: m_rowsTranspositions.resize(**size**);
Created attachment 296 [details] Proposed patch to fix loss of Integer Precision warnings. I'm new to Eigen, so I just went through the code trying to find suspicious uses of 'int' instead of traits::Index. Where a typedef was missing for traits::Index I added one. In one case this conflicted with a function argument already called Index, so I named it IndexType.
Created attachment 474 [details] fixes for tricky long to int conversions The following changeset fixes many implicit conversions: https://bitbucket.org/eigen/eigen/commits/2a7bf8832563/ however, there are still many which are more tricky to solve. The attached patch is a first attempt to fix the remaining ones. One issue is what to do when mixing expressions having different index types? This is typically the case of expressions storing array(s) of indices for which in most cases 'int' is preferred to limit the memory consumption: PermutationMatrix, Transpositions, and SparseMatrix. Another difficulty is that the 'Index' type of Matrix/Array is not templated. As a result, for those objects, there is currently an ambiguity between the type used to store the indices and the 'Index' type used to define the dimensions, for the arguments of coeff(), and the likes. Currently, they are the same and thus we have many long-to-int-to-long conversions when, e.g., accessing the dimensions of a PermutationMatrix of int: PermutationMatrix<int>::rows() calls size() on the Matrix<int> object used to store the indices. This later returns a 'DenseIndex' while PermutationMatrix<int>::rows() returns an 'int' which is typically converted to a 'DenseIndex' in the callee code. In the proposed patch, I introduced an 'IndexType' which should be read as 'StorageIndexType' for both PermutationMatrix and Transpositions. This type is only used for the storage and as the return type of coeff(). All other methods use 'DenseIndex'. This seems to be the cleanest possible solution. Unfortunately, this is also slightly changing the API of these classes, though such changes are unlikely to be noticeable. Ideally, I would also make the same changes for SparseMatrix as the current workarounds to fix these conversions warnings are rather ugly.
I think your proposed API changes are ok. I doubt that any `regular user' works directly with PermutationMatrix. Actually, it could make sense to be able to specify the IndexType as a parameter of pivoting decompositions (e.g., as optional second parameter after MatrixType) -- though I admit that there are barely cases where it would make a practical difference because usually the coefficients of the matrix take up magnitudes more of memory and we are not at the point yet where we can decompose matrices bigger than 2^31 in both dimensions.
Patch applied. I'll do the changes for sparse matrices directly in the evaluator branch. https://bitbucket.org/eigen/eigen/commits/b92d6db282da/ Changeset: b92d6db282da User: ggael Date: 2014-07-17 13:34:26 Summary: Bug 397: add a warning for 64 to 32 bit integer conversion and fix many of these warning by splitting the index type used for storage and as size/coefficient indexes in PermutationMatrix and Transpositions.
Seems to be fixed by now.
-- 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/397.