Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide more friendly memory error for TiffImagingExtractor #352

Open
h-mayorquin opened this issue Aug 13, 2024 · 4 comments
Open

Provide more friendly memory error for TiffImagingExtractor #352

h-mayorquin opened this issue Aug 13, 2024 · 4 comments

Comments

@h-mayorquin
Copy link
Collaborator

This is related to
catalystneuro/neuroconv#995

Check out the lines:

except ValueError:
warn(
"memmap of TIFF file could not be established. Reading entire matrix into memory. "
"Consider using the ScanImageTiffExtractor for lazy data access."
)
with tifffile.TiffFile(self.file_path) as tif:
self._video = tif.asarray()

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
Copy link
Member

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
Copy link
Collaborator Author

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:
#171

@CodyCBakerPhD
Copy link
Member

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
Copy link
Collaborator Author

Good point!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants