kushalkolar / mesmerize-viz

Widgets for calcium imaging analysis visualization in notebooks
GNU General Public License v3.0
10 stars 5 forks source link

Failed to create view for 'FloatSliderView' in eval params tab #38

Closed pseudomanu closed 10 months ago

pseudomanu commented 11 months ago

Hi, When creating a Mesmerize-viz visualization (using viz_cnmf = df.cnmf.viz() then viz_cnmf.show()), the sliders for min_SNR and SNR_lowest fail to load.

image

Any idea what could be wrong here?

Thanks

kushalkolar commented 11 months ago

Did you run component eval?

If you did, my guess is the SNR values are weird. If you get the cnmf_obj for that item what does cnmf_obj.estimates.snr_comps look like?

pseudomanu commented 11 months ago

We had not run component eval.
Now we run it like this: df.iloc[1].cnmf.run_eval({'SNR_lowest': 0.5, 'min_SNR': 2}). However the error is still there when re-plotting the widget. We also tried to get the SNR values, after running the evaluation, with

cnmf_obj = df.iloc[1].cnmf.get_output()
print(cnmf_obj.estimates.snr_comps )

But we get this error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[56], line 2
      1 cnmf_obj = df.iloc[1].cnmf.get_output()
----> 2 print(cnmf_obj.estimates.snr_comps )

AttributeError: 'Estimates' object has no attribute 'snr_comps'

What should we do? Thanks

kushalkolar commented 11 months ago

Sorry it might be snr_comp or SNR_comp, you can use tab completion to find it. Caiman unfortunately doesn't really document it.

vncntprvst commented 11 months ago

It is SNR_comp. For reference, it is defined in CaImAn/caiman/source_extraction/cnmf/estimates.py, in the Estimates class.

            SNR_comp: np.ndarray
                trace SNR for each component
pseudomanu commented 11 months ago
cnmf_obj = df.iloc[1].cnmf.get_output()
print(cnmf_obj.estimates.SNR_comp)

returns

[18.45274505  9.54067621         inf ...  2.09858359  2.1697136
  8.04065384]

Could the inf value(s) be the issue?

kushalkolar commented 11 months ago

Yes! I don't understand how there can be an infinite value for SNR, if you replace the infs with 0 it should work.

@EricThomson any idea why SNR values could be infinity? :joy:

EricThomson commented 11 months ago

Ugh. I'm looking over this old issue but it isn't helping: https://github.com/flatironinstitute/CaImAn/issues/503

I will have to think about it, I thought this was fixed because I saw that issue and it was closed đŸ˜Ŧ

I would be curious to see the value of estimates before you ran evaluate_components(), is there a zero in a denominator somewhere or something funky like that? This is something we should fix.

vncntprvst commented 11 months ago

I'd be interested to see those initial estimates values as well, but I don't know how to go about it. If I place breakpoints (using VS Code) in the caiman or mesmerize code, their get ignored when running in debug mode. e.g., in the simple code below, breakpoints in get_output() are bypassed.

            from mesmerize_core import load_batch
            df = load_batch("/path/to/batch_dataframe_file.pickle")
            cnmf_obj = df.iloc[1].cnmf.get_output()

I assume this is because the function are called from an object, but I don't know. How do you do that?

kushalkolar commented 11 months ago

Component evaluation is always done after CNMF, sometimes run_eval() is useful if there were weird values, but AFAIK some component evaluation is always done.

Someone would have to dig into the SNR calculation to figure out where the inf is coming from, maybe the noise floor is computed to be zero for some reason for that component? What does the problematic component with snr = inf look like?

EricThomson commented 11 months ago

I have definitely seen these (years ago) and they typically look like perfectly reasonable high-signal units. I had incorrectly assumed they were fixed though ☚ī¸ My guess is this will be a relatively easy fix. I'm not sure I have a data set that has them though so we need to get one or make one :smile:

kushalkolar commented 11 months ago

