Skip to content
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

Merged
merged 13 commits into from
Jan 25, 2024
Merged

CNMFE notebook update #1207

merged 13 commits into from
Jan 25, 2024

Conversation

EricThomson
Copy link
Contributor

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:

  • Background model (ring model).
  • Initialization (corr/pnr).

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.

@EricThomson
Copy link
Contributor Author

Yikes I don't know how I let that 7 file in, or how it even came to exist. Will get rid of that.

Copy link
Member

@pgunn pgunn left a 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

Copy link
Member

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

@@ -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
Copy link
Member

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

Copy link
Contributor Author

@EricThomson EricThomson Oct 29, 2023

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

Copy link
Member

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`."
Copy link
Member

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",
Copy link
Member

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?

Copy link
Contributor Author

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",
Copy link
Member

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

Copy link
Contributor Author

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):"
Copy link
Member

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`."
Copy link
Member

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:"
Copy link
Member

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",
Copy link
Member

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."
Copy link
Member

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

Copy link
Contributor Author

@EricThomson EricThomson Oct 29, 2023

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.

@EricThomson EricThomson mentioned this pull request Jan 7, 2024
45 tasks
@EricThomson
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants