Re: [eigen] nesting

[Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>]

The only persistent bug is adjoint_4 and this one is really
unfortunate. We are back to the need of NestByValue() or .eval().
Removing

| NestParentByRefBit
| EIGEN_PROPAGATE_NESTING_BIT(Lhs::Flags|Rhs::Flags)

from the ProductBase::Flags fixes the issue.

This line

VERIFY_IS_APPROX((m1.adjoint() * m2).adjoint(),           m2.adjoint() * m1);

and in particular this expression

(m1.adjoint() * m2).adjoint()

causes the crash. Adding .eval() solves the issue as well - of course,
since nothing will be nested by reference anymore. Ultimately I think
we need the expression analyzer. Crashes like this are really nasty
and one of the core reasons we wanted to have copy by value. Or am I
wrong?

- Hauke

On Sat, Feb 6, 2010 at 5:15 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> 2010/2/6 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
>> I am ready to push. These tests are currently failing (GCC 4.4.1)
>>
>> 98% tests passed, 8 tests failed out of 380
>>         84 - adjoint_4 (Failed)
>>        182 - stable_norm_1 (Failed)
>>        183 - stable_norm_2 (Failed)
>>        184 - stable_norm_3 (Failed)
>>        185 - stable_norm_4 (Failed)
>>        197 - lu_2 (Failed)
>>        375 - nullary_7 (Failed)
>>        376 - nullary_8 (Failed)
>>
>> Stable norm is always failing over here for the others I am not sure.
>> I don't want to look into the failing tests right now but I'ld like to
>> push what is there so far. Any concerns regarding pushing?
>
> I'm not so concerned about stable_norm as it is a non central feature,
> but the assortment of other test failures suggests that something
> really central is still not right.
>
> Could you, for now, rather push to a fork?
> Just create a fork on bitbucket and do
> cd eigen
> hg push https://clone/url/of/new/fork
>
> Thanks
> Benoit
>
>
>>
>> TODO:
>> - long term we still might need Gael's full expression processor approach
>> - regression tests for product related expressions and the number of temporaries
>> - maybe a new unit test, checking the nesting type
>> - verify that the failing tests have nothing to do with the applied changes
>>
>> - Hauke
>>
>> On Sat, Feb 6, 2010 at 3:48 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>>> Wow, this is a great test!
>>> I haven't tried to think if it is enough or if you should add
>>> something, but at least it's a great piece of testing code.
>>>
>>> Benoit
>>>
>>> 2010/2/6 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
>>>> It is working. Now I just have to double check all Flag definitions
>>>> within Eigen, something around ~53, since potentially I need to add
>>>> the EIGEN_PROPAGATE_NESTING_BIT define.
>>>>
>>>> I could also do a lot of testing - what do you think how much is sane?
>>>> See the attached file which basically tests Replicate, Reverse, Select
>>>> in combination with and without different kinds of products. I think
>>>> testing every single combination is a little bit overkill - what do
>>>> you think?
>>>>
>>>> - Hauke
>>>>
>>>> On Sat, Feb 6, 2010 at 12:38 PM, Hauke Heibel
>>>> <hauke.heibel@xxxxxxxxxxxxxx> wrote:
>>>>> It should have been "toggle only the storage order."
>>>>>
>>>>> Since we now have bit which are not inherited, we might rename
>>>>> HereditaryBits bits and introduce a new set. How about
>>>>>
>>>>> DominantBits /* always inherited */
>>>>> RecessiveBits /* only in rare cases inherited */
>>>>>
>>>>> Then, in the future one could do
>>>>>
>>>>> Flags = OtherType::Flags & ~RecessiveBits
>>>>>
>>>>> to disable only those we don't want to inherit. This will of course
>>>>> only then make sense, when there is the possibility for new recessive
>>>>> bits in the future or just because we like it...
>>>>>
>>>>> - Hauke
>>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
>
>
>