diff --git a/docs/changelog.md b/docs/changelog.md index e3627e20..767fef46 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -27,14 +27,18 @@ behavior they verify rather than listed separately. supported. - Configure the total number of gray background points in the Amplitude View with `n_spikes_amplitudes_background` (10,000 by default). -- Waveform, Amplitude, and Correlogram views use fixed total spike budgets - across multi-cluster selections, while retaining their per-cluster ceilings. +- Waveform, Amplitude, and Correlogram views support optional fixed total spike + budgets across multi-cluster selections. They use fixed per-cluster budgets + by default so sampling accuracy does not decrease as selections grow. - Hidden or inactive-tab Waveform, Amplitude, and Correlogram views defer selection plotting until they become visible, retaining only the latest selection. ### Fixed +- Keep dataset-local view settings isolated from global GUI state. In + particular, a Firing Rate time range saved or leaked from another recording + no longer clips spikes in a fresh dataset. - Cluster and Similarity View filters only take keyboard focus after an explicit click, including when a table is first shown or refreshed. Enter, Escape, and outside clicks release filter focus so global shortcuts resume. diff --git a/docs/cli.md b/docs/cli.md index ae885ec6..0bb61a76 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -75,10 +75,11 @@ make on-demand extraction slow. The saved subset is a pool for the GUI, not the exact set drawn at once. For each selected cluster, the Waveform View chooses up to -`controller.n_spikes_waveforms` spikes from that pool (100 by default), subject -to the 400-spike `controller.n_spikes_waveforms_total` display budget. Merges -and splits continue to use whichever constituent spike IDs are present in the -pool. +`controller.n_spikes_waveforms` spikes from that pool (100 by default). +`controller.n_spikes_waveforms_total` is `None` by default, preserving the +per-cluster count as selections grow; set it to an integer to impose a shared +display budget. Merges and splits continue to use whichever constituent spike +IDs are present in the pool. It does **not** pre-extract every spike in the recording. The current phylib implementation samples from 20 representative raw-data chunks and applies the diff --git a/docs/performance.md b/docs/performance.md index da443cff..4409c85a 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -10,19 +10,27 @@ limits do not discard spikes from the dataset or change the saved clustering. | Controller setting | Default | Applied to | | --- | ---: | --- | | `n_spikes_waveforms` | 100 | Per-cluster ceiling in the Waveform View | -| `n_spikes_waveforms_total` | 400 | All displayed Waveform View clusters together | +| `n_spikes_waveforms_total` | `None` | Optional shared ceiling for displayed Waveform View clusters | | `batch_size_waveforms` | 10 | Persisted compatibility setting; the current selector does not read it | | `n_spikes_features` | 2,500 | Each displayed cluster in the Feature View | | `n_spikes_features_background` | 2,500 | Background points across the recording | | `n_spikes_amplitudes` | 10,000 | Per-cluster ceiling in the Amplitude View | -| `n_spikes_amplitudes_total` | 40,000 | All selected Amplitude View clusters together | +| `n_spikes_amplitudes_total` | `None` | Optional shared ceiling for selected Amplitude View clusters | | `n_spikes_correlograms` | 100,000 | Per-cluster ceiling for ACG/CCG computation | -| `n_spikes_correlograms_total` | 400,000 | All clusters in one ACG/CCG computation | +| `n_spikes_correlograms_total` | `None` | Optional shared ceiling for one ACG/CCG computation | -The total budgets are divided fairly among the displayed clusters. Small -clusters keep all of their available spikes and their unused shares are -redistributed to larger clusters. Setting a total budget to `None` restores -the corresponding per-cluster-only behavior. +The default `None` total settings give every cluster the corresponding +per-cluster budget, so selecting more clusters does not reduce the sampling +accuracy of any one cluster. For example, eight large clusters in the +Waveform View may each display 100 waveforms. `None` disables only the +secondary shared cap; it does not disable the per-cluster limit or load all +spikes. + +Set a total setting to an integer to opt into a fixed shared budget when +predictable runtime or memory use matters more. For example, +`n_spikes_waveforms_total = 400` divides 400 waveforms across the displayed +clusters, subject to the 100-per-cluster ceiling. Small clusters keep all +their available spikes and leave their unused shares for larger clusters. Other common cluster display limits are eight clusters for the Waveform, Feature, Amplitude, and scatter views, and 20 for histogram and Probe views. @@ -51,9 +59,15 @@ controlled separately by the controller's `n_chunks_kept` implementation. raw waveforms. - Background feature points are regularly spaced through the full spike list, independently of cluster identity. -- Correlograms use a random subset bounded by both - `n_spikes_correlograms` per cluster and - `n_spikes_correlograms_total` across the view. +- Correlograms use a random subset bounded by + `n_spikes_correlograms` per cluster. If + `n_spikes_correlograms_total` is an integer, it also bounds the entire + view. + +The Firing Rate View is different: it uses every spike in each displayed +cluster, with no spike-sampling budget. Its bin count and visible time range +control the histogram. The time range is saved in the dataset's `.phy/state.json` +and is not reused for fresh datasets. Some actions explicitly request `load_all=True` and bypass a display limit. That does not turn the corresponding view setting into a global analysis @@ -107,6 +121,8 @@ class ExampleNspikesViewsPlugin(IPlugin): @connect(sender=controller) def on_gui_ready(sender, gui): controller.n_spikes_waveforms = 500 + # Optional shared totals: remove these three lines, or use None, + # to retain the same per-cluster accuracy as selections grow. controller.n_spikes_waveforms_total = 2000 controller.n_spikes_features = 5000 controller.n_spikes_features_background = 5000 diff --git a/docs/plugins.md b/docs/plugins.md index 2bc7c5f5..011d55f6 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -130,6 +130,11 @@ class ExampleNspikesViewsPlugin(IPlugin): controller.n_spikes_features_background = 5000 # default: 2,500 total controller.n_spikes_amplitudes = 20000 # default: 10,000 per cluster controller.n_spikes_correlograms = 250000 # default: 100,000 per cluster + # These optional shared budgets default to None. Set an integer + # only when total rendering or computation cost must be bounded. + controller.n_spikes_waveforms_total = None + controller.n_spikes_amplitudes_total = None + controller.n_spikes_correlograms_total = None ``` diff --git a/docs/visualization.md b/docs/visualization.md index 8409da04..59fd815b 100644 --- a/docs/visualization.md +++ b/docs/visualization.md @@ -184,9 +184,11 @@ Snippets This view shows the waveforms of a selection of spikes, on the relevant channels (based on amplitude and proximity to the peak waveform amplitude channel). -The parameters `controller.n_spikes_waveforms=100` and -`controller.n_spikes_waveforms_total=400`, by default, specify the maximum -number of spikes per cluster and across all displayed clusters, respectively. +The parameter `controller.n_spikes_waveforms=100` specifies the maximum number +of spikes per cluster. By default, +`controller.n_spikes_waveforms_total=None`, so every displayed cluster gets +its own budget. Set the total to an integer, such as `400`, to share that fixed +number across all displayed clusters. The parameter `controller.batch_size_waveforms=10`, by default, specifies the number of batches used to extract the waveforms. Each batch corresponds to a set of successive spikes. The different batch positions are uniformly spaced in time across the entire recording. You can select a channel with **Control+click** (this impacts the feature view). You can change the scaling of the channel positions and the waveforms. @@ -313,11 +315,12 @@ class CorrelogramLimitPlugin(IPlugin): The horizontal line shows the baseline firing rate. Vertical lines show the refractory period, which defaults to 2 ms. You can change it with the view menu or with the `:cr` snippet. -The parameters `controller.n_spikes_correlograms` (100,000 by default) and -`controller.n_spikes_correlograms_total` (400,000 by default) bound the number -of spikes per selected cluster and across the entire auto- and -cross-correlogram calculation. These spikes are picked randomly. See -[Spike sampling and performance](performance.md) before increasing them. +The parameter `controller.n_spikes_correlograms` (100,000 by default) bounds +the number of spikes per selected cluster. The default +`controller.n_spikes_correlograms_total=None` preserves that per-cluster +budget as the selection grows. Set the total to an integer to bound the entire +auto- and cross-correlogram calculation. These spikes are picked randomly. +See [Spike sampling and performance](performance.md) before increasing them. You can dynamically change the window size and bin size with control+mouse wheel and alt+mouse wheel. @@ -448,9 +451,10 @@ You can toggle between different types of amplitudes by pressing `a`: #### Number of spikes. -The parameters `controller.n_spikes_amplitudes=10000` and -`controller.n_spikes_amplitudes_total=40000`, by default, bound the number of -selected spikes per cluster and across the Amplitude View. +The parameter `controller.n_spikes_amplitudes=10000` bounds the selected +spikes per cluster. The default `controller.n_spikes_amplitudes_total=None` +preserves that budget for every selected cluster. Set the total to an integer +to share a fixed budget across the Amplitude View. *Note*: currently, this number is divided by 5 for the `raw` amplitudes, so as to keep loading delays reasonable. @@ -488,7 +492,12 @@ Keyboard shortcuts This generic view shows histogram related to the selected clusters. Built-in statistics views include: * Inter-spike intervals (computed using all spikes for the selected clusters) -* Instantenous firing-rate (computed using all spikes for the selected clusters) +* Instantaneous firing rate (computed using all spikes for the selected clusters) + +The Firing Rate View does not sample or impose a spike budget. Its number of +bins and visible time range only control how all cluster spikes are grouped. +The time range is dataset-local, so a range saved for one recording does not +clip a fresh dataset. ![image](https://user-images.githubusercontent.com/1942359/58951704-193e5080-8792-11e9-873f-91a9115a9e7c.png) diff --git a/phy/apps/base.py b/phy/apps/base.py index 26062564..c3d82c79 100644 --- a/phy/apps/base.py +++ b/phy/apps/base.py @@ -184,7 +184,7 @@ def apply(self, arr, axis=None, name=None): class WaveformMixin: n_spikes_waveforms = 100 - n_spikes_waveforms_total = 400 + n_spikes_waveforms_total = None batch_size_waveforms = 10 _state_params = ( @@ -916,7 +916,7 @@ class BaseController: # Number of spikes to show in the views. n_spikes_amplitudes = 10000 # Total number of selected spikes to show in the amplitude view. - n_spikes_amplitudes_total = 40000 + n_spikes_amplitudes_total = None # Total number of background spikes to show in the amplitude view. n_spikes_amplitudes_background = 10000 @@ -925,7 +925,7 @@ class BaseController: _amplitude_functions = () n_spikes_correlograms = 100000 - n_spikes_correlograms_total = 400000 + n_spikes_correlograms_total = None # Number of raw data chunks to keep when loading waveforms from raw data (mostly useful # when using compressed dataset, as random access triggers expensive decompression). diff --git a/phy/apps/tests/test_base.py b/phy/apps/tests/test_base.py index b3a0213b..8985d469 100644 --- a/phy/apps/tests/test_base.py +++ b/phy/apps/tests/test_base.py @@ -167,6 +167,17 @@ def test_allocate_spike_counts_redistributes_total_budget(): _allocate_spike_counts([100, 2], per_cluster=10, total=None), [10, 2], ) + assert WaveformMixin.n_spikes_waveforms_total is None + np.testing.assert_array_equal( + _allocate_spike_counts( + [100, 100, 100], + per_cluster=WaveformMixin.n_spikes_waveforms, + total=WaveformMixin.n_spikes_waveforms_total, + ), + [100, 100, 100], + ) + assert BaseController.n_spikes_amplitudes_total is None + assert BaseController.n_spikes_correlograms_total is None def test_controller_close(tempdir): @@ -239,7 +250,7 @@ def test_get_correlograms_rate_fast_path(): controller.model = Bunch(duration=4.0) controller.supervisor = Bunch(clustering=clustering) controller.n_spikes_correlograms = 2 - controller.n_spikes_correlograms_total = None + assert controller.n_spikes_correlograms_total is None def fail_selector(*args, **kwargs): raise AssertionError('The correlogram-rate path must not invoke SpikeSelector.') diff --git a/phy/cluster/views/base.py b/phy/cluster/views/base.py index fd9ddd3f..043e1dfd 100644 --- a/phy/cluster/views/base.py +++ b/phy/cluster/views/base.py @@ -285,6 +285,9 @@ def on_visibility_changed(visible): self.dock.visibilityChanged.connect(on_visibility_changed) + local_keys = [f'{self.name}.{key}' for key in self.local_state_attrs] + gui.state.add_local_keys(local_keys) + # Set the view state. self.set_state(gui.state.get_view_state(self)) diff --git a/phy/cluster/views/tests/test_histogram.py b/phy/cluster/views/tests/test_histogram.py index f43b7d27..7a5e4a47 100644 --- a/phy/cluster/views/tests/test_histogram.py +++ b/phy/cluster/views/tests/test_histogram.py @@ -7,7 +7,7 @@ import numpy as np from phylib.utils import Bunch -from ..histogram import HistogramView +from ..histogram import FiringRateView, HistogramView from . import _stop_and_close # ------------------------------------------------------------------------------ @@ -68,3 +68,21 @@ def test_histogram_view_0(qtbot, gui): v.decrease() _stop_and_close(qtbot, v) + + +def test_firing_rate_view_ignores_global_x_max(qtbot, gui): + gui.state.FiringRateView = Bunch(n_bins=200, x_max=12.0) + v = FiringRateView( + cluster_stat=lambda cluster_id: Bunch( + data=np.array([1.0, 20.0]), + x_min=0.0, + x_max=30.0, + ) + ) + + v.attach(gui) + assert v.x_max is None + v.on_select(cluster_ids=[0]) + assert v.x_max == 30.0 + + _stop_and_close(qtbot, v) diff --git a/phy/gui/state.py b/phy/gui/state.py index a52c61f8..9000cc97 100644 --- a/phy/gui/state.py +++ b/phy/gui/state.py @@ -82,6 +82,18 @@ def _recursive_update(d, u): return d +def _nested_keys(value, prefix=''): + """Return dotted paths for all leaves in a nested mapping.""" + out = set() + for key, item in value.items(): + path = f'{prefix}.{key}' if prefix else key + if isinstance(item, Mapping): + out.update(_nested_keys(item, path)) + else: + out.add(path) + return out + + def _get_local_data(d, local_keys): """Return the Bunch of local data from a full GUI state, and a list of local keys of the form `ViewName.field_name`.""" @@ -139,6 +151,7 @@ def __init__( self, path=None, local_path=None, default_state_path=None, local_keys=None, **kwargs ): super().__init__(**kwargs) + self._loaded_local_keys = set() self._path = Path(path) if path else None if self._path: ensure_dir_exists(str(self._path.parent)) @@ -155,7 +168,11 @@ def __init__( def get_view_state(self, view): """Return the state of a view instance.""" - return self.get(view.name, Bunch()) + state = deepcopy(self.get(view.name, Bunch())) + for key in getattr(view, 'local_state_attrs', ()): + if f'{view.name}.{key}' not in self._loaded_local_keys: + state.pop(key, None) + return state def update_view_state(self, view, state): """Update the state of a view instance. @@ -171,6 +188,11 @@ def update_view_state(self, view, state): if name not in self: self[name] = Bunch() self[name].update(state) + local_keys = { + key.split('.', 1)[1] for key in self._local_keys if key.startswith(f'{name}.') + } + if local_keys: + self._loaded_local_keys.update(f'{name}.{key}' for key in state if key in local_keys) logger.debug('Update GUI state for %s', name) def _copy_default_state(self): @@ -204,7 +226,9 @@ def load(self): # After having loaded the global state, load the local state if it exists. # If values already exist, they are updated. if self._local_path and self._local_path.exists(): - _recursive_update(self, _load_state(self._local_path)) + local_data = _load_state(self._local_path) + self._loaded_local_keys = _nested_keys(local_data) + _recursive_update(self, local_data) @property def _global_data(self): diff --git a/phy/gui/tests/test_state.py b/phy/gui/tests/test_state.py index 5fba89ab..23fb64cb 100644 --- a/phy/gui/tests/test_state.py +++ b/phy/gui/tests/test_state.py @@ -89,3 +89,39 @@ def test_gui_state_view_3(tempdir): data_1 = {'a': {'b': 3, 'c': 3}} assert state == data_1 assert state._local_data == {'a': {'b': 3}} + + +def test_gui_state_view_local_attributes_do_not_leak_from_global_state(tempdir): + global_path = tempdir / 'global/state.json' + local_path = tempdir / 'local/state.json' + view = Bunch(name='FiringRateView', local_state_attrs=('x_max',)) + save_json( + global_path, + {'FiringRateView': {'n_bins': 200, 'x_max': 12.0}}, + ) + + state = GUIState(global_path, local_path=local_path) + state.add_local_keys(['FiringRateView.x_max']) + + assert state.get_view_state(view) == {'n_bins': 200} + + +def test_gui_state_view_loads_and_updates_local_attributes(tempdir): + global_path = tempdir / 'global/state.json' + local_path = tempdir / 'local/state.json' + view = Bunch(name='FiringRateView', local_state_attrs=('x_max',)) + save_json( + global_path, + {'FiringRateView': {'n_bins': 200, 'x_max': 12.0}}, + ) + save_json(local_path, {'FiringRateView': {'x_max': 30.0}}) + + state = GUIState(global_path, local_path=local_path) + state.add_local_keys(['FiringRateView.x_max']) + assert state.get_view_state(view) == {'n_bins': 200, 'x_max': 30.0} + + state.update_view_state(view, {'n_bins': 300, 'x_max': 40.0}) + assert state.get_view_state(view) == {'n_bins': 300, 'x_max': 40.0} + state.save() + assert load_json(global_path) == {'FiringRateView': {'n_bins': 300}} + assert load_json(local_path) == {'FiringRateView': {'x_max': 40.0}} diff --git a/plugins/n_spikes_views.py b/plugins/n_spikes_views.py index a53c8c0d..1910f43e 100644 --- a/plugins/n_spikes_views.py +++ b/plugins/n_spikes_views.py @@ -23,3 +23,8 @@ def on_gui_ready(sender, gui): controller.n_spikes_features_background = 5000 # default: 2,500 total controller.n_spikes_amplitudes = 20000 # default: 10,000 per cluster controller.n_spikes_correlograms = 250000 # default: 100,000 per cluster + # These optional shared budgets default to None. Set an integer + # only when total rendering or computation cost must be bounded. + controller.n_spikes_waveforms_total = None + controller.n_spikes_amplitudes_total = None + controller.n_spikes_correlograms_total = None