Re: [eigen] about .lazy()

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]


On Sat, Aug 15, 2009 at 10:48 PM, Benoit Jacob<jacob.benoit.1@xxxxxxxxx> wrote:
> 2009/8/14 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>> know this is not a very nice situation, and my suggestion to solve
>> this mess is to remove the .lazy() function. Here are some arguments
>> against ".lazy()":
>>
>> a - it is generic concept, but it only makes sense for product expressions
>
> Mostly, indeed. Initially we didn't know to which expressions it would
> apply. Indeed it turns out that it's almost only product expressions.
> There are exceptions too: at least the Random expression (more
> generally any non-repeatable nullaryExpr, but afaik it's the only one
> so far).

hm, right I forgot about that, but Random only have the
EvalBeforeNestingBit flags, so do we still need a way to remove that
flag ? If so .lazy() would be perfect but if we keep .lazy() for that
purpose we'll have strong backward compatibility issues. Unless we
overload .lazy() in CwiseNullaryOp...

>>
>> c - it covers two different features at once:
>>
>>  c1 - it means that the result does not alias with the operands of
>> the product, but for that purpose it makes more sense to control that
>> via a special operator=, like res.noalias() = ...
>>
>>  c2 - it also means that the product should not be evaluated
>> immediately, but evaluated as a standard expression. However, in
>> practice it is (almost) never a good idea to do so, and when it is not
>> the case the speed difference is negligible.
>
> I don't really understand the difference between c1 and c2, indeed
> lazy() used to do 2 things, remove EvalBeforeAssigning and
> EvalBeforeNesting flags, i guess that's what you meant. Anyway it
> doesn't matter.

yes that's what I meant, and basically .lazy() would be good at
removing EvalBeforeNestingBit only (e.g., for Random) and .noalias()
to bypass EvalBeforeAssigning. Our mistake was to use .lazy() for
both.

>> So to summary, I'd be in favor in removing .lazy(), replace the
>> EvalBeforeAssignBit flag by a MightAliasBit flag, and add a no-alias
>> mechanism on the result side.
>
> OK so a.noalias() = xpr would be the way to obtain what you called the
> "optimal" solution, right?

hm, not really because:

D.noalias() = A*B + C;

 it is the same as:

D.lazyAssign( (A*B) + C);

which is the same as:

D = (A*B).eval() + C;

But here you can avoid a temporary and still have an efficient
evaluation of the product by doing:

D = C;
D.noalias() += A*B;

(best for dynamic sizes)

or:

D.noalias() += A*B;
D += C;

(best for small fixed sizes)



> I agree with this change: indeed, if we agree that lazy() only removed
> evalbeforeassigning, then indeed it's best placed in front of the = ,
> so noalias() makes perfect sense.

yes, and it is also much easier to use, you don't have to worry about
where to put the .lazy(), put the parenthesis at the right place, etc.


> However there's one (small) aspect of your changes about which I don't
> agree (at the moment): it's the renaming of EvalBeforeAssigningBit to
> MayAliasBit. The old name described exactly what it does while the new
> name seems imprecise to me. Indeed there are a lot of expressions that
> "may alias" but still don't have the MayAliasBit. For example the
> Transpose expression. And even Block if you take blocks on both sides
> of an assignment (think row(i) = col(j)). So actually, a lot of
> expressions "may alias", so the name MayAliasBit isn't what we mean.
> Also in this documentation:
>
>  /** \ingroup flags
>   *
> -  * means the expression should be evaluated before any assignement */
> -const unsigned int EvalBeforeAssigningBit = 0x4;
> +  * Means the expression cannot be evaluated safely if the result alias one
> +  * of the operands of the expression. Therefore it should be evaluated
> +  * before any assignement. */
> +const unsigned int MayAliasBit = 0x4;
>
> According to this doc, the Transpose expression should have the
> MayAliasBit! (Which i'm not advocating! Just explaining why i think
> this part of the changes is wrong).

ok, I'll revert that.

>> Finally, there is also the question whether operator+= and -= should
>> be "no-alias" by default ? This because I think that in 99% of the
>> case, when you write:
>>
>> m += <product>
>>
>> it's very unlikely that m is one of the operand of the product. The
>> drawback is that might be confusing for the user, (because operator=
>> and operator+= would behave differently wrt aliasing).
>
> Yes, i'm afraid of this drawback. I'd rather not make += noalias by
> default since = isn't.
>
> But I have a question. since a+=b expands to a=a+b, as soon as b has
> the EvalBeforeNestingBit, there can be no aliasing problem. b will
> just be evaluated into a temporary c which won't have any
> EvalBefore... bit, and from there one we have a=a+c which will be
> lazy. On the other hand, noalias() allows to ignore
> EvalBeforeAssigning on the sum expression. So basically the only case
> where noalias() might be useful in a.noalias()+=b, is a case where b
> has EvalBeforeAssigning and NOT EvalBeforeNesting. I don't think
> there's any example of that??? So basically i'm saying that I don't
> see any use case for a.noalias()+=b. Do you see any?

this is because we have overloads for += <product> and -= <product> so
that you can have:

c += (a*b).lazy()

oops, now you should rather write:

c.noalias() += a*b;

and evaluate it without temporary because this what the underlying
optimized routine does.

And yes, if you write:

c = c + a*b;

then you lost this optimization, and a*b is evaluated into a temporary.

However without noalias():

c += a*b;

is the same as:

c += (a*b).eval();




Perhaps I should also say that:

2 * (a * b)

actually returns a product expression, and not a CwiseUnaryOp because
the optimized routine performs this scaling anyway.

ah, and another details: when you write c.noalias() = a*b (with c
large enough), it is actually evaluated as:

c.setZero();
c.noalias() += 1 * a * b;


Gael



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/