Add splitting functionality to curation and SortingAnalyzer
#3817
Conversation
| old_template = arr[self.sorting_analyzer.sorting.ids_to_indices([split_unit_id])[0], ...] | ||
| new_indices = np.array([new_unit_ids.index(unit_id) for unit_id in new_splits]) | ||
| new_array[new_indices, ...] = np.tile(old_template, (len(new_splits), 1, 1)) |
There was a problem hiding this comment.
@samuelgarcia this needs to be discussed. What should we do if the waveforms extension is not there? The current behavior is copying, but we mught want to force recompute here
There was a problem hiding this comment.
maybe add warning here!
|
This is amazing camarade. I would do first a PR to focus only on the format. And then merge this one. |
|
I did camarade! #3760 |
| full_spike_indices = [] | ||
| for label in np.unique(self.labels): | ||
| label_indices = np.where(self.labels == label)[0] | ||
| full_spike_indices.append(label_indices) |
There was a problem hiding this comment.
maybe make a helper function that returns a dict of indices given an array
| # we propagate random spikes to avoid random spikes to be recomputed | ||
| extension_dict_ = extension_dict_split.copy() | ||
| extension_dict_.pop("random_spikes") | ||
| analyzer_hard.extensions["random_spikes"] = analyzer_split.extensions["random_spikes"] |
There was a problem hiding this comment.
do a copy and ref to the analyzer here
|
|
||
|
|
||
| ### SPLITTING ZONE ### | ||
| def apply_splits_to_sorting(sorting, unit_splits, new_unit_ids=None, return_extra=False, new_id_strategy="append"): |
There was a problem hiding this comment.
maybe a small doc no ?
You are becoming more lazy than me.
| new_unit_ids.append([str(m + i) for i in range(num_splits)]) | ||
| else: | ||
| # we cannot automatically find new names | ||
| new_unit_ids.append([f"split{i}" for i in range(num_splits)]) |
There was a problem hiding this comment.
Maybe we could put also the old unit id... not sure
| else: | ||
| # dtype int | ||
| new_unit_ids.append(list(max(old_unit_ids) + 1 + np.arange(num_splits, dtype=dtype))) | ||
| old_unit_ids = np.concatenate([old_unit_ids, new_unit_ids[-1]]) |
There was a problem hiding this comment.
using this variable name for the growing new unit id set is a bo confusing at the first read.
There was a problem hiding this comment.
And the [-1] is not obvious
| split_2 = split_2[~np.isin(split_2, split_indices)] | ||
| new_split_indices = [split_indices, split_2] | ||
| else: | ||
| new_split_indices = split_indices |
There was a problem hiding this comment.
Do we need to check ompleness here ?
There was a problem hiding this comment.
salut camarade. I just made some changes and check for completeness in the apply_splits_to_sorting and SortingAnalyzer.split_units.
This forces the user to provide complete spike indices, which is done automatically though the curation module and apply_curation function
| keep_mask=keep_mask, | ||
| verbose=verbose, | ||
| **job_kwargs, | ||
| ) | ||
| elif merging_mode == "hard": | ||
| recompute_dict[extension_name] = extension.params | ||
| else: | ||
| # split | ||
| try: |
There was a problem hiding this comment.
try is not agood idea here for this new mechanism, we must capture any new bug!
There was a problem hiding this comment.
removed and added splitting mode
| assert analyzer_curated.sorting.get_property("pyramidal", ids=[unit_id])[0] | ||
|
|
||
|
|
||
| def test_apply_curation_with_split_multi_segment(): |
There was a problem hiding this comment.
I would make only a multi segment tests every where to avoid mono/multi segment tests
| { | ||
| "unit_id": 2, | ||
| "mode": "labels", | ||
| "labels": split_labels.tolist(), |
There was a problem hiding this comment.
should we tests also the other representation by indices ?
Co-authored-by: Garcia Samuel <[email protected]>
|
yeah! |
Depends on #3760
Implements splitting functionality in:
sorting_tools:apply_splits_to_sortingSortingAnalyzer:.split_units()AnalyzerExtension:_split_extension_data(implemented function for all extensions and extended tests)