-
Notifications
You must be signed in to change notification settings - Fork 245
Valid unit periods extension #4299
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
Merged
Merged
Changes from all commits
Commits
Show all changes
118 commits
Select commit
Hold shift + click to select a range
3dc5729
Test IBL extractors tests failing for PI update
alejoe91 d1a0532
Merge branch 'main' of github.com:SpikeInterface/spikeinterface
alejoe91 79ca022
original commit - good times
m-beau 22501da
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 7ca3d35
good times - progress
m-beau e3f31bf
merge
m-beau ab0e8dc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] fe0aaf1
Merge remote-tracking branch 'alessio/select_sorting_periods' into go…
m-beau 7279b67
wip
alejoe91 13ebb8f
Merge remote-tracking branch 'alessio/select_sorting_periods' into go…
m-beau 1962f21
Fix test for base sorting and propagate to basevector extension
alejoe91 7fbe160
wip
m-beau 5645ee6
Merge branch 'select_sorting_periods' of https://github.com/alejoe91/…
m-beau 528c82b
Fix tests in quailty metrics
alejoe91 fccdbe3
finished implementing good periods
m-beau 7adab75
Merge branch 'select_sorting_periods' into goodtimes
alejoe91 f36c7fc
Some fixes
alejoe91 775dda7
Fix retrieval of spikevector features
alejoe91 6f02b7f
Merge branch 'select_sorting_periods' into goodtimes
alejoe91 15df754
Fix tests, saving and loading
alejoe91 40e3417
started working on get_data method for good periods
m-beau cdf7846
Solve conflicts, still wip
alejoe91 81d745e
done refactoring self.data serializable format and get_data method
m-beau 93a53ca
credits
m-beau 493d215
Make good_periods blazing fast!
alejoe91 a1fb167
Add credits
alejoe91 e8518b0
Solve conflicts
alejoe91 f6752ac
Fix tests
alejoe91 a251826
oups
alejoe91 983d255
Sam's review + implement select/merge/split data
alejoe91 c5dbb93
Rename to valid_unit_periods and wip widgets
alejoe91 f382f89
Fix imports
alejoe91 ad50845
Add widget and extend params
alejoe91 bb46f27
Update src/spikeinterface/core/sorting_tools.py
alejoe91 121a0b1
Apply suggestion from @chrishalcrow
alejoe91 cbf3213
refactor presence ratio and drift metrics to use periods properly
alejoe91 4409aa5
Fix rp_violations
alejoe91 71f8668
implement firing range and fix drift
alejoe91 1ea0d68
fix naming issue
alejoe91 a86c2d3
remove solved todos
alejoe91 d98ff66
sync with select_sorting_period PR
alejoe91 84da1a2
wip: test user defined
alejoe91 c539f58
wip: tests
alejoe91 d8e1f90
Merge branch 'main' of github.com:SpikeInterface/spikeinterface into …
alejoe91 3f93f97
Implement select_segment_periods in core
alejoe91 cd85456
remove utils
alejoe91 7a42fe3
rebase on #4316
alejoe91 4f754cb
Merge with main
alejoe91 cbc0986
Fix import
alejoe91 56b672e
Merge branch 'select_sorting_periods_core' into select_sorting_periods
alejoe91 cd2ba0b
wip
alejoe91 046430e
fix import
alejoe91 bb86253
Add misc_metric changes
alejoe91 accbc31
Merge branch 'select_sorting_periods' into goodtimes
alejoe91 807f5c6
Add tests for user defined and combined
alejoe91 89d563b
Add to built_in extensions
alejoe91 50f33f0
fix tests
alejoe91 f2d48ba
Remove debug print
alejoe91 6b48730
Merge branch 'select_sorting_periods' into goodtimes
alejoe91 6b1284b
Merge branch 'goodtimes' of github.com:m-beau/spikeinterface into goo…
alejoe91 e173a63
wip: fix intervals
alejoe91 80bc50f
Change base_period_dtype order and fix select_sorting_periods array i…
alejoe91 4c8fa23
fix conflicts
alejoe91 e1f5bab
Merge metrics implementations
alejoe91 96e6a53
fix tests
alejoe91 3198911
Fix generation of bins
alejoe91 bbc28c5
Refactor generation of subperiods
alejoe91 9d2ad09
fix conflicts
alejoe91 8312db2
fix conflicts2
alejoe91 87fbe9a
Merge branch 'main' of github.com:SpikeInterface/spikeinterface into …
alejoe91 7446a43
Use cached get_spike_vector_to_indices
alejoe91 873a687
Solve conflicts
alejoe91 bc91b81
fix conflicts3
alejoe91 51e906a
Fix error in merging
alejoe91 88da6fc
Merge branch 'select_sorting_periods' into goodtimes
alejoe91 ab5a771
fix conflicts
alejoe91 2209514
Add supports_periods in BaseMetric/Extension
alejoe91 b23c431
wip: test metrics with periods
alejoe91 6fb26a4
almost there?
alejoe91 0fe7f3e
Fix periods arg in MetricExtensions
alejoe91 f087e08
Make bin edges unique
alejoe91 e785b64
fix conflicts with selecto_sorting_periods
alejoe91 173e747
Add support_periods to spike train metrics and tests
alejoe91 066c378
Force NaN/-1 values for float/int metrics if num_spikes is 0
alejoe91 65e1848
Fix test_empty_units: -1 is a valid value for ints
alejoe91 f1c4682
Fix firing range if unit samples < bin samples
alejoe91 3291638
fix noise_cutoff if empty units
alejoe91 b5bf3c3
Move warnings at the end of the loop for firing range and drift
alejoe91 8aeedcc
clean up tests and add get_available_metric_names
alejoe91 d4db43c
simplify total samples
alejoe91 d0a1e66
Go back to Pierre's implementation for drifts
alejoe91 6926532
Merge branch 'select_sorting_periods' into goodtimes
alejoe91 a1e750d
solve conflicts and fix tests
alejoe91 4909bfb
Add in docs
alejoe91 2739df8
Add use_valid_periods param to quality metrics
alejoe91 2b27c40
Force int64 in tests
alejoe91 a0aad71
Add clip_amplitude_scalings arg in plot
alejoe91 4c203d5
Fix serialization/deserialization to Zarr
alejoe91 d42b6a2
Fix reloading extension data in zarr
alejoe91 e4e21cb
Simplify extension data format and ass helper functions
alejoe91 4130565
Set segment_index to 0 if one segment in widget
alejoe91 4ce906c
Sort extensions by dependency in recompute_dict
alejoe91 a92a44e
Chris' comments and proper sorting upon recompute
alejoe91 5008024
Remap unit_index in all/valid periods after merge/split
alejoe91 1d257b6
propagate logic to split
alejoe91 c699686
name -> extension_name
alejoe91 03e83b9
Fix merges and splits
alejoe91 ac65c99
remove redundant extension sort
alejoe91 99a5150
fix docs, citation, and slice
alejoe91 bdd000d
Merge branch 'main' into goodtimes
alejoe91 43fdf2b
Use built-in segment slices
alejoe91 89dffa1
greatly simplify remapping with remap_unit_indices_in_vector function
alejoe91 45032c6
Fix tests
alejoe91 a3ca29e
Use unmerged/unsplit when keeping unit ids
alejoe91 30c67ef
test for remap_unit_indices_in_vector
samuelgarcia 6d18d83
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 72a4e68
Apply suggestion from @alejoe91
alejoe91 c4e5551
Merge branch 'main' into goodtimes
alejoe91 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why is this so?
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.
Because they are much more robust. Spike amplitudes only sample the voltage value on the max channel, amplitude scalings use sparse templates and fit them spike-by-spike, also handling collisions. Downside is it's slower to compute
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.
@alejoe91 Thanks for the explanation.
Currently, the documentation is unclear. Both spike_amplitudes and amplitude_scalings have the same note but users won't know whether they need to compute both or only one of them, which one is the recommended one (if any) or what happens if you compute both.

Currently, spike_amplitudes is the default everywhere:
amplitude_cutoff,noise cutoffwill use spike_amplitudes by default even if amplitude_scalings is available;ampitude_cvreceives the extension as argument (default is spike_amplitudes) andamplitude mediandoes not even consider amplitude_scalings.Maybe the note should just say, one of the two is required or just recommend one of them. I particularly prefer spike_amplitudes, even if noisier, it is more understandable and a lot less margin for error.