Currently in v3.0.* all of the aicsimageio.readers submodules / classes actually hold onto the file pointer after reading the data in, this isn't the problem, the problem is that they also hold onto the entire already read data block which leads to a more than doubling of memory when using AICSImage. After discussion with @toloudis, this is because when we originally designed the AICSImage to keep using self.reader operations to access data for the user.
A minimal example to show that there is at least a doubling of memory when using AICSImage:
from aicsimageio import AICSImage
im = AICSImage("aicsimageio/tests/resources/s_1_t_1_c_10_z_1.ome.tiff")
id(im.reader.data) # returns memory location of reader data numpy array
id(im.data.shape) # returns memory location of AICSImage data numpy array
With @toloudis most recent PR (#44), to address adding a context manager to AICSImage, and after brief discussion with @heeler, I think it is perfectly acceptable and desirable to encourage the following behavior:
from aicsimageio import AICSImage
with AICSImage("aicsimageio/tests/resources/s_1_t_1_c_10_z_1.ome.tiff") as im:
# some operation
But, I think we should make any operation that is done by the reader be an on-demand operation. If someone calls self.reader.data the reader object uses the open file pointer to re-read the data block instead of having it stored in memory. AICSImage will still store the transformed data, so that users have fast access to the transformed data but if they really want to use the reader, then they will have to read the file each time.
API "UX" changes suggested by @heeler that I agree with, would be changing reader.data to reader.get_data() and reader.metadata to reader.get_metadata() for two reasons:
- It makes it much more explicit about "Hey this will be reading the file again"
- It sets us up nicely for the long term goal of chunked reading (Ex.
reader.get_data(Z=0, T=1))
Thoughts?
Currently in
v3.0.*all of theaicsimageio.readerssubmodules / classes actually hold onto the file pointer after reading the data in, this isn't the problem, the problem is that they also hold onto the entire already read data block which leads to a more than doubling of memory when usingAICSImage. After discussion with @toloudis, this is because when we originally designed theAICSImageto keep usingself.readeroperations to access data for the user.A minimal example to show that there is at least a doubling of memory when using
AICSImage:With @toloudis most recent PR (#44), to address adding a context manager to
AICSImage, and after brief discussion with @heeler, I think it is perfectly acceptable and desirable to encourage the following behavior:But, I think we should make any operation that is done by the
readerbe an on-demand operation. If someone callsself.reader.datathereaderobject uses the open file pointer to re-read the data block instead of having it stored in memory.AICSImagewill still store the transformed data, so that users have fast access to the transformed data but if they really want to use the reader, then they will have to read the file each time.API "UX" changes suggested by @heeler that I agree with, would be changing
reader.datatoreader.get_data()andreader.metadatatoreader.get_metadata()for two reasons:reader.get_data(Z=0, T=1))Thoughts?