|Re: [eigen] Re: Today's joke|
[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
- To: eigen@xxxxxxxxxxxxxxxxxxx
- Subject: Re: [eigen] Re: Today's joke
- From: Gael Guennebaud <gael.guennebaud@xxxxxxxxx>
- Date: Fri, 11 Jun 2010 19:16:37 +0200
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:mime-version:received:in-reply-to :references:from:date:message-id:subject:to:content-type; bh=ex86f1wAkOvTmFXn1HOtmD1C0PD/jUKU3rww/msoq38=; b=tqjeHN0O0Foqnmsr17fHUzkxzl2wfjM3QgjNgOU8rclZ0VWuYuFoneJr8V0X5uT/Jl XHOB+EhVzmxBaE08DrndIE6asTdDGCtOmr+3THGxXfNpTzpqrjElXIpAYurP3croEsjn XQ3DDYZekzoI0HKTiTad42p035p6MPzvFBXTo=
- Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type; b=mH5a287h0OubznB/byd59dTRUgVQaKpUYOc+hmbjV0iT+Q+WsQY284D2CEyVXAVJnQ fG/TyIfYROs2S4oi3tZXFS0Xej1rjFMU76XljeTxKQU4WbFtgfjjZ2Ka5uJfOa2SncaU BWSN+Q24bycSK7+uCyFJjv1/Xjk1EsX0lqvS4=
On Fri, Jun 11, 2010 at 2:37 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
This is now checked in: the value of Dynamic has been changed to -1.
A couple of practical consequences:
- be careful that expressions like RowsAtCompileTime<=4 are now true
in the Dynamic case, so make sure to check explicitly for Dynamic.
- when writing loop meta-unrollers, use 0, not -1, as the stop case.
This was an occasion to sanitize some code: the macros
EIGEN_ENUM_MIN/MAX were abused, since they never were fit to check
compile-time sizes where the special values Dynamic, 0, and 1 need to
be special-cased. So they have been renamed to
EIGEN_PLAIN_ENUM_MIN/MAX, the EIGEN_SIZE_MIN macro has been polished,
and a new macro has been introduced: EIGEN_MAXSIZE_MIN. Explanation:
when you try to get the min between some fixed value, say, 4, and
Dynamic, do you expect the min to be 4 or Dynamic ? If you are
checking something like RowsAtCompileTime, then it should be Dynamic,
because a dynamic block in a 4x4 matrix has a Dynamic number of rows.
But if you are checking something like MaxRowsAtCompileTime, then you
want the min to be 4. Because a dynamic block in a 4x4 matrix has at
most 4 rows. In conclusion:
- use EIGEN_SIZE_MIN when comparing sizes-at-compile-time. It treats
Dynamic as a small value. Only 0 and 1 are treated as even smaller
than Dynamic, as special cases.
- use EIGEN_MAXSIZE_MIN when comparing max-sizes-at-compile-time. It
treats Dynamic as a large value.
I'm not sure about these rules. Actually it seems to me that such a EIGEN_SIZE_MIN is meant for very specific cases. For instance there are many lines like:
InnerSize = EIGEN_SIZE_MIN(_LhsNested::ColsAtCompileTime, _RhsNested::RowsAtCompileTime)
which now are wrong, because here since the sizes are compatible the min of Dynamic and 4 has to be 4. So Perhaps, EIGEN_SIZE_MIN should be renamed to a more explicit one, but I have no good ideas.
2010/5/30 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
> Hi again,
> so, while doing the index types change, I realized something that's
> going to make me take a more humble approach in my rationalizations in
> the future :)
> Many people asked why the strange value 33331 for the constant
> Dynamic. An explanation is given in the code (see Constants.h), but
> that basically has a lot to do with the assumption that indices are at
> least 32bit wide. Of course, since today, that is no longer always
> true. For now the only valid use case for changing that is in the
> Sparse module, where we don't use that Dynamic value much, but still,
> it's unsafe. And since it's a usual template parameter value for
> Matrix, it impacts a lot our ABI. In short, once Eigen 3.0 is
> released, we can't tweak that value anymore. So having it rely on
> technical details is not comfortable.
> So i think after all I'll follow people's sound advice and change it to -1 !!!!
> My big argument against that is that e.g. (RowsAtCompileTime <= 4)
> will no longer mean what it seems to (it will be true in the Dynamic
> case !!!) but that is fortunately not a kind condition that we use too
> frequently in the code. Should be a reasonably small change.