Re: [eigen] port of (c)minpack to eigen : status

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Ok, me too, a few remarks...

First of all, as Hauke noted, there are public data members, I don't
think that's a good idea. It's true that, being a pure headers
library, we can't magically solve ABI compatibility problems by
protecting data members. But still, there are other advantages in
protecting them rather than having the user directly access them.

Also, we had a convention of prefixing all data members with m_. Once
you do that, it's cumbersome enough that you prefer to use accessor
methods ;)

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
party.

The code seems to be only for double, right? Is there a good reason to
forbid float and other types?

It's hard to make more comments, as I don't know this stuff and there
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.

The functors idea seems to have been implemented right. Then again,
not knowing about this stuff, I can't know if in this situation it's
better to use functors (so static polymorphism, resolved at compile
time) or virtual functions (so dynamic , runtime polymorphism). The
functors approach allows to have more things known at compile-time and
to make direct function calls.

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?

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.
The prerequisite for this is to make very extensive unit tests, so one
is sure not to introduce regressions.

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..?

Benoit

2009/9/24 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
> Ok, finally a few remarks...
>
> 1) I think it might be of interest for users to change the cost function at
> run-time. With the current design this is hardly possible since it requires
> to instantiate a new LevenbergMarquardt class for each cost-function.
> Pre-Instantiation of all possible optimizers is here a bit cumbersome. An
> alternative would be to define a pure virtual cost function base class.
> Actually, in our framework we have that even for optimizers since we also
> like to switch between let's say Downhill Simplex (or Nelder Mead) and
> LevenbergMarquardt. But that is really optional - though in our group we
> utilize it a lot.
>
> 2) void abort() would be nice in the optimizer's interface. There exist
> optimization procedures that take for longer than 5 minutes and here it
> makes a lot of sense to be able to terminate early. How this might be done
> a) Introduce a member: Status m_status;
> b) Replace each 'return Running;' by 'return m_status;'
> c) Replace each remaining 'return <status_type>' by 'm_status =
> <status_type>; return m_status;'
> d) Add 'void abort() { m_status = UserAsked; }
>
> 3) It seems, as if currently different functions are called for numerical
> differentiation vs. optimal storage vs. analytical differentiation. I am
> just wondering whether these special cases are not candidates for
> templating. These are basically policies, right? Like 'use as much mem as
> you want' vs. 'use minimal memory' and 'differentiate analytically' vs.
> 'differentiate by forword diffs'.
>
> 4) Optional: A callback function would be cool. Basically, some way of
> getting updates of intermediate optimization results. It oftentimes helps a
> lot in the course of debugging/tuning specific optimization problems because
> you can visualize the intermediate results.
>
> 5) Finally, I would not expose member variables directly and I am wondering
> whether all of those functions are required in the public interface.
>
> In general I like the work - it's cool to have a non-linear optimization
> based on Eigen. :) Thanks for your all the effort.
>
> Hauke
>