Created attachment 173 [details] Implementation of the Kronecker product for matrices Hi everybody, Kolja Brix and I implemented the Kronecker tensor product for dense and sparse matrices, see the attachement for a proposal. At the moment one calls the function by Eigen::kroneckerProduct(A, B, AB); where matrix AB will be the Kronecker tensor product of the dense or sparse matrices A and B. If either A or B is of sparse matrix type, then AB has to be a sparse matrix too. We would like to provide our code, so that it potentially can be integrated into Eigen and be used by everybody. Is there any chance to do so?
hi, some comments on the patch: - I don't understand why you declared the result arguments AB as const ref instead of non const ref. This would avoid the ugly const_cast... - since the two implementation functions kroneckerProduct_* are purely internal, no need to declare the result argument as MatrixBase or SparseMatrixBase. - these two functions should put into the internal namespace - I don't yet in which module to put this feature, so in the meantime I'd put in into the unsupported SparseExtra module. This means this file should be put into the unsupported/Eigen/src/SparseExtra folder and included from unsupported/Eigen/SparseExtra main header file. - Then the Eigen:: namespace prefix could be removed - The indentation should be fixed (2 spaces) - you should add a unit test in unsupported/test/kronecker_product.cpp. Have a look at the other tests for some examples. Basically, you have to implement a void test_kronecker_product() function that will be automatically called. The rest sounds good to me. The next step be to replace the kroneckerProduct(A, B, AB) function by AB = A.kroneckerProduct(B) using expression templates, but it's better to wait for the refactoring of the expression evaluator...
(In reply to comment #1) > - I don't understand why you declared the result arguments AB as const ref > instead of non const ref. This would avoid the ugly const_cast... We wanted to write a generic implementation, so that the return argument AB can be an arbitrary dense or sparse matrix. Sometimes one has to resize the matrix AB according to the dimensions of A and B, which does not work for fixed size matrices. We found the solution with const_cast in the documentation of Eigen, see http://eigen.tuxfamily.org/dox/TopicFunctionTakingEigenTypes.html#TopicResizingInGenericImplementations Therefore we used it. Is there a better way to solve the resize problem? > - since the two implementation functions kroneckerProduct_* are purely > internal, no need to declare the result argument as MatrixBase or > SparseMatrixBase. > > - these two functions should put into the internal namespace > > - I don't yet in which module to put this feature, so in the meantime I'd put > in into the unsupported SparseExtra module. This means this file should be put > into the unsupported/Eigen/src/SparseExtra folder and included from > unsupported/Eigen/SparseExtra main header file. > > - Then the Eigen:: namespace prefix could be removed > > - The indentation should be fixed (2 spaces) > > - you should add a unit test in unsupported/test/kronecker_product.cpp. Have a > look at the other tests for some examples. Basically, you have to implement a > void test_kronecker_product() function that will be automatically called. Okay, these points should be clear. I'll upload an update for the patch soon, if we have done the corrections and some unit tests. > The rest sounds good to me. Fine. Thank you for the comments!
Created attachment 176 [details] Kronecker product for matrices The result arguments AB are still declared as const ref.
Created attachment 177 [details] Unit test for kroneckerProduct()
Created attachment 185 [details] Kronecker product for matrices * removed unnecessary derived() * removed const_cast in kroneckerProduct_full() and kroneckerProduct_sparse() and solved the resize problem for dense fixed size matrices with the function in line 103-108 * added an additional function with const_cast to make it possible, that a dense output matrix can be of type Eigen::Block, see line 122-127
Created attachment 186 [details] Unit test for kroneckerProduct() * added an additional test with a matrix of type Eigen::Block as input and output
committed. I don't close this bug because this module will require some refactoring once the evaluator implemented.
-- 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/275.