Skip to content
New issue

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

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

Already on GitHub? # to your account

Save memory by returning a sparse array from extract_binary_masks_from_structural_channel #1395

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

ethanbb
Copy link
Contributor

@ethanbb ethanbb commented Aug 25, 2024

Description

The docstring for extract_binary_masks_from_structural_channel says that it returns a sparse column format matrix, but it actually just returns an ndarray. I was running out of memory when trying to use this to generate a mask to input to CNMF. This changes the function to return a scipy.sparse.csc_array instead.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Has your PR been tested?

caimanmanager test and caimanmanager demotest pass. I don't think it's a good idea to add a test for memory usage, but here is what I see for my use case:

Before:

>>> tracemalloc.start()
>>> Ain, _ = rois.extract_binary_masks_from_structural_channel(mean_img, gSig=9)
>>> curr_mem, peak_mem = tracemalloc.get_traced_memory()
>>> curr_mem
1822996704

After:

>>> tracemalloc.start()
>>> Ain, _ = rois.extract_binary_masks_from_structural_channel(mean_img, gSig=9)
>>> curr_mem, peak_mem = tracemalloc.get_traced_memory()
>>> curr_mem
971327

@pgunn
Copy link
Member

pgunn commented Aug 26, 2024

@kushalkolar This looks good to me; is there any chance this might break anything with Mesmerize?

@ethanbb
Copy link
Contributor Author

ethanbb commented Aug 26, 2024

Hold off, it seems to break something (also getting this error in demo_seeded_CNMF.ipynb):

Traceback (most recent call last):
  File "/u/ethan/mesmerize-core/mesmerize_core/algorithms/cnmf.py", line 96, in run_algo
    cnm = cnm.fit(images)
          ^^^^^^^^^^^^^^^
  File "/u/ethan/CaImAn/caiman/source_extraction/cnmf/cnmf.py", line 523, in fit
    self.update_spatial(Yr, use_init=True)
  File "/u/ethan/CaImAn/caiman/source_extraction/cnmf/cnmf.py", line 915, in update_spatial
    update_spatial_components(Y, C=self.estimates.C, f=self.estimates.f, A_in=self.estimates.A,
  File "/u/ethan/CaImAn/caiman/source_extraction/cnmf/spatial.py", line 173, in update_spatial_components
    ind2_, nr, C, f, b_, A_in = computing_indicator(
                                ^^^^^^^^^^^^^^^^^^^^
  File "/u/ethan/CaImAn/caiman/source_extraction/cnmf/spatial.py", line 1066, in computing_indicator
    ind2_ = [np.hstack((np.where(iid_)[0], nr + np.arange(f.shape[0])))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/u/ethan/CaImAn/caiman/source_extraction/cnmf/spatial.py", line 1067, in <listcomp>
    if np.size(np.where(iid_)[0]) > 0 else [] for iid_ in dist_indicator]
               ^^^^^^^^^^^^^^
  File "/u/ethan/.conda/envs/mescore/lib/python3.11/site-packages/scipy/sparse/_base.py", line 396, in __bool__
    raise ValueError("The truth value of an array with more than one "
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all().

@ethanbb
Copy link
Contributor Author

ethanbb commented Aug 26, 2024

OK, this should fix it, but probably would be a good idea for someone else to check that notebook demo.

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

Successfully merging this pull request may close these issues.

2 participants