Re: [eigen] Re: Today's joke

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

2010/6/11 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
> 2010/6/11 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>>
>>
>> On Fri, Jun 11, 2010 at 2:37 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> wrote:
>>>
>>> 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.
>
> Ah yes, at some point I thought about that use case, but forgot.
>
> What we can do is adopt explicit names, for what these macros are
> doing. One is preferring Dynamic, the other is preferring fixed
> values. Based on that, a good English speaker might be able to come up
> with good names :)
>
> Naive attempt:
>  EIGEN_SIZE_MIN ---> EIGEN_MIN_PREFER_DYNAMIC
>  EIGEN_SIZE_MIN ---> EIGEN_MIN_PREFER_FIXED

I mean:
EIGEN_MAXSIZE_MIN ---> EIGEN_MIN_PREFER_FIXED

Benoit

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