flatironinstitute / CaImAn

Computational toolbox for large scale Calcium Imaging Analysis, including movie handling, motion correction, source extraction, spike deconvolution and result visualization.
https://caiman.readthedocs.io
GNU General Public License v2.0
640 stars 370 forks source link

Handle autoregressive parameter p consistently #1153

Open EricThomson opened 1 year ago

EricThomson commented 1 year ago

I didn't realize until #1110 that p is defined twice in the parameters object, first as a preprocess value and second as a temporal value (it takes the same default in each case). This is not the intended use of CNMFParams. Every parameter is meant to have a unique name, otherwise param.change_params() becomes ill defined.

I'm not sure what the best resolution is: either remove one of the p params, or split them into two p parameters for the two parameter dictionaries (e.g., the preprocess one is used more in the online algorithms). I haven't looked deeply enough here to have a good feel for the differences, to have an opinion of the best road forward.

As an aside, we should probably better explain the flow of p in demo_pipeline.py. There, it says we are turning off deconvolution by setting p to 0, but that line of code is actually commented out.