Re: [eigen] bug in data() on col/row

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

oh, I completely forgot about .stride(). So it was a conscious
decision to allow data() even on non-contiguous blocks.

Benoit

2009/8/30 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
> hi,
>
> actually, it does not make sense to use .data() without .stride()
> unless you know what you are doing (i.e., your data is sequentially
> stored in memory).
>
> However, yes I know that:
>
> 1 - there is a bug in .stride() for vectors
>
> 2 - the doc about .data() and .stride() have to be improved
>
> gael.
>
> On Sun, Aug 30, 2009 at 5:54 PM, Benjamin
> Schindler<bschindler@xxxxxxxxxxx> wrote:
>>
>>
>> Benoit Jacob wrote:
>>>
>>> 2009/8/30 Benjamin Schindler <bschindler@xxxxxxxxxxx>:
>>>
>>>>
>>>> Hi
>>>>
>>>> Well, I know that this was my expectation - and I'm fully aware that this
>>>> code is kinda bogus. But on the other hand, writing
>>>>
>>>> somematrix.row(2).data() does give the expectation that you get that row
>>>> data (as this is what the code is trying to tell me).
>>>> That's why I wrote that mail - I think that code is unsafe and can lead
>>>> to
>>>> "unexpected" behaviour and if there is something Eigen can do about this,
>>>> it
>>>> should be done
>>>>
>>>
>>> Ah, OK, I understand your point. Sorry I didn't get it the first time.
>>>
>>> But what can Eigen do, when it sees data() being called on a
>>> non-memory-contiguous block, such as a row in a column-major matrix?
>>>
>>> Should it just forbid that (with an assertion) ?
>>> That's open for discussion. There is an argument against, though:
>>> there's a use case for using data() to retrieve the address of the
>>> first coefficient in the row, and then iterate by incrementing this
>>> pointer by the size of a column, to traverse the row.
>>>
>>
>> Now when I think more about it, the problem of detecting whether a block is
>> contiguous or not is not that easy. If the block is contiguous, the code is
>> just fine, so it should be accepted. So throwing an assert based on that if
>> would make sense to me.
>> For your use case: I think the use case is valid, but there are many other
>> ways of doing this, like: &m(2,0) + cols*i (which I consider a much better
>> way as the code tells you directly what storage order is expected)
>>
>> Opinions?
>>
>> Benjamin
>>
>>
>>
>>> Benoit
>>>
>>>
>>>
>>
>>
>>
>
>
>