Pickle API: Avoid static Series hack, allow multiple Series #1633
Conversation
franzpoeschel
force-pushed
the
unpickle-multiple-series
branch
2 times, most recently
from
e5ea191
to
010cacd
Compare
franzpoeschel
force-pushed
the
unpickle-multiple-series
branch
2 times, most recently
from
c465a65
to
d04a687
Compare
There was a problem hiding this comment.
@franzpoeschel Just tested and it works. I can now change series within one kernel instance.
Btw. I didn't new that, but one can install the api from a specific branch with pip like this pip install git+https://github.com/franzpoeschel/openPMD-api.git@unpickle-multiple-series, pretty cool.
|
Nice, thank you for testing this!
I didn't know that either. But I guess it means that you need to install ADIOS2 or HDF5 manually before that since they have no Pip packages? We usually just build them into the Wheel, but that doesn't help when compiling manually. |
I have installed adios2, hdf5, and blosc with conda. But see, https://pypi.org/project/adios2/ there is adios on pypi. |
franzpoeschel
force-pushed
the
unpickle-multiple-series
branch
from
d04a687
to
6089bbd
Compare
Ah right, they actually have a package by now. Maybe we should add this as an optional dependency in our Pip-package @ax3l |
franzpoeschel
force-pushed
the
unpickle-multiple-series
branch
from
6089bbd
to
e5c705b
Compare
| template <typename T> | ||
| T &makeOwning(T &self, Series s) |
There was a problem hiding this comment.
Let's add a nice doxygen string here :D
|
|
||
| namespace internal | ||
| { | ||
| template <typename T> |
There was a problem hiding this comment.
maybe document the idea here with a little inline comment
| // for DASK. | ||
| static auto series = openPMD::Series(filename, Access::READ_ONLY); | ||
| return seriesAccessor(series, group); | ||
| /* |
There was a problem hiding this comment.
let's isolate this to a separate PR which goes into the 0.17 development branch
franzpoeschel
force-pushed
the
unpickle-multiple-series
branch
2 times, most recently
from
645519d
to
1a18e76
Compare
franzpoeschel
force-pushed
the
unpickle-multiple-series
branch
from
1a18e76
to
0f99488
Compare
Close #1458
Theoretically, it should be possible to modify our public API such that each handle, not only the Series handle would keep alive the Series, but that would require some greater internal restructuring.
Instead, this PR avoids the problem of finding a place to store the
Seriesnot by putting it in a static variable, but by hiding it inside the returned value's destructor. This does not lead to a reference cycle because the returned handle is a non-owning shared pointer whose destructor lambda owns all actual data. As soon as the handle is let go, the destructor is run and the owned objects are freed.static(or betterthread_local) to cache the latest opened Series for workflows that unpickle lots of single objectsSeriesandIteration@pordyna Can you check if this helps your use case?