Re: [eigen] still the solve() API debate

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

On Fri, Sep 11, 2009 at 3:44 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> 2009/9/11 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>> On Fri, Sep 11, 2009 at 3:22 PM, Hauke Heibel
>> <hauke.heibel@xxxxxxxxxxxxxx> wrote:
>>> On Fri, Sep 11, 2009 at 3:13 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>>>> Same thing when we do A.solveInPlace(b.start(n)), start() returns a
>>>> temporary, same problem, whence the idea of inplace() or whatever we
>>>> call it.
>>>
>>> Ok, I just caught up with re-reading... that's indeed ugly. But as you
>>> guys already observed, in new compiler versions we already have rvalue
>>> references.
>>
>>> Benoit, what about your idea of passing Matrices by
>>> reference and MatrixBase by value? You suggested to do that in the
>>> "MatrixBase::swap - why const" thread? This approach would allow an
>>> easy transition towards rvalues references in the future.
>>
>> I've thought about it and I really don't see how to it safely and in a
>> generic way, and I really doubt that's possible.
>>
>>
>>
>> I also have to say that my Reference<> solution becomes very tricky as
>> soon as you want to restrict your function to a MatrixBase<>, e.g.:
>>
>> template<Derived> Derived& solve(Reference<Derived> b);
>>
>> will match any type which can be explicitly converted to a Reference,
>> while we usually want to do:
>>
>> template<Derived> Derived& solve(Reference<MatrixBase<Derived> > b);
>>
>> but if you declare your function like this,
>>
>> MatrixXf m;
>> A.solve(m.ref());
>>
>> won't compile because the type of m.ref() is Reference<Matrix<...> >.
>> The unique way to make it working is to make Reference<Matrix<...> >
>> inheriting Reference<MatrixBase<Matrix<...> > >. (this is because the
>> solve function is template, and casting by inheritance is only kind of
>> casting which is allowed to select the function to  instanciate)
>
> Wait, why so complicated?
>
> I understand that in an ideal world we would restrict all our template
> functions to the types that make sense for them. In practice, c++
> doesn't make that easy at all (i guess the "concepts" were meant to
> address that) and we often don't do it, already. For exemple the
> current solve() methods take a ResultType parameter that can be any
> type:
>
> template<typename Other, typename ResultType>
> void solve(const MatrixBase<OtherDerived>& b, ResultType *result)
>
> it's not the most elegant, but it's very simple and we've never seen a
> user complain about that!
>
> So why not do the same here??
>
> template<typename Other, typename ResultType>
> void solve(Reference<ResultType> result)
>
> Thus we have already restricted to Reference<T>. Since the user
> usually gets his Reference object by calling inplace() or ref(), and
> he can only call that on MatrixBase objects, there's little chance
> that he might ever construct a bad Reference type: he would have to do
> it explicitly.
>
> By the way, if you agree with my inplace() suggestion, we might also
> make it as separate type InPlace instead of Reference, to further
> control that the user is doing the right  thing (if we also have a
> Reference for other uses).


ok then that's fine.

About "ref", vs "inout", vs "inplace", I don't know... "inplace" is
nice because it allows to safely remove the "InPlace" suffix, but it
sounds a bit weird to me... actually I don't care much.

About your new swap proposal, they look very odd to me.

Finally, I don't remember if I've already mentioned that, but one
alternative is to define:

#define EIGEN_INOUT const

and then declare:

template<OtherDerived> MatrixBase::swap(INOUT MatrixBase<OtherDerived>& other);

Of course that still does not allow to honor constness, but at least
that makes the source code and doc good looking ;)

gael.