search

catalystneuro / roiextractors

Python-based module for extracting from, converting between, and handling optical imaging data from several file formats. Inspired by SpikeInterface.
https://roiextractors.readthedocs.io/en/latest/index.html
BSD 3-Clause "New" or "Revised" License
12 stars 7 forks source link

Provide more friendly memory error for `TiffImagingExtractor` #352

Open h-mayorquin opened 2 months ago

h-mayorquin commented 2 months ago

This is related to https://github.com/catalystneuro/neuroconv/issues/995

Check out the lines: https://github.com/catalystneuro/roiextractors/blob/7592bbac765f19bcbf57b398c979fb9ee1f6c283/src/roiextractors/extractors/tiffimagingextractors/tiffimagingextractor.py#L59-L65

Sometimes this will fail because there is not enough memory.

Two options:

  1. You can wrap this into something like this: https://github.com/catalystneuro/neuroconv/blob/ef656a0f13607ec661b091b9569e326fc91af8db/src/neuroconv/tools/spikeinterface/spikeinterface.py#L520-L548

Where we provide a friendlier error message and the suggestion that is currently a warning.

  1. Another option is to delay this intialization until get_video gets called. This is likely to be a small call and the error and will avoid most problems.

I suggest 1 because is easier but I think memmaps should not be dangling with the object so I think 2 is better long-term. We had some problems because of this in spikeinterface.

CodyCBakerPhD commented 2 months ago

Note that (2) still wouldn't be viable in this users case since it would trick them into thinking everything is working until they attempt to create a file (initialization of the interface will work quickly without error on NeuroConv, meaning the GUIDE will let them input metadata and all that)

Unless you know of how to load only a partial amount of the tiff file as an array (corresponding to the range of the get_traces call) in that case?

h-mayorquin commented 2 months ago

There is a lot of functionality to do this in the library: https://github.com/cgohlke/tifffile/blob/d173acd27afcc26f58c489fe6b74bb831e308f93/tifffile/tifffile.py#L4383-L4393

Sadly it is not very well documented.

I am pretty sure we could work it out though if it needs be

However, there was this concerning thing a while back that puts me pause in that direction https://stackoverflow.com/questions/72581592/what-to-do-when-gohlkes-python-wheel-service-shuts-down

Which prompted: https://github.com/catalystneuro/roiextractors/issues/171

CodyCBakerPhD commented 2 months ago

Exactly, I think best long term solution is to refactor the base tiff library to use something that is more memory friendly from the start

https://github.com/denisecailab/minian had some good examples of this as I recall

h-mayorquin commented 2 months ago

Good point!