Re: [eigen] How to resize a partially fixed matrix

[Markus Benjamin Fröb <grey_earl@xxxxxx>]

Am Mittwoch, 24. Juni 2009 schrieb Benoit Jacob:
> 2009/6/24 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
> > On Wed, Jun 24, 2009 at 6:06 PM, Markus Fröb<grey_earl@xxxxxx> wrote:
> >> 24.06.09 17:55:10 "Benoit Jacob" <jacob.benoit.1@xxxxxxxxx>
> >>
> >>> I tried to do that change and now I think that it's very nontrivial:
> >>> not sure anymore that it's a good move!
> >>> Here's what makes me change my mind. Look at what happens when you do:
> >>>
> >>> Matrix<float,3,4> m;
> >>> m.resize(4);
> >>>
> >>> How to interprete this? Should this be a NOP or a failed assertion?
> >>> There are many such issues in Matrix.h alone and I am afraid that any
> >>> way to allow that will result in a much more complex Matrix.h and less
> >>> consistent API....
> >>>
> >>> Benoit
> >>
> >> I vote for a failed assertion here and still do the change otherwise.
> >> The single-argument resize already tests if it's a vector, no?
> >> So I think the change shouldn't be too big ...
> >
> > same here.
>
> Here is the current code:
>
>     inline void resize(int size)
>     {
>       EIGEN_STATIC_ASSERT_VECTOR_ONLY(Matrix)
>       if(RowsAtCompileTime == 1)
>         m_storage.resize(size, 1, size);
>       else
>         m_storage.resize(size, size, 1);
>     }
>
> As you can see, we forgot(!) so far to add the assertion!
> Here, the assertion would be :
>     ei_assert(SizeAtCompileTime == Dynamic || SizeAtCompileTime == size);
>
> Here is a way to implement the proposed change:
>
>     inline void resize(int size)
>     {
>       EIGEN_STATIC_ASSERT_AT_MOST_ONE_DYNAMIC_DIM(Matrix)
>       if(RowsAtCompileTime != Dynamic)
>         m_storage.resize(size*RowsAtCompileTime, RowsAtCompileTime, size);
>       else
>         m_storage.resize(size*ColsAtCompileTime, size, ColsAtCompileTime);
>     }
>
> The question that's up for discussion now is: what assertion to add here?
>
> As far as I can see, there is no good choice. Indeed, we want this to work:
>
>     Matrix<float,Dynamic,2> m;
>     m.resize(3);
>
> but then we also want this to work:
>
>     Matrix<float,3,2> m;
>     m.resize(3);
>
> Indeed, it could happen that the user notices that in a certain
> situation the first dimension is already known to be 3 at compile
> time, but he still wants to call on such a matrix a generic function
> that he wrote for the general case!
>
> This has been a design principle for a long time in Eigen and I think
> that we should stick with it:
> "Making a dynamic dimension fixed (keeping the same value) should work."
> But as discussed above, the proposed resize(int) change is
> incompatible with that.
>
> Opinions?
>
> Otherwise, I'll add the assert in resize(int) and update the
> documentation with what Tim Hutt proposes.
>
> Cheers,
> Benoit

I don't see why the second case should work, since a) it's ambigous what the 
user wanted and b) in the documentation it says explicitly "Of course, fixed-
size matrices can't be resized."

Of course, the user may call a generic function - but for such a function to 
work with generic matrices he always had to call resize(int, int) now, so that 
isn't a problem for old code.

So I'm strongly for your code change, but changing the assertion to 
EIGEN_STATIC_ASSERT_EXACTLY_ONE_DYNAMIC_DIM(Matrix)

But I'm also fine with separate resizeCols(int) and resizeRows(int) or the 
nochange variant, or both.

Greetings, Markus