[WIP] Array4: __array_interface__ #17
Conversation
Implement the `__array_interface__` in Array for.
ax3l
force-pushed
the
topic-arrayBufferProtocols
branch
from
afa91ed to
864a6ff
Compare
There was a problem hiding this comment.
Looking good @ax3l seems like numpy <-> Array4 roundtrip is working already?
Currently, I see a conflict between how a user would expect to index into an array in Python vs. AMReX C++ code. In AMReX we expect to index using a Box so the starting index will not be zero. And in presence of ghost cells, negative indices will yield the ghost cells on the lo side.
| [](Array4<T> const & a4) { | ||
| std::stringstream s; | ||
| s << a4.size(); | ||
| return "<amrex.Array4 of size '" + s.str() + "'>"; |
There was a problem hiding this comment.
This should indicate the polymorphic type.
| auto shape = py::make_tuple( // Buffer dimensions | ||
| len.x < 0 ? 0 : len.x, | ||
| len.y < 0 ? 0 : len.y, | ||
| len.z < 0 ? 0 : len.z//, // zero-size shall not have negative dimension | ||
| //a4.ncomp | ||
| ); |
There was a problem hiding this comment.
How does this handle ghost cells?
There was a problem hiding this comment.
Not yet at all, let's have a VC about how you solved it in the past.
So far I am mainly experimenting with the data interface (trying the GPU version next)
|
|
||
| auto a4 = std::make_unique< Array4<T> >(); | ||
| a4.get()->p = (T*)buf.ptr; | ||
| a4.get()->begin = Dim3{0, 0, 0}; |
There was a problem hiding this comment.
This probably needs to be adjusted to a box index space
There was a problem hiding this comment.
Note that this function here is mainly to support views into numpy and other python buffer objects via Array4.
In #19 currently mapped to a zero-based index space for incoming buffer protocols.
Potentially can implement shift-like functions later on to adjust the index space if we like to.
| py::buffer_info buf = arr.request(); | ||
|
|
||
| auto a4 = std::make_unique< Array4<T> >(); | ||
| a4.get()->p = (T*)buf.ptr; |
There was a problem hiding this comment.
If I read this correctly, this aliases the Array4 to the python buffer. Idk enough about the buffer protocol, so I am asking here: will arr be kept alive as long as this instance of pyamrex.Array4?
There was a problem hiding this comment.
Also what happens when the user changes to contents of arr?
There was a problem hiding this comment.
Thanks, at the moment (and I am just trying the design of the data access) I made the Python Array4 behave like the C++ Array4: it's purely a view to data that is managed elsewhere.
If you create a numpy array and pass it to an Array4, the data is owned by Python. If you return an Array4 that is created on from C++ data, then the C++ side owns the data.
This might not be the final design because it can shoot Python users in the foot and I'll discuss with @sayerhs (feel free to join!) how to design this in the end. In his prior design, he basically did not expose Array4 as a user-creatable type at all but instead used it to return views to C++ data only. This sounds to me like something we might want to do again - but I need to catch up with him to see the details.
| # assert(arr[1, 1, 1] == 42) | ||
|
|
||
| # copy to numpy | ||
| c_arr2np = np.array(arr, copy=True) |
There was a problem hiding this comment.
Dumb question -- but I need to learn eventually -- is this the "official" way to define a amrex -> python deepcopy (i.e. using the np.array constructor)? Or do we need to define a dedicated deepcopy also?
There was a problem hiding this comment.
Followup question: do we need to define a non-aliasing constructor (i.e. python -> amrex deepcopy)?
I can imaging that for data analytics applications we might want to keep a time series in python (looking at you machine learning folks) -- for that we need the former (amrex -> python) which is covered by this.
For experimental science applications we frequently have python provide a data source. A python -> amrex deepcopy (probably a simple copy constructor?) would be helpful in ensuring that the data is owned by amrex.
Come to think of it: A lot of algorithms in amrex are involve creating copies of data and then applying functions to them. Users might want to preserve this workflow.
My opinion is that this case best approach would be to expose the Array4 copy constructor (assuming that hasn't been deleted), and settle on the workflow: i) shallow-copy into amrex; ii) call copy constructor to create a deep copy.
There was a problem hiding this comment.
I added some details above. This is just a view at the moment, the same way as the C++ Array4 is a plain view to non-owned data.
I just added the numpy overload so I can test around with it, maybe in the end we do not let users create this type directly because it will easily lead to memory issues if used wrongly (and segfaulting interpreters are never well received by users, even if they caused it by not following an API contract).
|
Discussed today:
Refs.:
# Looping
for mfi in mfab:
bx = mfi.tilebox()
marr = mfab.array(mfi)
for i, j, k in bx:
mar[i, j, k, 0] = 10.0 * i
mar[i, j, k, 1] = 10.0 * j
mar[i, j, k, 2] = 10.0 * k |
|
|
||
|
|
||
| template< typename T > | ||
| void make_Array4(py::module &m, std::string typestr) |
There was a problem hiding this comment.
Discussed in #19: we want to make sure that users do not construct Array4s on their own and only get them from MFIter loops
|
This is now merged into #19 and improved therein. |
__array_interface__Array4construction__cuda_array_interface__Close #9