This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 275 - Patch: Kronecker tensor product
Summary: Patch: Kronecker tensor product
Status: NEW
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - matrix products (show other bugs)
Version: 3.0
Hardware: All All
: --- Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-20 21:03 UTC by Andreas Platen
Modified: 2019-12-04 10:46 UTC (History)
1 user (show)



Attachments
Implementation of the Kronecker product for matrices (5.30 KB, text/x-chdr)
2011-05-20 21:03 UTC, Andreas Platen
no flags Details
Kronecker product for matrices (4.82 KB, text/x-chdr)
2011-05-28 12:15 UTC, Andreas Platen
no flags Details
Unit test for kroneckerProduct() (6.97 KB, text/x-c++src)
2011-05-28 12:17 UTC, Andreas Platen
no flags Details
Kronecker product for matrices (5.45 KB, text/x-chdr)
2011-06-15 18:16 UTC, Andreas Platen
gael.guennebaud: review+
Details
Unit test for kroneckerProduct() (7.19 KB, text/x-c++src)
2011-06-15 18:20 UTC, Andreas Platen
gael.guennebaud: review+
Details

Description Andreas Platen 2011-05-20 21:03:28 UTC
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?
Comment 1 Gael Guennebaud 2011-05-23 20:31:36 UTC
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...
Comment 2 Andreas Platen 2011-05-24 18:24:49 UTC
(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!
Comment 3 Andreas Platen 2011-05-28 12:15:17 UTC
Created attachment 176 [details]
Kronecker product for matrices

The result arguments AB are still declared as const ref.
Comment 4 Andreas Platen 2011-05-28 12:17:37 UTC
Created attachment 177 [details]
Unit test for kroneckerProduct()
Comment 5 Andreas Platen 2011-06-15 18:16:47 UTC
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
Comment 6 Andreas Platen 2011-06-15 18:20:07 UTC
Created attachment 186 [details]
Unit test for kroneckerProduct()

* added an additional test with a matrix of type Eigen::Block as input and output
Comment 7 Gael Guennebaud 2011-06-22 14:40:48 UTC
committed. I don't close this bug because this module will require some refactoring once the evaluator implemented.
Comment 8 Nobody 2019-12-04 10:46:58 UTC
-- 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.

Note You need to log in before you can comment on or make changes to this bug.