rochefort-lab / fissa

A Python toolbox for Fast Image Signal Separation Analysis, designed for Calcium Imaging data.
GNU General Public License v3.0
29 stars 26 forks source link

Stuck on 'Doing signal separation...' #147

Open pocky12 opened 3 years ago

pocky12 commented 3 years ago

Hello,

First of all, thank you for developing and publishing this fantastic resource for neuropil subtraction.

I have been trying to implement FISSA to extract responses from cells identified through suite2p. I followed the uploaded Jupyter Notebook tutorials and have checked my suite2p outputs, but whenever I run FISSA, the program cannot move past the 'Doing signal separation....' output when I run 'experiment.separate' (seems to be in an infinite loop as I left my program running for 7+ hours on a relatively small data-set before having to force-kill the program). I checked to make sure I was running the most recent FISSA package (0.7.2) as I noticed a previous user had also posted a similar issue in Spyder and the issue appeared to resolve with the the 0.7 version. Launching the example Jupyter binder on this GitHub for using Suite2p + FISSA on example data also runs into the same issue, so I do not believe the issue to be specific to my data.

Any assistance you could provide would be much appreciated!

Thank you, pocky12

swkeemink commented 3 years ago

Hi, thanks for your interest in our toolbox, and letting us know about this bug!

In particular we will have to look at the binder implementation. Since it apparently stopped working even on the binder there might have been a package update in one our dependencies that has broken things.

Couple of questions:

pocky12 commented 3 years ago

Hello,

Thank you for the assistance!

It looks like the issue is isolated to interacting with Suite2P and SIMA. The standard example notebook as well as the cNMF example notebook does not run into the bug. I also run into the same issue when I try to run the Suite2P example notebook on my personal Jupyter Notebook.

scottclowe commented 3 years ago

Hi, @pocky12, Thank you for reporting the issue to us.

@swkeemink, I tried running the Suite2P notebook on Binder and it ran out of memory (exceeded 2GB), so I think it is probably due to a recent update to Suite2P which has changed its output. Although our unit tests currently report as failing on the main repo page, that's only on Python <3.5, where holoviews is broken. Python 3.6 and 3.7 are still passing unit tests.

pocky12 commented 3 years ago

Hello,

I apologize in advance if any of the information I provide is not what you are asking for, python is not my forte.

conda env list output

image

pip freeze output (a bit long, sorry):

appdirs==1.4.4 atpublic==1.0 attrs==19.3.0 backcall==0.1.0 bleach==3.1.4 certifi==2020.12.5 cffi==1.14.0 chardet==3.0.4 cloudpickle==1.4.1 colorama==0.4.3 conda==4.8.3 conda-pack==0.4.0 conda-package-handling==1.7.0 configobj==5.0.6 cryptography==2.9.2 cycler==0.10.0 Cython==0.29.17 cytoolz==0.10.1 dask==2.17.0 decorator==4.4.2 defusedxml==0.6.0 descartes==1.1.0 distro==1.5.0 dpath==2.0.1 dvc==0.94.0 entrypoints==0.3 fissa==0.7.2 flatten-json==0.1.7 flufl.lock==3.2 funcy==1.14 future==0.18.2 gitdb==4.0.5 GitPython==3.1.2 grandalf==0.6 h5py==2.10.0 humanize==2.4.0 idna==2.9 imageio==2.8.0 importlib-metadata==1.6.0 inflect==3.0.2 ipykernel==5.1.4 ipython==7.13.0 ipython-genutils==0.2.0 ipywidgets==7.5.1 jedi==0.17.0 Jinja2==2.11.2 joblib==0.15.1 json5==0.9.5 jsonpath-ng==1.5.1 jsonschema==3.2.0 jupyter==1.0.0 jupyter-client==6.1.3 jupyter-console==6.1.0 jupyter-core==4.6.3 jupyterlab==2.2.9 jupyterlab-server==1.2.0 kiwisolver==1.2.0 llvmlite==0.32.1 MarkupSafe==1.1.1 matplotlib==3.1.3 mistune==0.8.4 mizani==0.7.2 mkl-fft==1.0.15 mkl-random==1.1.1 mkl-service==2.3.0 nanotime==0.5.2 natsort==7.0.1 nbconvert==5.6.1 nbformat==5.0.6 networkx==2.4 notebook==6.0.3 numba==0.49.1 numpy==1.18.1 olefile==0.46 packaging==20.4 palettable==3.3.0 pandas==1.2.0 pandocfilters==1.4.2 parso==0.7.0 pathspec==0.8.0 patsy==0.5.1 pexpect==4.8.0 pickleshare==0.7.5 Pillow==7.1.2 plotnine==0.7.1 ply==3.11 prometheus-client==0.7.1 prompt-toolkit==3.0.4 ptyprocess==0.6.0 pyasn1==0.4.8 pycosat==0.6.3 pycparser==2.20 pydot==1.4.1 Pygments==2.6.1 pygtrie==2.3.2 pyOpenSSL==19.1.0 pyparsing==2.4.7 pyqtgraph==0.10.0 pyrsistent==0.16.0 PySocks==1.7.1 python-dateutil==2.8.1 pytz==2020.5 PyWavelets==1.1.1 PyYAML==5.3.1 pyzmq==18.1.1 qtconsole==4.7.4 QtPy==1.9.0 rastermap==0.1.3 read-roi==1.6.0 requests==2.23.0 ruamel-yaml==0.15.87 ruamel.yaml.clib==0.2.0 scanimage-tiff-reader==1.4 scikit-image==0.16.2 scikit-learn==0.22.1 scipy==1.4.1 Send2Trash==1.5.0 Shapely==1.7.1 shortuuid==1.0.1 sip==4.19.13 six==1.14.0 smmap==3.0.4 statsmodels==0.12.1 suite2p==0.7.5 terminado==0.8.3 testpath==0.4.4 texttable==1.6.2 tifffile==2020.5.25 toolz==0.10.0 tornado==6.0.4 tqdm==4.46.0 traitlets==4.3.3 treelib==1.6.1 urllib3==1.25.8 voluptuous==0.11.7 wcwidth==0.1.9 webencodings==0.5.1 widgetsnbextension==3.5.1 zc.lockfile==2.0 zipp==3.1.0 Note: you may need to restart the kernel to use updated packages.

