Re: [eigen] Geometry module - Quaternion fitting alternative

[ Thread Index | Date Index | More Archives ]

wow that's a very great contribution !

I did not read the code but I still have a few comments.

On Tue, May 26, 2009 at 3:31 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> 2009/5/26 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
>> This means that a prerequisite to vectorize that is that your matrices
>> src and dst be RowMajor.
> Oh but I overlooked the fact that dst and src are the arguments passed
> to the function. So we don't control their storage order, the user
> does, and the default is col-major. We could say that you change the
> implementation so that now the _rows_ are the points, so vectorization
> can be done in the col-major case... but that would mean interleaved
> storage, so the construction of the matrices in the first place would
> be less efficient (less memory locality) so it's probably not worth
> it.

then I would say use the highest level Eigen expression as you can,
and hope that in some situation the vectorization will be enabled. For
instance there are still a few possibilities to improve the
vectorization of partial reduction. Also note that the geometry module
already requires the Array module.

Also it would be very cool to describe the underlying algorithm in 2/3
sentences so that the user know what to expect (complexity, etc.). If
this is already the case just forgot what I said !

I'm wondering whether the Geometric module is the right place for that
? More generally I'm wondering whether we should favor few but fat
modules, or many but light ones ? For instance, if we want to go for
the few but fat solution, then we could also have a single
Decomposition module with LU, LLT, QR, SVD,, etc. !! Personally, I
prefer the "many but light" solution. This is why I'm suggesting to
put this algorithm in a new module, just like a Levenberg-Marquat
solver should have its own module. Again, this is just a suggestion...


> So I'd say don't worry about vectorization here....
> Benoit

Mail converted by MHonArc 2.6.19+