Re: [eigen] Student contribution

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

Hi,

ok for a base SVD class (SVDBase would be a better name) but only if
we can share a little bit more than typedefs that have to be imported
anyway owing to template programming. I believe there is more to
share, like parts of the allocation logic and the solving step. Some
using Base::m_membername; might save you many this-> statements. Be
aware that dynamic polymorphism (virtual functions) is not allowed in
Eigen, so the concept of a base classe in Eigen, and more generally in
heavily templated c++ code might be subtly different to what you're
used to . You probably already noticed it though ;)

I also think that most of the unit tests could be shared via, for
instance, a svd_common.h file. For instance, jacobisvd_check_full
could be templated on the complete SVD type:

template<typename MatrixType, typename SVD>
void jacobisvd_check_full(const MatrixType& m, const SVD& svd) { ... }

If generalizing a function can save you a copy/paste, then that's the
right way to go ;)

Also, check your editor settings. The indentation in  JacobiSVD.h has
been completely messed up. A diff between the original JacobiSVD.h and
modified one should only report lines that really changed
(http://eigen.tuxfamily.org/index.php?title=Developer%27s_Corner#Coding_rules).

Regarding the "specifications of the methods we are about to
implement", I'd say that as long as the public API stay compatible
with JacobiSVD (that's also why it's good to have shared unti tests),
then you're free to decompose the actual algorithm to what sounds to
best  to you. I don't have a clear enough view of the algorithm to do
smart suggestions here.

keep up the hard work!

gael

On Wed, Jun 5, 2013 at 7:13 PM, BRUN Gauthier <brunga@xxxxxxxxxxxxxx> wrote:
>
> Hello,
> Due to the fact that we had our end year exams last week, we couldn't work
> significantly on our project.
> However we would like to submit a reorganization of the JacobiSVD class in
> the SVD directory, indeed, we realized that we had to use some parts of the
> JacobiSVD class in our functions. So we created a mother class "GenericSVD"
> where there are functions used by both the JacobiSVD class and our newly
> created class "BDCSVD" (for bidiagonal divide and conquer SVD)  ("JacobiSVD"
> and "BDCSVD" extend "GenericSVD").
> "BDCSVD" is not finished yet : for the moment there's only the
> specifications of the class and a part of the algorithm which is in debug
> phase.
>
> We would greatly appreciate if you could give us a feedback about the
> reorganization first and then about the specifications of the methods we are
> about to implement.
> Note : the restructuration seems to work well for Eigen 3, the tests named
> jacobisvd and geo* are still working when compiled with gcc on Linux. Yet,
> it is not compatible with Eigen 2 and we do not know why. We would
> appreciate any help on that matter.
>
> Here is the link to our project :
> https://bitbucket.org/gl27/eigen
> The changed files are in the SVD directory
>
> Best regards,
> Team GL27
> Gauthier Brun, Nicolas Carre, Jean Ceccato, Pierre Zoppitelli
>
>