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

[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
>>
>>
>>
>
>
>