-
Notifications
You must be signed in to change notification settings - Fork 370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CNMFE notebook update #1207
CNMFE notebook update #1207
Conversation
Yikes I don't know how I let that |
04c3533
to
8a60ae7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks great! One thing - I suggest getting rid of the word "Hence" everywhere in the document. I left a few other notes.
I like the new graphics.
@@ -1153,22 +1153,37 @@ def nb_inspect_correlation_pnr(corr, pnr): | |||
|
|||
pnr: ndarray | |||
peak-to-noise image created with caiman.summary_images.correlation_pnr | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be better to type-annotate these in the prototype rather than give them types in the comment, and document the return function there too
caiman/utils/visualization.py
Outdated
@@ -1153,22 +1153,37 @@ def nb_inspect_correlation_pnr(corr, pnr): | |||
|
|||
pnr: ndarray | |||
peak-to-noise image created with caiman.summary_images.correlation_pnr | |||
|
|||
cmap: str | |||
colormap for plotting corr and pnr images |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to what the valid strings are? Or if there's a well-defined term like "a FooBarBaz colormap" use that? This is a little mysterious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add link to holoviews colormap which is what this is: https://holoviews.org/user_guide/Colormaps.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good solution
"\n", | ||
"Some tools for visualization of movies and results are also included. \n", | ||
"\n", | ||
"> This demo follows a similar pattern to the CNMF demo in `demo_pipeline.ipynb`. It includes less explanation except where there are important differences. If you want to get a more explanation-heavy picture of the fundamentals, we suggest starting with `demo_pipeline.ipynb`." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@@ -51,6 +64,7 @@ | |||
" pass\n", | |||
"import bokeh.plotting as bpl\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ordinarily I'd suggest putting these up with the other pre-caiman imports, but I'm guessing this needs to be here so it happens after the run_line_magic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I'm not sure why those are like that.
" level=logging.WARNING, style=\"{\") #logging level can be DEBUG, INFO, WARNING, ERROR, CRITICAL\n", | ||
"\n", | ||
"# set env variables \n", | ||
"os.environ[\"MKL_NUM_THREADS\"] = \"1\"\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do this in the environment entry scripts? Not sure we need to repeat that here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't hurt, esp for people who are in dev mode.
"1) Did we filter with a `gSig` value small enough so that we aren't blending different neurons together? To see what it is like when this happens, set `gsig_tmp` to `(6,6)` and inspect the above plots. \n", | ||
"2) More importantly, we want to find the threshold correlation and pnr values so that the *lower* threshold eliminates most of the noise and blood vessels from the plots, leaving behind as much of the neural elements as possible. For this data it will be around a correlation value lower bound somewhere between 0.8 and 0.9, and and pnr lower bound somewhere between 10 and 20 (as with CNMF, there is no perfect value: it is often an iterative search, but keep in mind it is better to have false positives later than false negatives).\n", | ||
"\n", | ||
"Use your judgment, and if you want to change the parameters you can do so here (here are some values that seem reasonable):" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Use your judgment" -> ""
"cell_type": "markdown", | ||
"metadata": {}, | ||
"source": [ | ||
"> Note there is a Qt-based corr-pnr viewer that some people prefer, `inspect_correlation_pnr()`. It provides a more intuitive interface than the notebook version above. It requires you to be in Qt mode (which you can enable using the cell magic `%matplotlib qt`." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot )
] | ||
}, | ||
{ | ||
"cell_type": "markdown", | ||
"metadata": {}, | ||
"source": [ | ||
"### Run the CNMF-E algorithm" | ||
"### Quilt plot for spatial parameters\n", | ||
"As discussed in `demo_pipeline.ipynb`, the other important paramters are those used for dividing the movie into patches for parallelization of source separation. The same processe is used for CNMFE. You want to be sure to pick `rf` and `stride` parameters so that many neurons fit in each patch, and at least one neuron fits in the overlap region between patches. You can visualize the patches using the `view_quilt()` function:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"You want to be sure to pick" -> "Pick"
] | ||
}, | ||
{ | ||
"cell_type": "markdown", | ||
"metadata": {}, | ||
"source": [ | ||
"### Alternate way to run the pipeline at once\n", | ||
"It is possible to run the combined steps of motion correction, memory mapping, and cnmf fitting in one step as shown below. The command is commented out since the analysis has already been performed. It is recommended that you familiriaze yourself with the various steps and the results of the various steps before using it." | ||
"These patches and overlaps are on the large side, but that is ok. The main concern would be making them too small, so let's just leave them be. The `demo_notebook.ipynb` goes through in some detail adjusting the spatial parameters, as we did above for the initialization params. The process would be the same here if you needed to change the patch parameters for your data.\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condense first two sentences
"source": [ | ||
"A couple of things to note about that movie:\n", | ||
"- The middle panel of neural activity includes *all* components (accepted and rejected), so you will see some of the blood vessel \"activity\" that was discovered and later rejected.\n", | ||
"- The residual includes some activity that looks neural in origin. You can try playing with different params to get them (which would you try?). The [Mesmerize](https://github.com/nel-lab/mesmerize-core) package is a great way to search parameter space and visualize results with more sophisticated visualization tools if you have a tricky data set." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"which would you try?" -- unclear what this is meant to add
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creativity juicer -- editing to make it a bit clearer.
d08761b
to
3315037
Compare
Just did a couple of final walkthroughs. This is ready to merge into dev, barring major problems. Will be useful to get user feedback at this point. |
Description
Updating the CNMFE demo notebook.
This builds on #1075 . Visualizations added, explanation of things that give people problems based on experience from issues/workshops (e.g., loading data after saving). Based on this, I next plan to go back into CNMF and make some changes to make them more symmetric (make CNMF a bit more concise in places).
It is fairly brief, suggesting readers go to the CNMF notebook for explanations. More explanations are offered where CNMFE differs, which is:
These are explained in more detail either in the main body or in sidebars (no appendices!). It also includes changes to
nb_correlation_pnr()
(issue #1206 ) that make it much more usable. It is almost as useful as the Matplotlib version now.