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

Allow export_report to run without waveforms #3493

Merged
merged 4 commits into from
Nov 4, 2024

Conversation

chrishalcrow
Copy link
Collaborator

Small edit to export_report so that it will generate a report without waveforms.

When we're being speedy, we often don't compute or don't copy over waveforms. This PR just changes export_report so that it does/doesn't include the UnitWaveformDensityMapWidget if the sorting_analyzer does/doesn't have waveforms computed.

A unit report without waveforms:

4

computed using

import spikeinterface.full as si
rec, sort = si.generate_ground_truth_recording()
sa = si.create_sorting_analyzer(recording=rec, sorting=sort)
sa.compute(["random_spikes",  "templates", "spike_amplitudes", "correlograms", "template_similarity"])
si.export_report(sorting_analyzer=sa, output_folder="my_report")

@chrishalcrow chrishalcrow added enhancement New feature or request widgets Related to widgets module labels Oct 21, 2024
@zm711
Copy link
Collaborator

zm711 commented Oct 21, 2024

I think this is an improvement. Another request we should think about is to have more flexibility in selecting what to plot in general. There are complaints about the slowness of making the summary when you only want some features to be saved. So something like this could be extended to help speed things up for people.

@samuelgarcia
Copy link
Member

Good idea. thanks Chris.

@chrishalcrow
Copy link
Collaborator Author

I think this is an improvement. Another request we should think about is to have more flexibility in selecting what to plot in general. There are complaints about the slowness of making the summary when you only want some features to be saved. So something like this could be extended to help speed things up for people.

Mmm, good idea. So at the moment we include 5 widgets. Your suggestion would be to allow the user to select which ones they'd like, and we format the report appropriately? Sounds doable (and extendable for other widgets!) in another PR.

@zm711
Copy link
Collaborator

zm711 commented Oct 23, 2024

in another PR.

agreed. :)

Copy link
Collaborator

@JoeZiminski JoeZiminski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice @chrishalcrow this is really cool, the export report is a great feature and making it more flexible is very useful. Also agree that custom selecting of subplots would be very nice (in another PR).

I ran it and it all worked nicely. Some small changes suggested, apologies some are outside this diff as I was reading the code in another editor, but I guess now would be a good time to change them.

src/spikeinterface/widgets/unit_summary.py Outdated Show resolved Hide resolved
src/spikeinterface/widgets/unit_summary.py Outdated Show resolved Hide resolved
src/spikeinterface/widgets/unit_summary.py Show resolved Hide resolved
src/spikeinterface/widgets/unit_summary.py Show resolved Hide resolved
src/spikeinterface/widgets/unit_summary.py Show resolved Hide resolved
@chrishalcrow
Copy link
Collaborator Author

Thanks @JoeZiminski - will take a look on Monday!

@chrishalcrow
Copy link
Collaborator Author

All updated - also fixed a column counting issue that occurred when correlograms and spike_amplitudes were on, but waveforms were off. Also added in a check for template_similarity, which is needed to compute correlograms it seems.

@zm711
Copy link
Collaborator

zm711 commented Oct 29, 2024

template_similarity, which is needed to compute correlograms it seems.

yeah the correlograms are only calculated for templates deemed similar enough. That's annoyed me in the past, but I understand why it is there.

@chrishalcrow
Copy link
Collaborator Author

Hello, could this be merged please(@samuelgarcia )? Just to sum up, here is every possible report output, made from

import spikeinterface.full as si
rec, sort = si.generate_ground_truth_recording(durations=[60])
sa = si.create_sorting_analyzer(recording=rec, sorting=sort)

then...

sa.compute(["random_spikes", "unit_locations","templates","template_similarity"])
0

sa.compute(["random_spikes", "unit_locations", "templates","template_similarity", "correlograms"])
0

sa.compute(["random_spikes", "unit_locations", "templates","template_similarity", "waveforms"])
0

sa.compute(["random_spikes", "unit_locations", "templates","template_similarity", "spike_amplitudes"])
0

sa.compute(["random_spikes", "unit_locations", "templates", "spike_amplitudes", "correlograms"])
(note: no template similarity => no correlograms!)
0

sa.compute(["random_spikes", "unit_locations","templates","template_similarity", "waveforms", "correlograms"])
0

sa.compute(["random_spikes", "unit_locations","templates","template_similarity", "waveforms", "spike_amplitudes"])
0

sa.compute(["random_spikes", "unit_locations","templates","template_similarity", "waveforms", "correlograms", "spike_amplitudes"])
0

@alejoe91
Copy link
Member

alejoe91 commented Nov 4, 2024

@chrishalcrow @zm711 I pushed an update that removes the need to compute template_similarity, which is counter-intuituve. In fact, this is only needed for cross-correlograms, but not for auto-correlogram plots.

Tested on my side and works like a charm!

@alejoe91 alejoe91 added the exporters Related to exporters module label Nov 4, 2024
@chrishalcrow
Copy link
Collaborator Author

@chrishalcrow @zm711 I pushed an update that removes the need to compute template_similarity, which is counter-intuituve. In fact, this is only needed for cross-correlograms, but not for auto-correlogram plots.

Tested on my side and works like a charm!

Beautiful - works on my side too.

@alejoe91 alejoe91 merged commit 0baf7e0 into SpikeInterface:main Nov 4, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exporters Related to exporters module widgets Related to widgets module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants