Re: [eigen] Re: Student contribution: Pull Request |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/eigen Archives
]
- To: eigen <eigen@xxxxxxxxxxxxxxxxxxx>
- Subject: Re: [eigen] Re: Student contribution: Pull Request
- From: Gael Guennebaud <gael.guennebaud@xxxxxxxxx>
- Date: Wed, 12 Jun 2013 23:26:51 +0200
- Cc: "simon.courtemanche" <simon.courtemanche@xxxxxxxx>, Pierre ZOPPITELLI <Pierre.Zoppitelli@xxxxxxxxxxxxxxx>, Nicolas Carré <carren@xxxxxxxxxxxxxx>, Jean CECCATO <Jean.Ceccato@xxxxxxxxxxxxxxx>, "matthieu.moy" <Matthieu.Moy@xxxxxxxxxxxxxxx>
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; bh=AWl/3iHytbDIwHhmMyn6gdlnvzqhwgCuqbGZ3XKkdBo=; b=VA0EYKs/qCY9yVY6+1GvEqVx+f0HdN00vUX2wnh6QlvBDY5NYDdRt6W4nNeVgUVxg3 Rg5b93TKhjnBeCfjKetuClETa+1zUOHgQCBNi0kyxxUFNXR2h76WH+b1EDiF+NdH/eCj yhSjJBy9D6JKITiJ53eGn2movYWH2calEzI6i2ysv8MQ6kpIFzfovVe/wg/+pBKv8Ydl G1dAsSrFr3twPtyojCkTflImTRy5d3C+dsEAdOe7CCZ1JMRbuL/CGOcTVRT7OKQ7cWR4 Q2x8xhWqf+J/XhvU13JZVFes/oeuFZnonHbNS0yAk9uXh52TYvOHq2D8kFEDU0PwOg+C maLQ==
Hi,
this is indeed pretty though to follow. You should really consider
adding comments in the code to explain what is the purpose of each
piece of code. Which books/papers did you use as a reference? You
should add these references into the comments as well. I'll look at
the algorithmic level later (need your references first), but here are
a few suggestions to make a better use of Eigen.
A loop like:
//compute U
for (int k=0; k<n; k++) {
mU(k,i) = temp(k)/coeffU;
}
can be written as:
mU.col(j) = temp/coeffU;
There are many of them in specialSVD(). Note that I also removed the
one-line comment that is useless here as the code is self-explanatory
here. On the other hand I'd to know what specialSVD is doing. What are
its inputs? outputs? etc.
Another one:
// echange V columns
temp = this->m_naiveV.col(I).segment(Zero, lastCol + shift);
this->m_naiveV.col(I).segment(Zero, lastCol + shift) <<
this->m_naiveV.col(J).segment(Zero, lastCol + shift);
this->m_naiveV.col(J).segment(Zero, lastCol + shift) << temp;
can be simplified as:
this->m_naiveV.col(I).segment(Zero, lastCol + shift).swap(
this->m_naiveV.col(J).segment(Zero, lastCol + shift) );
When you work on a column (or row), use the col(.) method instead of
bock(....): it is faster and it makes it clearer you are working on a
part of a column and not on a general sub-matrix. E.g.:
this->m_naiveU.block(firstCol,firstCol,k + 1,1)
would become:
this->m_naiveU.col(k+1).segment(firstCol,firstCol)
Also be careful to the name of your variable. For instance, the
variable "lastNonNul" actually represents the number of non-zero
singular values.
cheers,
gael
On Wed, Jun 12, 2013 at 9:45 PM, BRUN Gauthier <brunga@xxxxxxxxxxxxxx> wrote:
>
> Hello,
> Our project is in progress, but we have to finish our work this friday.
> We would like to have advice about our work :
>
> There are three steps in our algorithm
> - Divide : implementation of the division and fusion
> - Deflation : during the fusion, we have to compute the SVD of a matrix M as
> big as the input matrix. The deflation step transforms M in order to apply
> the third step
> - Special SVD : do a SVD on the transformed M matrix (and only works on this
> one). this SVD is much faster than others SVD.
>
> The divide step is finished but works only on matrix defined with dynamic
> rows and columns, and we are currently debugging the two others steps.
> Also we are afraid that our code may be hard to read, so we would appreciate
> if you could check it (mainly the Divide and Deflation parts) and tell us
> what you think of it.
>
> Here are the links to our work: https://bitbucket.org/gl27/eigen
>
>
>
> Best regards,
> Team GL27
> Gauthier Brun, Nicolas Carre, Jean Ceccato, Pierre Zoppitelli
>
>