CaImAn icon indicating copy to clipboard operation
CaImAn copied to clipboard

Handle autoregressive parameter p consistently

Open EricThomson opened this issue 1 year ago • 0 comments

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.

EricThomson avatar Aug 15 '23 15:08 EricThomson