Restore per-cluster view spike budgets

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.

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

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

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
 
 ```
 

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)
 

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).

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.')

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))
 

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)

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):

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}}

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