Simulated data with no noise?

If these components are reasonable, can we give them a meaningful number for SNR? If it is inf then it's not really usable for comparison with other components. Or we could do a thing where components with inf SNR get a special color, the rest use the regular cmap?

EricThomson commented 11 months ago

Vincent could you share your hdf5 file from this dataset (that results when you run cnmf.estimates.save())? I can take a look pretty soon I'd like to track down this problem.

kushalkolar commented 11 months ago

Vincent could you share your hdf5 file from this dataset (that results when you run cnmf.estimates.save())? I can take a look pretty soon I'd like to track down this problem.

No, just give us the file at df.iloc[index].cnmf.get_output_path()

EricThomson commented 11 months ago

Whatever works in whatever framework :smile:

kushalkolar commented 11 months ago

We could add a df.iloc[index].cnmf.send_to_devs() :joy:

kushalkolar commented 11 months ago

@rob-the-bot I just remembered that at SFN you were talking about how you're aware of all the bugs caiman has had for years! I'm wondering if you encountered this bizarre situation where snr = inf :smile:

rob-the-bot commented 11 months ago

I don't recall ever dealing with snr=inf in my work. I spent some time on the GetSn function, because it seemed to give lower noise level estimates for data with slower imaging rate. However it's interesting that it produces near 0 value for the snr to go to infinity.

Edits: oops, turns out GetSn function isn't used actually in the components evaluation part of the code base.

Interested in having a look once you guys get that hdf5 file.

vncntprvst commented 11 months ago

Hi everyone, You can find the hdf5 file at this link. Let me know if you need anything else! Thanks

EricThomson commented 11 months ago

Thanks Vincent! I will hopefully get to look later this week. If someone else does first, please let us know!

rob-the-bot commented 11 months ago

I'm not sure why inf shows up, the calculations of SNR in caiman requires the imaging file Y. These components certainly don't look that bad, indices below

image

