nel-lab / mesmerize-core

High level pandas-based API for batch analysis of Calcium Imaging data using CaImAn
Other
61 stars 15 forks source link

Error while computing reconstructed background or residuals #195

Closed X4ndri closed 9 months ago

X4ndri commented 1 year ago

Running CNMF-E algo with mesmerize 0.2.0 I get the following traceback when trying to get the reconstructed background or the residuals. can compute the reconstructed movie though.


AttributeError                            Traceback (most recent call last)
Cell In[127], line 2
      1 rcm = attempt.cnmf.get_rcm()
----> 2 rcb = attempt.cnmf.get_rcb()
      3 # res = attempt.cnmf.get_residuals()

File ~/miniconda3/envs/mescore/lib/python3.10/site-packages/mesmerize_core/caiman_extensions/_utils.py:25, in validate.<locals>.dec.<locals>.wrapper(self, *args, **kwargs)
     23     tb = self._series["outputs"]["traceback"]
     24     raise BatchItemUnsuccessfulError(f"Batch item was unsuccessful, traceback from subprocess:\n{tb}")
---> 25 return func(self, *args, **kwargs)

File ~/miniconda3/envs/mescore/lib/python3.10/site-packages/mesmerize_core/caiman_extensions/cache.py:165, in Cache.use_cache.<locals>._use_cache(instance, *args, **kwargs)
    157         self.cache.drop(
    158             index=self.cache.sort_values(
    159                 by=["time_stamp"], ascending=False
   (...)
    162             inplace=True,
    163         )
    164         self.cache = self.cache.reset_index(drop=True)
--> 165     return_val = func(instance, *args, **kwargs)
    166     self.cache.loc[len(self.cache.index)] = [
    167         instance._series["uuid"],
    168         func.__name__,
   (...)
    172         time.time(),
    173     ]
    174 # no matter the storage type if size is not going to be exceeded for either, then item can just be added to cache
    175 else:

File ~/miniconda3/envs/mescore/lib/python3.10/site-packages/mesmerize_core/caiman_extensions/cnmf.py:558, in CNMFExtensions.get_rcb(self)
    555 spatial = cnmf_obj.estimates.b
    556 temporal = cnmf_obj.estimates.f
--> 558 return LazyArrayRCB(spatial=spatial, temporal=temporal, frame_dims=dims)

File ~/miniconda3/envs/mescore/lib/python3.10/site-packages/mesmerize_core/arrays/_cnmf.py:34, in LazyArrayRCM.__init__(self, spatial, temporal, frame_dims)
     14 def __init__(
     15         self,
     16         spatial: np.ndarray,
     17         temporal: np.ndarray,
     18         frame_dims: Tuple[int, int],
     19 ):
     20     """
     21     Parameters
     22     ----------
   (...)
     31         
     32     """
---> 34     if spatial.shape[1] != temporal.shape[0]:
     35         raise ValueError(
     36             f"Number of temporal components provided: `{temporal.shape[0]}` "
     37             f"does not equal number of spatial components provided: `{spatial.shape[1]}`"
     38         )
     40     self._spatial = spatial

AttributeError: 'NoneType' object has no attribute 'shape'```
kushalkolar commented 1 year ago

I don't think the low-rank bankground b is stored in the estimates object from the way CNMFE is currently written, @EricThomson is more familiar with CNMFE and can give more details.

EricThomson commented 1 year ago

Note I haven't looked at mesmerize in much detail I am really only familiar with caiman, so I can talk about that a bit (it looks like mesmerize is invoking caiman's estimates.b so I can talk about that).

In general, the behavior of the background models in caiman should be better documented. I don't fully understand the behavior yet and I will be going into this labyrinth shortly. Some of the parts I do understand: when you run CNMFE using standard parameters from the demo, the background model is broken into two components, a "constant" component Bc, and "fluctuating" component, Bf (the ring model).

Bf is modeled for every pixel in the image (and is captured in W in estimates). Bc is just a constant baseline value for each pixel that is captured in b0 (the comment in the demo says it is returned as b, which is an error, because b is reserved for the fluctuating low-rank model in CNMF, which isn't used if nb -- which is set by gnb in the notebook, is non-positive). I also was wrong about this recently in gitter I think I said the constant part was simply not used because b was none, but actually it is used, and is in b0.

Things get more complicated if you set nb greater than 0, but this is typically not done in CNMFE because background is absorbed pretty well by the ring model. But if you do, then you will have a estimates.b not none, and I believe your background model will now have both b0 and b (but I've never run this so am not sure, so this is where I don't fully understand the behavior).

In terms of getting residuals, this should be done as part of the refactor in caiman: to my knowledge this is done in different places on the fly and it should just be a separate function/method. For instance, in estimates.play_movie() it is generated on line 659 in the standard way (residual = imgs - AC - B ): https://github.com/flatironinstitute/CaImAn/blob/80e1681bbce8fb4a36b04c57d9e42ffb75a32d58/caiman/source_extraction/cnmf/estimates.py#L659

I think basically this topic deserves its own notebook because if we address it thoroughly within the standard CNMFE notebook people will be like "Why are you going down this rabbit hole of background model details" but for people that want to know more it will be useful. I'll be going down this rabbit hole more thoroughly the next few weeks and probably make a special notebook for it.

kushalkolar commented 1 year ago

Yes Eric is right that "get_rcb()" in mesmerize-core is just doing "b * f", as shown in the docstring: https://mesmerize-core.readthedocs.io/en/latest/api/cnmf.html#mesmerize_core.CNMFExtensions.get_rcb

the only difference from caiman (or really just numpy) is that mesmerize-core returns a LazyArray instead of a standard numpy "everything in RAM array".

kushalkolar commented 1 year ago

And the residuals depends on RCB, Y - (A C) - (b f)

EricThomson commented 1 year ago

Right: it needs to be calculated using b0 instead like in the play_movie method. We need a residual getter in estimates. I will put this on my list.

kushalkolar commented 1 year ago

Ah so for cnmfe it's b0 instead of b? Is there a reason for this api inconsistency? Confusing at first at least. If you can make more sense of it it's an easy fix to use b0 instead of b for the RCB getter in mesmerize-core.

EricThomson commented 1 year ago

Yes it could probably be more clean. My guess is the thought was: the calculations are very different in the back end, they have different meanings and notations in the papers so let's follow that.

And then they were simply used as toggles (e.g., the existence of b/f is a check for CNMF, while the presence of b0/W is a check for CNMFE: https://github.com/flatironinstitute/CaImAn/blob/80e1681bbce8fb4a36b04c57d9e42ffb75a32d58/caiman/source_extraction/cnmf/estimates.py#L647).

Unfortunately, this is not explicitly stated because the division between CNMF/CNMFE only implicitly runs through most of the code -- it's basically a bunch of easter eggs :) ).

kushalkolar commented 1 year ago

And then they were simply used as toggles (e.g., the existence of b/f is a check for CNMF, while the presence of b0/W is a check for CNMFE

Something to fix in v2. Anyways for now, b0 corresponds to b and W corresponds to f? Does it make sense to just drop in b0 and W for getting reconstructed background and residuals?

EricThomson commented 1 year ago

No it is more complicated than that because W has the ring model which is separate from the constant term b0 for each pixel, and an independent factor. Here is calculation of background B for CNMFE (corresponds to equation 5 and more importantly 8 of CNMFE paper -- I'd just focus on the ssub_B == 1 case so you can focus on the ideas more easily):

https://github.com/flatironinstitute/CaImAn/blob/80e1681bbce8fb4a36b04c57d9e42ffb75a32d58/caiman/source_extraction/cnmf/estimates.py#L634

Compare that B calculation to the one for CNMF on line 647.

Then the residual is given on line 659: Y_res = imgs - AC - B (which is rearranging eqn 1 of CNMFE paper)

kushalkolar commented 1 year ago

I'm closing this because it's more of a caiman CNMFE algorithm related question than a mesmerize discussion.

kushalkolar commented 1 year ago

Discussed with Eric: we can fix this at next hackathon or something...

kushalkolar commented 9 months ago

not happening in mescore, Amol is working on new stuff which has this