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

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

2009/9/11 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.

ok, cool

>
> 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.

ok, then please let's use inplace(); it seems to be a matter of taste,
because to me it doesn't sound any more weird than ref() or inout().

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

OK, forget about them, then :)

>
> 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 ;)

Why not, that doesn't hurt.

Benoit