Re: [eigen] Unifying decomposition interfaces

[ Thread Index | Date Index | More Archives ]

2009/5/21 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
> On Thu, May 21, 2009 at 5:42 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> So there's no way around adding bool m_initialized in general. On
>> second thought, this also makes code easier to read.
> Done ...


>> (Side note: our unit tests for lu and qr currently only test
>> dynamic-size, that's bad)
> I just added fixed size tests - at least for the assert verifications.

Good, that's a starting point...

>> Another small remark is that the default constructors you added need
>> to have some (even very short) documentation. It's just scary for the
>> user when an item is undocumented.
> Done ...


> A final comment. I did not touch the SelfAdjointEigenSolver - it
> already has several undocumented ctors and I am not sure about the use
> cases for them.


> You can take a look at the final patches over here:
> I tried out the patch queues - IMHO, they rock. Only patches will be
> versioned in a patch queue - i.e. you are actually working on shallow
> copy of the Eigen repository. Each patch in a patch series can be
> pushed/poped and modified until you like them. Even now, in case you
> have some remarks left, the patches can be easily changed.

I've yet to read your link below and understand how patch queues are
better than simply examining the diff between eigen2 and a clone...

> Once I get the go, the I will apply the patches

Yes, please go ahead! Assuming that the unit-tests pass (didn't check).

> - my assumption is
> that it will result in four individual commits which I consider
> reasonable given the number of changes. If you want a single commit
> for the changes just let me know.

Do as you think makes for the most meaningful commit history. We don't
have rules for that.


> p.s. When I have time, I can write a how-to for the patch queues
> though the introduction given here is very good

Mail converted by MHonArc 2.6.19+