Re: [eigen] TriangularView::solve() interface

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

Yes with the ReturnByValue class API 2 could work without any
temporaries. In short ReturnByValue maps:

a = m.bar(b);

to

bar_impl(m,b,a);

On the other hand it is more painful to write the solve function
because you have to write a mini pseudo expression class. One question
is whether the ReturnByValue class should inherit MatrixBase.
Currently it is the case. The pro is that allows us to use it right
away in any expression, e.g:

x += 2 * m.solve(b);

The cons are that m.solve(b)[0] won't compile (that's ok) and generic
functions must have to make sure they honor the EvalBeforeNesting bit.
E.g the function foo:

template<typename Derived>
void foo(const MatrixBase<Derived>& _x)

must start by:

{
  typename Derived::Nesting x(_x.derived());

and to avoid multiple useless instanciations of the code of foo, the
second and last line of this function should be:

  return foo_impl(x);
}

Note that you can always explicitly evaluate the pseudo ReturnByValue
expression:

m.solve(b).eval();

This is important because 1- that allows to call functions which does
not honor the above rules, and 2- if you think it is safer not to make
ReturnByValue inheriting MatrixBase, the eval() function will still
allow us to use the result right away. To summary:

x = m.solve(b); // OK
x = m.solve(b).eval(); // one useless copy
x += m.solve(b).eval(); // OK
x += m.solve(b); // won't compile if ReturnByValue does not inherit MatrixBase
foo(m.solve(b)); // won't compile if ReturnByValue does not inherit
MatrixBase, and this might not work if foo() does not honor the above
rules.
foo(m.solve(b).eval()); // OK

So basically I'm in favor of safety and remove the current inheritance
to MatrixBase.


Some additional remarks:

- in case we don't manage to make API 2 work as we expect, for API 1
what about using the .ref() solution I proposed in another thread
instead of pointers ? (or .out(), .inout(), as you prefer)

- a few solvers advantageously work "inplace" (triangular, LLT, LDLT),
so shall we keep our current and explicit "m.solveInPlace(b)" or go
for an overloaded version of solve(), i.e. m.solve(b.ref()) ? This
"inplace" version of solve could even return a ref to b that'd be nice
to use the result right away in an expression: x += m.solve(b.ref());

With API 1 this is ok because the number of parameters is different,
but with API 2 it is only a matter of adding .ref() to the unique
parameter... For instance both:

x = m.solve(b);

and

x = m.solve(b.ref());

will compile but in one case b is modified and then copied to x. Yes
this is stupid but a good API should prevent stupid uses as most as
possible.

Basically, .ref() does not make it clear b is going to be modified, so
perhaps having a special .inout() still make sense ? (see the other
thread) or we don't make the "inplace" solve function returns a ref to
b and then x = m.solve(b.ref()); won't compile.

- Finally what about adding a RealScalar*  parameter to the solve
functions optionally returning an estimation of the safeness of the
result ? Here I propose a pointer to make it optional.

gael.


On Wed, Aug 26, 2009 at 1:07 PM, Markus Fröb<grey_earl@xxxxxx> wrote:
>> As I'm going to explain ASAP in a mail to this list, I believe that we
>> can drop altogether the "bool success" in all solve() methods.
>>
>> There remains the choice of an API between
>> API 1: a.solve(b,&result)
>> and
>> API 2: result = a.solve(b)
>>
>> The main argument in favor of API 1 is the one Keir mentions.
>>
>> But what about the wonderful little ReturnByValue class that Gael
>> wrote? I suggest we investigate that, at the moment I don't see why it
>> wouldn't work here! It would allow to have API 2 with the guarantee of
>> it unfolding to the same as API 1.
>>
>> Maybe Gael can comment.
>> Meanwhile, is there consensus that, if it works, then we want it?
>> Again, this is assuming that the bool return value are removed, i'm
>> going to write to the list to advocate that.
>>
>> Benoit
>>
>
> My vote for API 2 if it works.
>
> ______________________________________________________
> GRATIS für alle WEB.DE-Nutzer: Die maxdome Movie-FLAT!
> Jetzt freischalten unter http://movieflat.web.de
>
>
>
>