Summary:  Patch: Kronecker tensor product  

Product:  Eigen  Reporter:  Andreas Platen <andiplaten>  
Component:  Core  matrix products  Assignee:  Nobody <eigen.nobody>  
Status:  NEW   
Severity:  Unknown  CC:  gael.guennebaud  
Priority:    
Version:  3.0  
Hardware:  All  
OS:  All  
Whiteboard:  
Attachments: 

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 103108
* added an additional function with const_cast to make it possible, that a dense output matrix can be of type Eigen::Block, see line 122127
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. 
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?