|Re: [eigen] port of (c)minpack to eigen : status|
[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
In data giovedì 24 settembre 2009 21:52:02, Benoit Jacob ha scritto:
> First of all, as Hauke noted, there are public data members, I don't
mm, ok, i'll have a look at this! and use the m_* convention as well.
> Speaking of coding style, the indentation rules vary from place to
> place, sometimes it's Eigen-ish, sometimes not at all. Obviously I'd
> ask, please to what you can to Eigen-ify that, but again I don't want
> to put a big burden on you as part of that code comes from a third
My answer to this is an answer to several other remarks as well. All this code
is a port from cminpack. But not everything is ported. The high-level stuff is
'ported' (it is c++, uses as much eigen stuff as possible, remove loops, and
so on). But the internal are not. For example the qr stuff is not ported
because i intend to remove it at the end. so no need to care about eigenize it
Those are considered 'ported' :
HybridNonLinearSolver.h LevenbergMarquardt.h lmpar.h chkder.h
Those are not considered 'ported' :
fdjac1.h : compute forward-diff for n*n jacobian, handle banded cases
fdjac2.h : compute forward-diff for m*n jacobian, does NOT handle banded cases
qform.h : compute Q from qr factorization
qrfac.h : col-pivoting householder qr factorization, provided as 'compact'
form, in the same way as eigen, but not using the exact same representation
qrsolv.h : solve Ax=b using qr factorization, or least-square solution if
system is not full-rank
r1mpyq.h r1updt.h rwupdt.h : those methods provide (i think) update to the qr
factorization, using givens rotation. It seems they mix householder and givens
All this eigenization / indentation is pretty trivial, especially thanks to
unit tests, so i'm not really bored about those. Also i'm sure lot of people
would be happy to help there. I'm really more into the API stuff and other
internal changes such as using eigen qr decompozition.
> The code seems to be only for double, right? Is there a good reason to
> forbid float and other types?
On the contrary it should not, why do you say so ? The tests are only using
double, cf next answer. The code is using the template parameter 'Scalar'
everywhere (or should, anyway)
> are very few comments in the code. For example, you mentioned that
> it's hard to reuse the ColPivotingHouseholderQR here, i would like to
> hear about the specifics so I can try to adapt it. Is it in ei_r1updt?
> It's hard to understand what this function does exactly.
you can find documentation at the bottom of
It's hard because they splitted the qr stuff in several files and i'm not sure
how to adapt those using eigen. This is mostly a problem of me understanding
cminpack, not eigen's fault. The files are qform, qfac, qsolve, r1mpyq.h
> At some places there is question of "superdiagonals" and
> "subdiagonals". I don't understand this code, but if this has to do
> with a band matrix, have you investigated the new BandMatrix class?
This is only done in the forward-differentiation, which needs to be done
> There are tons of 'for' loops that can and should be replaced by Eigen
> expressions, but that's not a blocker, can be finished later.
indeed. see my previous answer on the topic :-)
> The prerequisite for this is to make very extensive unit tests, so one
> is sure not to introduce regressions.
i agree. i've tried to make a lot because of this.
> By the way, your unit test looks rather good. I just don't know how
> extensive it is (but at least it's big!). It's also funny that it
> works on prescribed instead of random matrices, i guess that comes
> from minpack..?
there are two kinds of tests : those coming from cminpack examples, and those
coming from the nist set. See my original mail for a description of those.
(from second mail)
> I guess I saw a comment mentioning that the ei_... functions should
> become member methods; obviously I support that; i'd like to mention
> that this would allow you to get rid of the wordy
> Matrix<Scalar, Dynamic, Dynamic>
> etc... that you have to do all the time; instead you could have a
> typedef "Matrix" for that, once and for all.
I intend to do that, but i first need all internals to be ported/uniform,
which they are not yet.
thanks for your feedback!,
Thomas Capricelli <orzel@xxxxxxxxxxxxxxx>