pocky12 commented 3 years ago

Hello,

I just wanted to follow-up on this issue to see if there have been any updates! Thanks again for your assistance :)

pocky12

swkeemink commented 3 years ago

Hi @pocky12 ,

Sorry for the delay. I'm working on it, but no update yet!

swkeemink commented 3 years ago

@scottclowe

I tried running the Suite2P notebook on Binder and it ran out of memory (exceeded 2GB), so I think it is probably due to a recent update to Suite2P which has changed its output. Although our unit tests currently report as failing on the main repo page, that's only on Python <3.5, where holoviews is broken. Python 3.6 and 3.7 are still passing unit tests.

Since the SIMA notebook has the same problem I am not sure the problem is with Suite2P specifically. Although it might be two different issues, because the SIMA notebook binder does not run out of memory.

pocky12 commented 3 years ago

Hello,

Not a problem, I appreciate the update and your time. Any other updates you could provide as you move forward are always appreciated! Please let me know if there's anything I can do on my end to help out :)

-pocky12

swkeemink commented 3 years ago

I'm working on updating the general Suite2p implementation in #150 . There was also a possibly related error reported in #148 .

Sorry for the slowness on this!

swkeemink commented 3 years ago

So after a long time and finally having fixed #150 (fix still to be added to the main repo), I have now finally succeeded in reproducing this problem on my own system and not just on Binder. Will hopefully be able to zero in on the reason soon.

swkeemink commented 3 years ago

There seems to be a problem with multiprocessing in this specific use-case. Setting ncores_seperation=1 when declaring an experiment: exp = fissa.Experiment(images, rois, output_folder, ncores_separation=1)

Lets the notebook run as normal (but this will be a lot slower of course).

swkeemink commented 3 years ago

After starting a new ipython instance everything works fine now and can no longer reproduce the bug. Will first push #150 and see if that also fixes the binder runs.

swkeemink commented 3 years ago

So I CAN reproduce the error now. If I run a clean notebook and try to run the Suite2p analysis first, and then use multiprocessing in the fissa analysis, this then breaks. Perhaps with suite2p (at least the way we run it) the multiprocessing pool does not get properly closed?

swkeemink commented 3 years ago

It might have something to do with that they switched from multiprocessing to numba for parallelization: https://github.com/MouseLand/suite2p/commit/fc06986b88c5f0606c1885198f8959ce17b17ce1

swkeemink commented 3 years ago

So from this blog post: https://pythonspeed.com/articles/python-multiprocessing/ I now somewhat understand what's going on, and using the following for our multiprocessing Pool definition fixes the problem with stuck signal separation:

from multiprocessing import get_context
def your_func():
    with get_context("spawn").Pool() as pool:
        # ... everything else is unchanged

Unfortunately using the spawn-context makes an entirely new python instance basically, and our current way of loading custom datahandlers (importing a new datahandler library) breaks. My fix for #150 unfortunately relied on that. It was a very inelegant way of doing datahandlers anyway, and I will finally have to get on #16 seriously.