Bugzilla – Full Text Bug Listing

This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen

Summary: | Extend API of TriangularView and SelfAdjointView | ||
---|---|---|---|

Product: | Eigen | Reporter: | Christoph Hertzberg <chtz> |

Component: | Core - general | Assignee: | Nobody <eigen.nobody> |

Status: | DECISIONNEEDED --- | ||

Severity: | Feature Request | CC: | chtz, daniel.vollmer, gael.guennebaud, jacob.benoit.1 |

Priority: | Normal | ||

Version: | 3.3 (current stable) | ||

Hardware: | All | ||

OS: | All | ||

Whiteboard: | |||

Bug Depends on: | |||

Bug Blocks: | 1608 |

Description
Christoph Hertzberg
2014-09-24 16:29:01 UTC
I'm not convinced that Triangular/SelfAdjoint-View should inherit MatrixBase as MatrixBase is really for "dense with full storage" objects, but I agree that we should support more features in TriangularBase. Hm, then it does not make much sense to me that, e.g., the following lines work: Eigen::MatrixXd A(3,3); foo(Eigen::MatrixXd::Identity(3,3)); foo(A.triangularView<Eigen::Upper>()*Eigen::MatrixXd::Identity(3,3)); //foo(A.triangularView<Eigen::Upper>()); // does not work So far, I always considered MatrixBase as any kind of dense matrix, including Matrix expressions. (In reply to Christoph Hertzberg from comment #2) > Hm, then it does not make much sense to me that, e.g., the following lines > work: > > Eigen::MatrixXd A(3,3); > foo(Eigen::MatrixXd::Identity(3,3)); > foo(A.triangularView<Eigen::Upper>()*Eigen::MatrixXd::Identity(3,3)); > //foo(A.triangularView<Eigen::Upper>()); // does not work Here, MatrixXd::Identity is explicitly a dense MatrixXd, but you build it a DiagonalMatrix, then none of the calls to foo will work. > So far, I always considered MatrixBase as any kind of dense matrix, > including Matrix expressions. True, but can we consider a TriangularView as a dense matrix? In the extreme case we also have SparseView, for which it is pretty clear that it should inherit SparseMatrixBase, and not MatrixBase. With that reasoning, I agree that SelfAdjointView is more debatable. The return type of SparseMatrixBase::triangularView inherits from SparseMatrixBase already, (last time I checked). I admit that things like DenseMatrix::triangularView<Upper>().triangularView<Lower>() are indeed debatable, but API-wise I would keep them 'Dense' until explicitly using .sparseView(). Motivation, why someone would like to use MatrixBase here instead of EigenBase is that MatrixBase::eval()/MatrixBase::nested<2>() will (should) always result in Direct-Access-types, which is not the case for Sparse matrices. Another problem is that the output stream operator does not work for triangularView (although it used to work with 3.2) To comment #5: The following does not work for me, neither with 3.1, nor 3.2: int main() { Eigen::MatrixXd A; std::cout << A.triangularView<Eigen::Upper>() << "\n"; } To comment #4: A counter argument is that you cannot access to a "dense" TriangularView as a general dense matrix: accesses to half of the coefficients are forbidden. I had for a long time the idea to introduce the concept of matrices defined by "columns (resp. rows) for which a unique dense sub-segment is non-zero". Here, "dense" really means that the respective sub-segment inherits MatrixBase with efficient random coeff/packet access. This way we could unify dense matrices, triangular-view, band matrices, etc. (In reply to Gael Guennebaud from comment #6) > The following does not work for me, neither with 3.1, nor 3.2: [...] > std::cout << A.triangularView<Eigen::Upper>() << "\n"; Yes, sorry I did not actually try this. The equivalent used to work with Eigen2.0 though (I noticed that while porting deprecated doxygen code snippets). And apparently, this is hardly ever required (I can't remember any discussion about that on bz/ML/IRC). > To comment #4: > > A counter argument is that you cannot access to a "dense" TriangularView as > a general dense matrix: accesses to half of the coefficients are forbidden. Yes, that would still require to specializes stream operators for TriangularView. I guess we already had the discussion about letting coeff return Scalar(0) for the other half? (Unfortunately, this is not easily possible with coeffRef, though -- and introduces overhead ...) > I had for a long time the idea to introduce the concept of matrices defined > by "columns (resp. rows) for which a unique dense sub-segment is non-zero". > Here, "dense" really means that the respective sub-segment inherits > MatrixBase with efficient random coeff/packet access. This way we could > unify dense matrices, triangular-view, band matrices, etc. Sounds interesting -- this would also allow, e.g., accessing sub-blocks of triangular matrices. I'd also just realised that triangularView doesn't have the "usual" methods (in my case I wanted isZero() for checking the type of a Butcher-tableau). I think it would be nice and consistent (IMO) to have them. Alright, so I changed the summary of this entry to reflect that what is most needed is to extend the feature set supported by triangular and selfadjoint views. This include: - is*() - operator<< - set*/fill - sum(), prod(), max/minCoeff() - *norm() - row/column-wise operations Of course all the reductions and checks will work on the triangular part only. Otherwise, prod() would always return 0, isOnes() would be always false, etc. (except for a 1x1 matrices...). -- 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/885. |