SortingAnalyzer: add read_only mode (different than lazy) - #4713
SortingAnalyzer: add read_only mode (different than lazy)#4713alejoe91 wants to merge 10 commits into
SortingAnalyzer: add read_only mode (different than lazy)#4713Conversation
alejoe91
added
core
alejoe91
changed the title
SortingAnalyzer: add read_only mode (different than lazy`)SortingAnalyzer: add read_only mode (different than lazy)
|
|
||
| def _split_extension_data(self, split_units, new_unit_ids, new_sorting_analyzer, verbose=False, **job_kwargs): | ||
| # splitting only affects random spikes, not waveforms | ||
| new_data = dict(waveforms=self.data["waveforms"].copy()) |
There was a problem hiding this comment.
Lets discuss a bit this copy( that disappear everywhere.
For the lazy it makes sens but for i memory there is drwback no ?
and also what if the source array disapear in between the futur save ?
There was a problem hiding this comment.
I added materialize_array for all returned extension data from select/split/merge. This is particularly relevant for sequential curations.
@chrishalcrow we can handle more efficient ways to apply curations later
| tmp_path / "analyzer_lazy", format="auto", lazy=True, read_only=True | ||
| ) | ||
| sorting_analyzer_lazy_ro.compute( | ||
| "quality_metrics", metric_names=metric_names, seed=1205, metric_params=metric_params, save=True |
There was a problem hiding this comment.
not sure to understand the API we trying to save a read only analyzer we should raiuse error no ?
what is the expected behavior ?
There was a problem hiding this comment.
right now it jus doesn't save. Should we raise? Or warn maybe?
There was a problem hiding this comment.
I think if a user (or external software e.g. spikeinterface-gui) uses read_only then they have a good reason for doing so. And it is correct that we shouldn't allow saving even if they pass save=True (they might accidentally do this). We would expect them to do this:
test_metrics = sorting_analyzer_lazy_ro.compute("quality_metrics", metric_names=metric_names, seed=1205, metric_params=metric_params)
which shouldn't warn or raise, since it's a legit things to do. Note that save=True is default.
So I think the current behavior is correct: don't save and don't warn and don't raise.
|
|
||
| correlograms, new_bins = deepcopy(self.get_data()) | ||
| correlograms = materialize_array(self.data["ccgs"]) | ||
| new_bins = self.data["bins"] |
There was a problem hiding this comment.
no copy ? this will keep a ref to the old analyzer that will be never garbage collected.
| If True the extension is saved to disk (only if sorting analyzer format is not "memory"). | ||
| If False the extension will only live in memory as long as the object is deleted. |
There was a problem hiding this comment.
add a comment about save being ignored if read_only
3 participants
In #4623 (and #4709) we added a
lazymode for loading theSortingAnalyzer. This kept extension data and spike vector unmaterialized to avoid loading large arrays in memory. The lazy mode though did not allow any computation.However, lazy mode could be used also to compute extensions for large datasets (see #4703) to minimize the overall memory footprint of computations.
To make these 2 use cases separate, this PR adds a
read_onlyflag for loadingSortingAnalyzer(e.g., when loading for the GUI), which prevents any saving of extensions). Thelazymode has been extended for creating analyzers, computing, and svaing/merging/splitting.For very large datasets, one could now use this (especially in conjunction with #4703), to avoid loading large arrays in memory:
The PR also propagates the lazy argument to the parallel computation of PCA metrics.