Code review comment for lp:~jobh/dolfin/fast-array

Revision history for this message
Joachim Haga (jobh) wrote :

>> I'm not very happy with our current use of Array. We use std::vector
>> in some places and Array in other places. I don't remember what the
>> original intention was with Array. Is the purpose just to be able to
>> pass shared data as arrays back and forth to Python?
>
> Yes.

The current usage is:
- As input (Python->C++) it wraps the numpy data.
- As output (C++->Python) the data is copied into a new numpy array.

I don't know off-hand if the input case is a requirement (that C++
modifies the data) or just an optimisation.

>> Or should it be a
>> general array class used throughout DOLFIN C++?
>
> No - there is no point in re-inventing std::vector. We should use
> std::vector where possible. It's more flexible and can be used with
> STL algorithms.

I agree in general, but note that there is no way to make a
std::vector wrap numpy data like in the input case above. An Array --
even if it's just a typedef to std::pair<T*,uint> -- may be nice to
standardise how wrapped data is passed.

> 1) Dynamic allocation in order to interface with UFC. Using plain
> pointers rather than smart pointers is likely faster, but I think that
> it's still not a solution. We should try to avoid these allocations in
> loops.

There is no dynamic allocation of actual data, not before and not now.
There was, however, dynamic allocation of the shared_array reference
count which is heap allocated. That is what this merge request fixes.

-j.

« Back to merge proposal