Re: [eigen] Maximum size at compile time

Hi,

if someone is willing to finalize and cleanup this PR, then we're still ok to merge it. I quickly looked at the PR and I think lot of boirlerplate codes and redundancies could be avoided by delegating the handling of the reserved memory to a base class of DenseStorage instead of messing with plain_array. It also remains to:

- make sure that this option is propagated to decompositions,

- add documentation

- (optional) add a PlainObjectBase::reserve(Index size) method enabled only if Options has the proper flag.

gael

On Thu, May 17, 2018 at 3:37 PM, Joseph Mirabel <joseph.mirabel@xxxxxxx> wrote:

Hi Adrien,

On 17/05/2018 15:01, Adrien Escande wrote:
> Hi Joseph,
>
> we have the same issue in our lab, and each time I start a new project I
> kick myself for not pushing this feature during the Eigen3 beta.
> The use of Map is not really satisfactory and error prone.

I did not think about it.

> Most critically, there is no workaround for matrix decompositions, for which
> there will be reallocation whenever the size of the matrix changes.
> Basically, there would be a need of a memory management mode in which a
> reallocation occurs only if the new buffer needs to be bigger than the
> current one. For now, the size of the buffer is supposed to be
> row()*cols(), so we would need an additional integer to keep track of
> the buffer size separately from the matrix size, which would break the ABI.

My intent is to add a flag to the Option template argument of Matrix..
This will not break ABI compatibility since it will be two different C++
types.
I did not check carefully matrix decomposition classes. I expect them to
take as template parameter a matrix type which would then contain the
memory management flag.

The only issue I see with this method is the non-compatibility between
Matrix<EnableMaxSizeAtRunTime> and Matrix<! EnableMaxSizeAtRunTime>.
Maybe it is possible to play with cast operator to fix this issue.

>
> Joris Vaillant began to tackle the problem a few years ago, but the PR
> has never been completely finished:
> https://bitbucket.org/eigen/eigen/pull-requests/125/789-memory-allocation-only-when-necessary/diff
> Maybe we could try to put it back on rail.
>

Thanks for the link. I'll have a look. Before I put too much effort on
this, I will wait for a confirmation that a patch would be welcome.

Joseph