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
639 stars 370 forks source link

Suggestion to incorporate fastplotlib plots in the demos for interactive visualizations #1287

Closed JohnStout closed 6 months ago

JohnStout commented 9 months ago

Wanted to pass a suggestion to incorporate fastplotlib (@kushalkolar) plots into the demos. While they take more code to plot a movie than the .play method, I think they will improve user experience, quality control of parameter setting, and visualization of CNMFE results (see my edit on the demo_pipeline_CNMFE notebook: https://github.com/JohnStout/calcium_imaging/blob/main/code/miniscope_cnmfe/demo_pipeline_cnmfE_fpl.ipynb; please note that this notebook is a bit messy). From personal experience, the fastplotlib plots helped me to identify components that I couldn't easily see in the raw film, so I'm loving it!

I know that fastplotlib plots are built into the mesmerize experience (https://github.com/nel-lab/mesmerize-core; https://github.com/kushalkolar/mesmerize-viz), but I just like the layout of the caiman notebook parameter space and so I've merged the fastplotlib plots into the caiman demos.

In that notebook, I replaced .play methods with fastplotlib plots (which improve speed and provide the user with options to adjust gain/brightness), added fastplotlib plots to visualize the results of motion correction, included fastplotlib plots to explore gaussian blurring and setting correlation/peak-to-noise ratio values (these fastplotlib plots really helped provide intuition on how gSig, min_corr and min_pnr were defined; code modified from the caiman event curtesy of fastplotlib team) and added fastplotlib plots to explore the results of CNMFE and aid in the curation of components (including rejection, merging, etc...).

Even if these suggestions don't get implemented, I hope they help a fellow caiman user along the way and so I wanted to post this here.

I downloaded caiman as such:

  conda install -n base -c conda-forge mamba   # install mamba in base environment
  mamba create -n caiman -c conda-forge caiman # install caiman
  conda activate caiman  # activate virtual environment
  caimanmanager install
  pip install "fastplotlib[notebook]"

My environment is attached as a .txt (note that I probably have things added to env that aren't needed). caiman_environment.txt

EricThomson commented 9 months ago

Thanks John. Yes the fpl based visualizations are much better than anything else available. See #1118 which discusses the same. There, I wrote:

this stuff is too good to not put in user's hands. It's like being in a clinical trial where one drug is clearly better than placebo and the trial needs to be stopped because it is unethical.

I think at this point fastpotlib is probably mature and stable enough that it can and should be incorporated. It's a matter of how and which notebooks etc.

kushalkolar commented 9 months ago

I have comments on the notebook, I'll put them later. There are some changes in fastplotlib@main which break minor things (controller kwargs) from the current release.

JohnStout commented 8 months ago

Hey Kushal and Eric, I wanted to follow up on this and share code that I created to do some relevant plotting functions that incorporate fastplotlib into caiman. Trying to make caiman+fastplotlib visualizations easier (consolidated) for my current group until some master version is released.

I just made a wrapper to clean up the code space (plot_tools.py). This notebook supports overlaying ROI with the raw movie, overlaying ROI with a gaussian filtered movie, and overlaying ROI with the spatial footprints. I have been using these combined with your nb_view_components function to identify components to keep/remove.

Here is the testing notebook that requires an hdf5 file output from cnmf. The demo_pipeline_endoscope outputs should work: (https://github.com/JohnStout/caiman_fastplotlib_notebooks/blob/main/tests/tester.ipynb)

Here is the source code: https://github.com/JohnStout/caiman_fastplotlib_notebooks/blob/main/code/utils/plot_tools.py

It is about as simple as feeding your images and cnmf_object into the play_cnmf_movie object in the plot_tools.py file, then calling one of the few attributes associated with the play_cnmf_movie object. The backend is just fastplotlib code!

cnmf_plotter = plot_tools.play_cnmf_movie(images=images,cnmf_object=cnmfe_model) # instantiate
mov = cnmf_plotter.play_movie_draw_roi(components_type='accepted') # draw components and play movie
mov.show(sidecar=True) # show movie as sidecar

I haven't run every feature that I wrote into it, but the tester notebook should work.

Environment details: https://github.com/JohnStout/caiman_fastplotlib_notebooks/blob/main/environment.yml

EricThomson commented 8 months ago

I’m traveling now but we should definitely talk!

JohnStout commented 8 months ago

Sounds great, safe travels!

kushalkolar commented 8 months ago

I think Eric is interested in this longterm, some notes:

  1. The fastplotlib and pygfx API are in flux, pygfx plans to be stable pretty soon but it's going to take much longer for fastpotlib. Nonetheless we are very good at documenting most breaking changes in our release notes.

  2. You don't need to use scatters, there are line collections, many examples with circles here: https://github.com/fastplotlib/fastplotlib/tree/main/examples/desktop/line_collection

  3. Instead of this, you can instead just plot all components and just toggle the visibility of the lines at specific indices.

  4. I don't think this is required anymore in the latest release or the latest fastplotlib@main, but I might be wrong. Nonetheless we don't want this to be required because it's messy

  5. Not entirely sure of this since apply_funcs can also just be toggled :thinking: .

  6. I ultimately think you can make one simpler widget and use tabs for different views, makes things simpler and reduces clutter if I understand what you're trying to do.

  7. If you want to make CI to automate your testing you can model it off of our CI pipeline, all our example nbs are also tests, such as imagewidget and using these utils, we auto generate a lot of test screenshots images to make sure stuff is displayed correctly. Not sure if this was your intent, but your main bottleneck with getting any form of real tests and CI with this is caiman itself.

  8. I think I saw earlier you had instructions for manually toggling components between accepted/rejected, it's very easy to add keyboard events, see this and this . In general thes mesviz codebase is probably good for you to look at

minor stuff:

  1. I would use camelcase for class names (play_cnmf_movie, it's the convention that people are used to)
  2. Not sure why __drawroi__ is a dunder method :thinking:
JohnStout commented 8 months ago

Hey Kushal,

This is all extremely helpful information, thank you! I will get back to all of these comments in a couple of days!

pgunn commented 8 months ago

@JohnStout Before you spend too much time on this, there are some issues that create difficulties for including it in notebooks in the main codebase:

1) Right now FPL has mostly a single developer working on it, alongside other things; it'd be easier if there were a core team with a few developers. Some of the other libraries also have few developers, but they're areas where we could rip them out or replace them if need be, but FPL in the notebooks would be a big commitment and may be hard to reverse if we needed to 2) It also currently doesn't have the packaging we need; it'd need to be on conda-forge with stable/workable/tested builds on Windows/OSX/Linux 3) (I think we're at peace with it requiring a GPU to work well in some environments with some data, although this came up in past discussions) 4) If the API isn't that stable yet, we might want to hold off until it is stable to avoid signing up to follow API changes

Many of these may be addressed in time to come, but we definitely can't just do it quickly in the short term.

kushalkolar commented 8 months ago
  1. Right now FPL has mostly a single developer working on it, alongside other things; it'd be easier if there were a core team with a few developers.

This is wrong, I'm not getting into details here. For what it's worth we have more bandwidth than caiman so this point is moot at best.

but FPL in the notebooks would be a big commitment and may be hard to reverse if we needed to

Nobody is saying that we replace all notebooks to rely on fpl.

  1. It also currently doesn't have the packaging we need; it'd need to be on conda-forge with stable/workable/tested builds on Windows/OSX/Linux

This isn't that important. We have CI and from workshop experience almost nobody has isses running or installing fpl across various OSes. WGPU and pygfx which we rely on has very strong industry backing. It works. People are trying to do science, let's not get bogged down in a local minima on packaging.

  1. (I think we're at peace with it requiring a GPU to work well in some environments with some data, although this came up in past discussions)

ok

  1. If the API isn't that stable yet, we might want to hold off until it is stable to avoid signing up to follow API changes

I am right here and I have offered to help with this... Every API evolves over time, newer ones are faster. Sometimes there is no alternative than a new library.

EricThomson commented 8 months ago

Currently at airport, but I second what Kushal said. Fastplotlib has multiple devs and was built as a tool for Caiman in Andrea Giovanucci’s lab. It would just be weird to not get the ball rolling. We use it in workshops. Pip/conda stuff should not be an impediment. We need to just break through on this and it would be great to have your help John. What you are doing looks fantastic!

pgunn commented 8 months ago

It at least needs to be available as a cross-platform conda package as a bare minimum.

JohnStout commented 8 months ago

@pgunn thanks for the comment and info!

I'm less privy to the practicality of implementing fastplotlib, but I am rather convinced that visualizations like these will help users gain an intuitive understanding of parameters and caiman outputs. The latter is especially true for 1P recordings that have low signal:noise.

Fastplotlib visualizations help me define some really critical parameters, like gSig, min_corr, and min_pnr. Likewise, it is really important (for me at least) to be able to zoom into an ROI and slowly drag the time cursor to convince myself that an ROI is actually a cell (again, 1P recordings).

If all this post does is help someone googling around for visualizations, that's cool by me 😀

pgunn commented 8 months ago

@JohnStout I'm sympathetic to such things and I think we'll get there; the role I have on the project is the often unpleasant one of being the SRE/software maintainer; keeping things supportable, installable, and well-behaved from a software perspective sometimes puts constraints on when or what we can take in. Without someone taking on this role, software looks like a lot of academic software does - nothing stable and composed mostly of shortcuts. It always seems like a cool idea in the short-term, but done consistently one gets a mess.

If fpl gets conda packaging then the other issues are less severe, but I don't want anything getting into the codebase where there are either conditional imports (we've been trying to get rid of those) or require instructions to use pip.

pgunn commented 8 months ago

I am entirely open to people building packages on top of caiman that don't need to be so long-term-maintainer-perspective, and we're hoping in general to eventually have more people building things that pull caiman in as a dependency. I think Mesmerise, which Kushal wrote, is an example of a success story on that - depends on caiman, provides additional useful functionality.

EricThomson commented 8 months ago

It is Friday night I suggest we table this for the weekend before things get heated 😄 🙃

pgunn commented 6 months ago

This work is now planned as part of a reworking of the packaging that Kushal and I are going to be working on next:

Closing this issue as the design/implementation work enters the pipeline