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

[MRG] ENH: plot_spikes_raster to always include all cell types #754

Merged

Conversation

samadpls
Copy link
Contributor

Closes #658

  • Updated plot_spikes_raster logic to always include all cell types.

@jasmainak
Copy link
Collaborator

Can you share before/after for a situation where the cell wasn't spiking before?

You can try plotting the spike raster in the alpha example which is subthreshold activity

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch from ba681c4 to 71bccb5 Compare May 9, 2024 14:38
@samadpls
Copy link
Contributor Author

samadpls commented May 9, 2024

Apologies for the delay. Here's the comparison,

Before After
image Figure_1

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch 2 times, most recently from b63b459 to 119fef3 Compare May 10, 2024 15:31
@samadpls samadpls changed the title [WIP] ENH: plot_spikes_raster to always include all cell types ENH: plot_spikes_raster to always include all cell types May 10, 2024
@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch from 119fef3 to d804451 Compare May 13, 2024 14:33
@jasmainak
Copy link
Collaborator

Here's the comparison,

something is off here ... you shouldn't be getting spiking in this example, no?

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch 2 times, most recently from ae51896 to 9217f42 Compare May 13, 2024 18:28
@samadpls
Copy link
Contributor Author

You're correct that in the simulation of alpha and beta rhythms in subthreshold conditions, cells should not be spiking.
here is the comparison,

Before After
image Figure_1

@jasmainak
Copy link
Collaborator

Better! But does the y-axis go from 0 to 270? That is to say ... how do we make it obvious to the user that there is no spiking or that only some subsets of neurons are spiking?

@jasmainak
Copy link
Collaborator

perhaps @ntolley or @rythorpe can share examples of raster plots they have seen in publications / other software ...

@samadpls
Copy link
Contributor Author

samadpls commented May 13, 2024

Better! But does the y-axis go from 0 to 270? That is to say ... how do we make it obvious to the user that there is no spiking or that only some subsets of neurons are spiking?

Perhaps we can set the y-axis limit to this particular range [0,270].

Edit: Perhaps we can set the y-axis limit to the range from -total_neurons to 0

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch 4 times, most recently from f064b35 to f0900f4 Compare May 20, 2024 14:44
@samadpls samadpls requested a review from ntolley May 20, 2024 14:45
@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch 2 times, most recently from a964437 to 2b0ae73 Compare May 26, 2024 14:51
@ntolley
Copy link
Contributor

ntolley commented May 29, 2024

Edit: Perhaps we can set the y-axis limit to the range from -total_neurons to 0

This is really all we need to do!

@jasmainak
Copy link
Collaborator

Let's do that. Can you share how the other example plots look after doing so?

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch from 2b0ae73 to 02299b2 Compare May 29, 2024 15:43
@samadpls
Copy link
Contributor Author

Let's do that. Can you share how the other example plots look after doing so?

Sure, I just ran the Gamma Rhythms. However, I received a graph with a lot of empty space at the bottom, which I believe was due to setting the Y-limit for all conditions. Therefore, I just moved the position and got the same plot as in the example. Here are the before and after changes.

Before After
output Figure_1

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch from 02299b2 to 166efc9 Compare May 29, 2024 15:49
@ntolley
Copy link
Contributor

ntolley commented May 30, 2024

Ah I see! Sorry I think the actual solution is to have the y-axis correspond to the gid of each cell. The tricky part is if we specify only a subset of cells to plot, then the gids used for plotting will need to be updated so that they stop at zero.

Ultimately the idea is that if you compare plots from two different simulations, you want the y-axis to mean the same thing. Right now the y-axis changes depending on the number of cells that spike.

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch 2 times, most recently from 2e55a16 to a331a23 Compare May 30, 2024 13:41
@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch 4 times, most recently from 3b97c3f to 9f1c7eb Compare June 12, 2024 18:23
@ntolley
Copy link
Contributor

ntolley commented Jun 14, 2024

@samadpls when you're back from exams can you try deleting the text in gui/index.rst? Everything except "The brand-new lightweight web-based hnn GUI that works with hnn-core."

I think the GUI docs are still technically being built despite removing it from the navbar through conf.py

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch 2 times, most recently from 4ee3ea1 to 83aea9b Compare June 14, 2024 16:12
@samadpls
Copy link
Contributor Author

Hey @ntolley , I made the changes and added gui/index.rst to exclude_patterns in conf.py to exclude it from the build. The GUI section is no longer being built.

@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch from 83aea9b to 636e365 Compare June 30, 2024 12:05
@samadpls samadpls changed the title ENH: plot_spikes_raster to always include all cell types [MRG] ENH: plot_spikes_raster to always include all cell types Jul 2, 2024
@samadpls samadpls force-pushed the fix/raster-plot-neuron-display branch from 636e365 to 909c000 Compare July 8, 2024 14:15
@samadpls
Copy link
Contributor Author

samadpls commented Jul 8, 2024

Hey @ntolley, this is all set for merging. Feel free to do it whenever it's convenient for you!

@ntolley
Copy link
Contributor

ntolley commented Jul 8, 2024

Looks great @samadpls!! Can you update whats_new.rst?

@ntolley ntolley merged commit 0778d87 into jonescompneurolab:master Jul 8, 2024
12 checks passed
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.

cell response raster plots should show all neurons in the network model
3 participants