From a software point of view, it might be useful to add inf check along with the existing nan check around line 1060 in [estimates.py in the caiman package].(https://github.com/flatironinstitute/CaImAn/blob/9b0b79ca61f20ce93259b9833e1fe18e26d4e086/caiman/source_extraction/cnmf/estimates.py#L1060)

pseudomanu commented 11 months ago

The fix above would be great. For the moment I can manually change the code. Alternatively, is there a way to update those inf values from the notebook? @kushalkolar, you mentioned that we could replace the infs with 0. This is what I tried (same data, but different mcorr and cnmf, so it only has one inf value):

cnmf_obj = df.iloc[index].cnmf.get_output()
# Get inf values
SNR_vals = cnmf_obj.estimates.SNR_comp
inf_val_idx = np.where(np.isinf(SNR_vals))
# Print indices and values
print(inf_val_idx)
print(SNR_vals[inf_val_idx])
# Change inf values to 0
SNR_vals[np.isinf(SNR_vals)] = 0
print(SNR_vals[inf_val_idx]) 

This returns:

(array([583], dtype=int64),)
[inf]
[0.]

This doesn't solve the issue, however, as the visualization is created with viz_simple = df.cnmf.viz(.... Is it possible to update the SNR values in df.cnmf ? Thanks

pseudomanu commented 11 months ago

I fixed the inf issue, adding this code after the NaN values check in estimates.py, as per @rob-the-bot recommendation:

        if np.any(np.isinf(SNR_comp)):
            logging.warning('inf values detected for trace SNR in {}'.format(np.where(np.isinf(SNR_comp))[0]) +
                            '. Changing their value to 0.')
            SNR_comp = np.where(np.isinf(SNR_comp), 0, SNR_comp)

After re-running the cnmf computation, there are no inf values in SNR_comp:

SNR_vals = cnmf_obj.estimates.SNR_comp
inf_val_idx = np.where(np.isinf(SNR_vals))
# Print indices and values
print(inf_val_idx)
print(SNR_vals[inf_val_idx])
(array([], dtype=int64),)
[]

Unfortunately, that did not solve the broken slider issue :cry: Same error as before:

image

kushalkolar commented 11 months ago

The proper way to fix this is within caiman, but until then here's a very hacky workaround.

When you manipulate the CNMF object it only changes the object in RAM. When you use cnmf.viz() it reloads the cached object or the object from disk (the cached object and object on disk are the same). So you will need to overwrite the existing CNMF obj and invalidate the cache, cnmf.viz() will then load the new object on disk.

Replace inf values with the max values from the array and save the cnmf obj.

# import cache
from mesmerize_core.caiman_extensions.cnmf import cnmf_cache

# a is cnmf_obj.estimates.SNR_comp
a[np.isinf(a)] = np.nanmax(a[~np.isinf(a)])

# get path of cnmf obj on disk so we can overwrite
path = df.iloc[index].cnmf.get_output_path()

# save cnmf obj to disk
cnmf_obj.save(path)

# clear cache
cnmf_cache.clear_cache()

# viz should work now
vncntprvst commented 11 months ago

Thank you Kushal! We'll give it a try. As you say, this is more a caiman issue, but I'll keep this thread here, if that's ok. Let me know if that makes more sense to open a new issue in CaImAn's repo. Regarding the issue of SNR having inf values, the comment by @rob-the-bot that SNR calculations in caiman require the imaging file Y makes me wonder if the issue isn't with the input movie file. As an aside, creating the input movie has been a perennial pain in the neck for us. The source data is a collection of tif files from a Bruker 2P microscope. The default method to load these data in mesmerize/caiman has been to first save them into a multi-page tiff, with something like this:

    fls = [glob.glob(os.path.join(sourcedir1,'*Ch2*.ome.tif')),
       glob.glob(os.path.join(sourcedir2,'*Ch2*.ome.tif')),
       glob.glob(os.path.join(sourcedir3,'*Ch2*.ome.tif'))]
    fls.sort()
    m = cm.load_movie_chain(fls)
    m.save(os.path.join(savedir,'data.tif'))

However, the size of the output file is not manageable. Source files are uint12 data, loaded as uint16. Caiman saves tif files as 32-bit. So, alternatively, we save files as hdf5, such as:

    m = cm.load_movie_chain(fls,outtype=np.ushort)
    m.save(os.path.join(savedir,'data.h5'),to32=False)

That works, in term of file size, but the downside is that, contrary to multi-page tiff files, we can't view the hdf5 movie in ImageJ, which is an important tool for us. So, instead, I wrote a script (link) to concatenate the Bruker ome.tif files into a BigTIFF, as uint16 data. This works with caiman and ImageJ, and the file size is manageable. Example file here.

Back to our issue: motion correction and cnmf with mesmerize/caiman seem to work well, but I'm a bit concerned by the data values in the mcorr movie.

original_movie = df.iloc[0].caiman.get_input_movie()
original_movie data type: uint16
original_movie min val: 16
original_movie max val: 65520
mcorr_movie = df.iloc[0].mcorr.get_output()
mcorr_movie data type: float32
mcorr_movie min val: -9863.3740234375
mcorr_movie max val: 84527.7421875

I understand why data is converted to float, but why do we get negative values here? How can it be properly converted back to uint16 range? If we normalize the data to the original movie's range [16 - 65520], we loose dynamic range, i.e., the darker values are more gray than the original.

vncntprvst commented 10 months ago

The motion correction's negative values issue is referenced (and solved) in the caiman issue above.

vncntprvst commented 10 months ago

Note that even after clipping values to uint16 ranges, we still get inf values in the SNR. And the mesmerize-viz sliders for SNR parameters still break. Any suggestion? Thanks

vncntprvst commented 10 months ago

So, in fact, it works :satisfied:. We now have working SNR sliders. To recap, in our case, it appears that we needed to perform these two steps:

Performing only either one was not sufficient. Thanks for your help and feedback everyone!

EricThomson commented 10 months ago

This is really helpful sleuthing! Will definitely have to revisit this in caiman after winter break and do a deep dive.

pseudomanu commented 10 months ago

Just to confirm image Thanks!

kushalkolar commented 10 months ago

So, in fact, it works 😆. We now have working SNR sliders. To recap, in our case, it appears that we needed to perform these two steps:

Performing only either one was not sufficient. Thanks for your help and feedback everyone!

From what you linked here, are you replacing the infs with zeros? I would recommend replacing them with the max SNR value in the array, not zero, assuming that these are actually components with high SNR and they're not completely "weird" because otherwise you would be excluding potentially good components.

vncntprvst commented 10 months ago

Hi @kushalkolar. Thanks for the comment. That makes sense. I updated our code to replace inf with max SNR values. In any case, I think we're better off ignoring this handful of weird units. The fix is just to allow the sliders to behave properly.

EricThomson commented 9 months ago

@vncntprvst I'm trying to reproduce the infinite SNR vals using that data you posted. If you find the time, could you possibly post the parameters you used for motion correction and CNMF (or a notebook that reproduces in caiman)? I just ran the dataset through CNMF and didn't get any inf, and while I used to see it a lot I haven't lately. I'd like to recreate so I can work on this problem. BTW, nice data!!

kushalkolar commented 9 months ago

I wonder how much variation there is between runs and between architectures with the SNR vals, or the noise floor calculation. In the mesmerize-core tests @clewis7 had to put tolerances up to 2-3 decimal places for some of the CNMF model outputs.

vncntprvst commented 9 months ago

@EricThomson Thanks for doing this! I'll send you the parameters tomorrow.

vncntprvst commented 9 months ago

I went back to the branch where I was debugging this. I used separate notebooks for motion correction and cnmf, and they are a bit of a mess, so it might be easier just to try the parameters. This is what I have in those notebook:

params_mcorr = \
{
  'main':
    {
        'strides': [18,18],
        'overlaps': [24, 24],
        'max_shifts': [12, 12],
        'max_deviation_rigid': 6,
        'border_nan': 'copy',
        'pw_rigid': True,
        'gSig_filt': None
    },
}
params_cnmf =\
{
    'main': 
        {
            'fr': 20, 
            'p': 1,
            'nb': 1,
            'merge_thr': 0.80,
            'rf': 20,
            'stride': 10, 
            'K': 8,
            'gSig': [5, 5],
            'ssub': 1,
            'tsub': 1,
            'method_init': 'greedy_roi',
            'min_SNR': 2.0,
            'rval_thr': 0.7,
            'use_cnn': True,
            'min_cnn_thr': 0.8,
            'cnn_lowest': 0.1,
            'decay_time': 0.4,
        },
    'refit': True, 
}

Let me know if you need anything else. Thanks!

EricThomson commented 9 months ago

Thanks a lot Vincent I'll give these a shot it might be next week as soon as I can. Will let you know once I've had a chance.

EricThomson commented 8 months ago

I wasn't able to reproduce in caiman. I very well may have missed a key parameter or something.

Link to notebook to try to reproduce. If anyone has success please let me know and I'll dig in.

vncntprvst commented 8 months ago

Interesting. Thanks for trying. Which version of caiman did you use? It's possible that the recent update fixes this issue?

EricThomson commented 8 months ago

I was in the most recent version (1.10.2). Maybe 🤷‍♂ī¸ But I'm not sure what would have done it 😆

blogeman commented 8 months ago

I've had the same issue discussed here; namely the same error with min_SNR and SNR_lowest sliders not working. Investigating .estimates.SNR_comp revealed Inf values that were corrected by using the solution proposed by @rob-the-bot. Visual inspection of these components showed that they are among the most robust responders so as @kushalkolar suggested I made sure to not set them to zero. This approach worked for me with out having to clip values after motion correction.