This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 706 - Allowing SparseQR and SparseCholesky to work with MappedSparseMatrix
Summary: Allowing SparseQR and SparseCholesky to work with MappedSparseMatrix
Status: NEW
Alias: None
Product: Eigen
Classification: Unclassified
Component: Sparse (show other bugs)
Version: 3.2
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-20 18:29 UTC by adam.p.harrison
Modified: 2019-12-04 12:49 UTC (History)
1 user (show)



Attachments
Patch for SimplicialCholesky.h (1.42 KB, patch)
2013-11-20 18:32 UTC, adam.p.harrison
no flags Details | Diff
Patch for SimplicialCholesky_impl.h (547 bytes, text/plain)
2013-11-20 18:33 UTC, adam.p.harrison
no flags Details
Patch for SparseQR.h (743 bytes, patch)
2013-11-20 18:33 UTC, adam.p.harrison
no flags Details | Diff
Updated patch for SparseQR.h (993 bytes, patch)
2013-11-21 01:48 UTC, adam.p.harrison
no flags Details | Diff

Description adam.p.harrison 2013-11-20 18:29:49 UTC
SparseQR and SparseCholesky will not compile when passed in a MappedSparseMatrix.
Comment 1 adam.p.harrison 2013-11-20 18:32:38 UTC
Created attachment 396 [details]
Patch for SimplicialCholesky.h

I believe the only thing stopping this are that isolated portions of the code use the MatrixType typedef instead of the QRMatrixType and CholMatrixType, respectively. I have attached three patches with very minor modifications that allow me to use MappedSparseMatrix for both these sparse modules. I hope it is useful.
Comment 2 adam.p.harrison 2013-11-20 18:33:07 UTC
Created attachment 397 [details]
Patch for SimplicialCholesky_impl.h

I believe the only thing stopping this are that isolated portions of the code use the MatrixType typedef instead of the QRMatrixType and CholMatrixType, respectively. I have attached three patches with very minor modifications that allow me to use MappedSparseMatrix for both these sparse modules. I hope it is useful.
Comment 3 adam.p.harrison 2013-11-20 18:33:35 UTC
Created attachment 398 [details]
Patch for SparseQR.h

I believe the only thing stopping this are that isolated portions of the code use the MatrixType typedef instead of the QRMatrixType and CholMatrixType, respectively. I have attached three patches with very minor modifications that allow me to use MappedSparseMatrix for both these sparse modules. I hope it is useful.
Comment 4 adam.p.harrison 2013-11-21 01:42:52 UTC
Comment on attachment 398 [details]
Patch for SparseQR.h

>--- SparseQR_old.h	2013-11-20 17:38:59.502945600 -0700
>+++ SparseQR.h	2013-11-20 17:38:16.990514000 -0700
>@@ -353,7 +353,7 @@
>     // all the nodes (with indexes lower than rank) reachable through the column elimination tree (etree) rooted at node k.
>     // Note: if the diagonal entry does not exist, then its contribution must be explicitly added,
>     // thus the trick with found_diag that permits to do one more iteration on the diagonal element if this one has not been found.
>-    for (typename MatrixType::InnerIterator itp(m_pmat, col); itp || !found_diag; ++itp)
>+    for (typename QRMatrixType::InnerIterator itp(m_pmat, col); itp || !found_diag; ++itp)
>     {
>       Index curIdx = nonzeroCol ;
>       if(itp) curIdx = itp.row();
>@@ -504,7 +504,7 @@
>   if(nonzeroCol<n)
>   {
>     // Permute the triangular factor to put the 'dead' columns to the end
>-    MatrixType tempR(m_R);
>+    QRMatrixType tempR(m_R);
>     m_R = tempR * m_pivotperm;
> 
>     // Update the column permutation
Comment 5 adam.p.harrison 2013-11-21 01:47:03 UTC
Sorry, as neophyte I've really messed up my attempt to add patches. I was trying to fix an omission in the patch to SparseQR.h, but didn't realize it would make a new reply/comment. I've just added a new patch for SparseQR.h. Please ignore the first patch, and this comment. 
(In reply to comment #4)
> Comment on attachment 398 [details] [review]
> Patch for SparseQR.h
> 
> >--- SparseQR_old.h	2013-11-20 17:38:59.502945600 -0700
> >+++ SparseQR.h	2013-11-20 17:38:16.990514000 -0700
> >@@ -353,7 +353,7 @@
> >     // all the nodes (with indexes lower than rank) reachable through the column elimination tree (etree) rooted at node k.
> >     // Note: if the diagonal entry does not exist, then its contribution must be explicitly added,
> >     // thus the trick with found_diag that permits to do one more iteration on the diagonal element if this one has not been found.
> >-    for (typename MatrixType::InnerIterator itp(m_pmat, col); itp || !found_diag; ++itp)
> >+    for (typename QRMatrixType::InnerIterator itp(m_pmat, col); itp || !found_diag; ++itp)
> >     {
> >       Index curIdx = nonzeroCol ;
> >       if(itp) curIdx = itp.row();
> >@@ -504,7 +504,7 @@
> >   if(nonzeroCol<n)
> >   {
> >     // Permute the triangular factor to put the 'dead' columns to the end
> >-    MatrixType tempR(m_R);
> >+    QRMatrixType tempR(m_R);
> >     m_R = tempR * m_pivotperm;
> > 
> >     // Update the column permutation
Comment 6 adam.p.harrison 2013-11-21 01:48:27 UTC
Created attachment 399 [details]
Updated patch for SparseQR.h

I believe the only thing stopping this are that isolated portions of the code
use the MatrixType typedef instead of the QRMatrixType and CholMatrixType,
respectively. I have attached three patches with very minor modifications that
allow me to use MappedSparseMatrix for both these sparse modules. I hope it is
useful.
Comment 7 adam.p.harrison 2015-01-06 22:35:35 UTC
Any thoughts on this? I know I made a mess of this bug report, but I believe the patches I included should fix this, and having the decompositions work on MappedSparseMatrix is an important feature, as it's often prohibitively expensive to perform a copy to a SparseMatrix. This is so in my application of interest. Thanks!
Comment 8 Nobody 2019-12-04 12:49:45 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/706.

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