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

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

I'd opine that the easiest would be to make array never own the data,
rather than always own the data. In that way, it can wrap anything.

* TimeSeries returns (owned) Arrays in two places. These could return
references to its own data (const vector<double>&), that would save a copy
(they are currently copied twice, once into an owned Array, then again in
swig into a numpy array). But maybe the amount of data returned is small,
in which case a vector<double> will be fine.

* Can't support resize() without ownership, so everywhere resize is used on
a reference argument should pass a vector<double>& instead. I think this is
a few places only.

At that point, Array will be a plain wrapper of others' data.

As for your (1), it might well be possible (keep a scratch array in the
GenericFunction class perhaps). But the thing is, the shared version was no
more safe. You can't keep a reference to an Array because you don't know if
it's shared or just wrapping data that will be deleted from under you. It
was "shared or borrowed" rather than "owned or borrowed", only without the
flag to tell the cases apart.

-j.

On 24 February 2012 19:37, Garth Wells <email address hidden> wrote:

> I'm not keen on ownership flags - they were a real mess before and it's
> been a lot better since we go rid of them.
>
> Would be it be possible to (1) re-use the Array so we avoid dynamic
> allocation or (2) have an Array-like structure that always owns the data?
> The class Array was introduced to better interface with Python, but I
> recall that we later became aware that the Numpy interface has has a flag
> to prevent resizing, which would make typemaps more robust.
> --
> https://code.launchpad.net/~jobh/dolfin/fast-array/+merge/94467
> You are the owner of lp:~jobh/dolfin/fast-array.
>

« Back to merge proposal