We would like to propose compile-time access to Eigen::Map dimensionality. To align the Map API as must as possible with the TensorMap API, NumIndices and NumDimensions could be exposed on the Map class and perhaps also on the Matrix class itself. Our use-case is a medical image processing library where compile-time access to image dimensionality is needed. As the medical imaging world is moving to Eigen, e.g. ITK and VTK, I can imagine this will be useful for other developers in future. I am happy to contribute a patch with whatever the maintainers deem the best name (NumDimensions or NumIndices or something else), the most appropriate implementation, and the best place to put it (e.g. in matrix class or only in map class). Thoughts? Comments?
Clarification: While there is compile-time access to dimensionality via IsVectorAtCompileTime, it would be useful to have this as a number named NumIndices and/or NumIndices, so that Map and TensorMap classes can be used interchangeably.
If we add this, this should be added to DenseBase (where SizeAtCompileTime, et al. are defined). Questions: What do we expect NumDimensions to be for 1 x 1: 0, 1 or 2? 1 x N: 1 or 2? N x 1: 1 or 2?
For maximum compatibility between the Map API and the TensorMap API, NumDimensions for 1 x 1 should be 0, NumDimensions for 1 x N should be 1, and NumDimensions for N x 1 should also be 1. I hope this does not clash with definitions/terminology elsewhere in the library.
Created attachment 849 [details] Adds NumDimensions to Eigen/src/Core/DenseBase.h This patch adds NumDimensions to all dense objects (matrix, vector, arrays, and related expression types). The value is equal to NumDimensions for tensors with 0, 1, and 2 dimensions for Scalars, vectors, and matrices, respectively.
Created attachment 854 [details] Add NumDimensions to Matrix, MatrixMap, Array and ArrayMap and tests thereof This patch adds support for NumDimensions to Matrix, MatrixMap, Array and ArrayMap. A helper function was added to Eigen/src/Core/util/XprHelper.h, which is used to set NumDimensions in DenseBase, which in turn is added EIGEN_GENERIC_PUBLIC_INTERFACE. I hope this is the eigen-way to add this functionality. Let me know if you have suggestions for changes/style. There are two limitations of this patch: * I could not figure out how to make new CALL_SUBTEST_# commands, so tests are added to an existing subtest number. If you would tell me how, and I am happy to add tests to a separate subtest number. * This is a git diff. I tested `hg import num_dimensions.patch` and it worked fine, but let me know if you run into any problems.
Created attachment 855 [details] Adds NumDimensions to ArrayBase This patch Adds NumDimensions to ArrayBase (array tests in patch 854 compiled fine without it, but this patch may be added for completeness).
Comment on attachment 854 [details] Add NumDimensions to Matrix, MatrixMap, Array and ArrayMap and tests thereof The following: NumDimensions = (internal::num_dimensions<internal::traits<Derived>::MaxRowsAtCompileTime, internal::traits<Derived>::MaxColsAtCompileTime>::ret), could be just: NumDimensions = int(MaxSizeAtCompileTime)==1 ? 0 : bool(IsVectorAtCompileTime) ? 1 : 2; And unless `NumDimensions` is used within the `Matrix`, `Array`, ... classes, there is no need for `using Base::NumDimensions;`
Created attachment 856 [details] Add NumDimensions to mapped matrices and mapped arrays Thank you for your suggestions Christoph. Non-mapped Matrix/Array NumDimensions is not needed for my particular use case, and perhaps it is better to leave it out (until someone needs it at least, in which case it is trivial to add). I have attached a patch with your suggestions.
Created attachment 857 [details] Add NumDimensions to SolverBase and SparseMatrixBase This patch adds NumDimensions to SolverBase and SparseMatrixBase. This is necessary because the previous patch added NumDimensions to EIGEN_GENERIC_PUBLIC_INTERFACE. If you are not happy with this, and you can think of another solution that does not require changing EIGEN_GENERIC_PUBLIC_INTERFACE, I am happy to implement it.
Dear all, shall we make a decision on this proposal?
Is there anything I can do help expedite this proposal?
applied with some simplifications and more unit tests: https://bitbucket.org/eigen/eigen/commits/cdd8f4e42bdf/ https://bitbucket.org/eigen/eigen/commits/15ab8a43dd4a/ https://bitbucket.org/eigen/eigen/commits/10f80c6eaf2c/
Great! Thank you for your effort!
-- 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/1531.