SpikeInterface / spikeinterface

A Python-based module for creating flexible and robust spike sorting pipelines.
https://spikeinterface.readthedocs.io
MIT License
527 stars 187 forks source link

Can't use docker sorters on nwb files #2628

Closed chesnov closed 7 months ago

chesnov commented 7 months ago

I am using a windows machine and try to sort from an nwb file, but get the following error:

SpikeSortingError: Spike sorting in docker failed with the following error: ... ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/root/.local/lib/python3.11/site-packages/spikeinterface/extractors/nwbextractors.py", line 784, in _fetch_other_properties from pynwb.ecephys import ElectrodeGroup ModuleNotFoundError: No module named 'pynwb'

I have attempted this with kilosort 2.5, 3, and 4, but the problem could be present in other docker container sorters as well. None of them seem to have the pynwb package installed in the container. My hacky workaround is to modify py script in runsorter.py to have subprocess.check_call(['pip', 'install', 'pynwb']) at the top of main (and import subprocess with imports), but I feel like there should be a more elegant way of fixing this.

samuelgarcia commented 7 months ago

Hi, The easiest to resolve this is to create you own docker image (that derive from ours) that also include the spikeinterface version you want + other deps like pynwb or h5py (which is faster)

h-mayorquin commented 7 months ago

Another solution that will help you to avoid possible headaches is to convert to a binary format using recording.save and then run the docker on that version of the recorder. The downside of that approach is that you will need to have as much disk space as your recording object requires.

samuelgarcia commented 7 months ago

And also I wrote recently a install_package_in_container() function that will help a lot for this case but we need to propagate this function in sorte_sorter() with a proper signature. (which should be very little works but long discussion on the signature...)

samuelgarcia commented 7 months ago

Oups, this is already done!!! My bad.

@chesnov : can you try this run_sorter(..., extra_requirements=["pynwb", "h5py"])

this internanlly use the install_package_in_container() using pypi.

h-mayorquin commented 7 months ago

That's great @samuelgarcia ! Let's see if that works for @chesnov

alejoe91 commented 7 months ago

We should add it as a requirement of the NWBRecordingExtractor, which BTW should not require pynwb by default

h-mayorquin commented 7 months ago

You mean in the pyproject.toml file?

alejoe91 commented 7 months ago

no, in the self.extra_requirements of the extractor ;) We probably left it behind in the various refactoring. See this example

The self.extra_requirements get automatically installed in the container

h-mayorquin commented 7 months ago

Good to know. I thought that that features was a relic from spikeextractors that is not used anymore.

chesnov commented 7 months ago

Oups, this is already done!!! My bad.

@chesnov : can you try this run_sorter(..., extra_requirements=["pynwb", "h5py"])

this internanlly use the install_package_in_container() using pypi.

Thank you! This solution worked for me!

alejoe91 commented 7 months ago

I reopen because we need a fix in the extractors