-
Notifications
You must be signed in to change notification settings - Fork 240
Fix edges of 2d-histogram in UnitWaveformDensityMapWidget #4283
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
base: main
Are you sure you want to change the base?
Conversation
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.
Not sure I like the idea of having a value hardcoded in the widget (num_bins=100), but it used to already be like that before, so we can leave it as it is. Thanks a lot for the patch! @samuelgarcia mayeb worth propagating in spikeinterface-gui also?
|
Hi. About the histogram vs "devide and round" did you benchmark the speed ? |
|
Hi @samuelgarcia. Just did some tests, newer solution becomes faster at around 1000-2000 spikes (both are under 20ms at that point anyway. For larger values numpy beats it. |
for more information, see https://pre-commit.ci
|
Added a couple of commits, one is just code formatting (c30f587). The other (87fdaa7) changes the x axis of the plot to show absolute time (msecs). I think it will be useful because no other plot shows the waveforms/templates in an absolute clock (plot_waveforms/templates has unitless x axis, it does have a scale bar but it only works under particular circumstances). It is nice to have the x axis as msec if you wanna estimate how wide a spike is or the distance between two peaks in the template or so on. I tested it with all params (1 vs many channels, and same_axis=True/False) and it works fine. Here's what the plots look like: Note: When all channels are plotted, rather than creating a different axes per channel, the concatenated image is plotted in a single axis and some vertical white lines are plotted to separate the channels. So the x axis still goes on real time but it is not separate per plot, not ideal but I think good enough. |
|
Really cool. |









Currently,

sw.plot_unit_waveforms_density_map(analyzer, unit_ids=[0], use_max_channel=True)produces this:The density map has some spurious high values on the very first row (more visible near the peaks) and also on the bottom although less visible in this example.
The issue comes from the clipping in
spikeinterface/src/spikeinterface/widgets/unit_waveforms_density_map.py
Line 129 in 5a58beb
that counts any value with amplitude less than bin_min as belonging to the first bin and any value above bin_max as belonging to the last bin of the histogram, thus producing those lines on the plot.
This PR uses equal-sized bins from bin_min to bin_max and uses np.histogram to count occurrences. It produces:

without any artifacts. (Also tested it with multiple channels and multiple units)
Granted, it looks uglier but it's more correct. And it will look better for nicer data, this is a 2-minute recording just for testing.