Fixes infinite values in SNR

[pseudomanu]

Description

SNR_comp sometimes contains inf values, probably due to some "bad" components having a trace at 0 except for one event. One problem is that it breaks Mesmerize viz' SNR sliders.

Type of change

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

Has your PR been tested?

Yes.

The code just adds a line in estimates.py to check for inf values in SNR, in addition to NaN, and replace them by 0.
Another options discussed previously is to replace by the max non-inf value of those traces. We decided against it eventually, but here's the code, just in case anyone is interested:

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 max SNR value.')
    # find the maximum SNR value that is not inf
    max_SNR = np.max(SNR_comp[np.isfinite(SNR_comp)])
    print('max non-inf SNR value: {}'.format(max_SNR))
    # replace inf values with the maximum SNR value
    SNR_comp = np.where(np.isinf(SNR_comp), max_SNR, SNR_comp)

[pgunn]

Using f-strings might be a slightly nicer way to write this

[pgunn]

This seems okay to me; @kushalkolar do you have any thoughts?

[pgunn]

Actually, thinking about this, I wonder if it'd be better to do this with the default value of a flag to evaluate_components, or make this a parameter in CNMFParams, so people who want to be able to distinguish a real 0 from the case described above still could. But I'm not sure if that's something people actually might want to do. Hm.

[kushalkolar]

We talked about this on mescore a while ago and I think replacing inf with 0 is incorrect since those components have high SNR. A proper fix requires digging into how the noise is calculated and writing a test.

[pgunn]

@kushalkolar I will defer to your judgement on this

[kushalkolar]

The correct way to fix this is probably by fixing the function that estimates the noise floor and make sure it always returns meaningful values, or figure out a way to handle SNR calculation when the noise floor is at or near 0. I don't have time to dig into this, maybe @j-friedrich ?

Setting them to zero is wrong, see: kushalkolar/mesmerize-viz#38 (comment)

I had suggested people set them to the max SNR of all the components but that was just a monkeypatch until someone figures out the numerically correct way to fix this. It wouldn't be correct to have that in the core caiman library because if you are using caiman you would expect the SNR values you get are actually correct SNR values.

[pseudomanu]

I agree with Kushal that the way to fix this is to figure out why the SNR calculation produces those errand values.
Regarding whether setting to 0 or the max non-inf value is the correct approach, our experience is that those cells with infinite SNR are really poor components (@blogeman may have a different opinion). See example screenshot below.

They are typically very small, with one bright pixel at best, often under a vessel or on the FOV margin. The denoised F usually consists in a single positive value, and lots of zeros.

[kushalkolar]

I'm going to close this and I propose users do this to circumvent it:

1. Replace the inf elements with the max value from the array
2. for mesmerize, see this Failed to create view for 'FloatSliderView' in eval params tab kushalkolar/mesmerize-viz#38 (comment)

[pgunn]

@kushalkolar What do you think about the idea of doing your #1 automatically and spitting out a warning?

[kushalkolar]

@kushalkolar What do you think about the idea of doing your #1 automatically and spitting out a warning?

I'll leave it up to you, it's technically wrong but it's a workaround, and I don't know how often people pay attention to warnings, especially given that sklearn always spits out warnings 🙃