Add time slicing benchmark by oruebel · Pull Request #203 · NeurodataWithoutBorders/nwb-benchmarks

oruebel · 2026-10-06T20:14:40Z

Benchmarks:

Added opt-in incremental slicing benchmark controls via RUN_INCREMENTAL_SLICING_BENCHMARKS environment variable, including support for running all modalities or selected ecephys, ophys, and icephys combinations. E.g., to run the incremental slicing tests for just icephys one can call:
```
RUN_INCREMENTAL_SLICING_BENCHMARKS=icephys \
nwb_benchmarks run \
--bench "track_incremental_slicing" 
```
Documented relevant runtime environment variables for network tracking, download benchmarks, and incremental slicing benchmark selection.
Extended database result export with slice_template and slice_strategy parameter metadata.
- slice_strategy tells the benchmark how to perform the incremental read sequence.
  - iterate_time_axis: used for ecephys/ophys array-like datasets. It repeatedly reads adjacent chunks along axis 0.
  - iterate_icephys_timeseries: used for icephys files. It iterates over TimeSeries-like objects in acquisition/stimulus and reads each one.
- slice_template defines the shape of each repeated slice for iterate_time_axis.
  - For example, the first ecephys/ophys slice is used as a template, and the benchmark advances that slice along the time axis until the dataset is exhausted.
  - Icephys does not use slice_template, because it reads whole TimeSeries objects instead.

Result Parsing:

Improved ASV result reduction to support both older and newer ASV result layouts, skipped/null results, and dict-valued cumulative slicing outputs.

Note:

For icephys instead of advancing a slice within a timeseries, I think it makes more sense to incrementally read the different timeseries, since each timeseries is small so incrementally reading slices quickly exhaust the timeseries and leaves most of the icephys file untouched since the data here is distributed across many small timeseries.
I limited the tests for now to LINDI, fsspec HTTPS cached, remfile cache, ROS3, and Zarr with consolidated metadata for remote slicing but we can add the others ones too if you want

…dalitities via an environment variable

Copilot

Copilot review overview

🟡 Changes recommended

Result-reduction errors and missing incremental benchmark implementations block approval.

Review effort: Balanced
Findings: 2 · 1

Open (3)

Preserve parameter-axis shape in legacy reduction · New
Prefer recorded samples over aggregate results · New
Implement and register incremental slicing benchmarks · New

What changed in this PR

Prepares opt-in incremental slicing benchmarks for NWB data and expands ASV result processing.

Changes:

Adds modality selection and incremental slicing parameters.
Extends result normalization and exported slicing metadata.
Documents runtime environment variables and benchmark commands.

File	Description
src/nwb_benchmarks/setup/_reduce_results.py	Normalizes ASV layouts and skipped results.
src/nwb_benchmarks/database/_models.py	Exports slicing template and strategy metadata.
src/nwb_benchmarks/benchmarks/params.py	Defines modality-filtered incremental slicing parameters.
src/nwb_benchmarks/__init__.py	Parses incremental benchmark opt-in settings.
docs/running_benchmarks.rst	Documents environment settings and usage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

Result reduction and database normalization can fail on supported benchmark outputs.

Review effort: Balanced
Findings: 3

Open (3)

Handle cumulative timing dictionaries without network-specific processing · New
Preserve outer sample grouping during parameter reduction · New
Guard against missing trailing fields in tracking rows · New

Resolved since last review (3)

Prefer recorded samples over aggregate results
Preserve parameter-axis shape in legacy reduction
Implement and register incremental slicing benchmarks

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

oruebel · 2026-10-06T23:30:51Z

@CodyCBakerPhD the changes to src/nwb_benchmarks/setup/_reduce_results.py were really just done with AI to fix the issues I ran into, but TBH I am not too familiar with the result parser so it'd be useful if you could take a closer look to make sure the changes there are necessary and correct.

Use sphinx-tabs group tabs (as in the nwb2bids docs) so each environment variable example shows a macOS / Linux and a Windows (PowerShell) tab; selecting a platform once switches every tab group on the page. Also adds PowerShell equivalents for the RUN_* examples and fixes a heading underline that was one character short. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

The incremental slicing benchmarks are a new benchmark family, which AGENTS.md treats as a minor bump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

CodyCBakerPhD · 2026-10-07T04:55:09Z

Two small points:

number = 1 and its comment do nothing for track_ benchmarks. ASV only uses number for time_ benchmarks.
The dicts named incremental_hdf5_*_params are reused for Zarr and LINDI, so the hdf5 in the name is misleading.

`track_cumulative_slice_times` now returns `{"cumulative_time_in_seconds": [...]}` instead of one `cumulative_slice_NNN` key per step. The keys stopped sorting correctly past 999 steps (zero-padded to 3 digits), and each one became its own variable in the database. The list keeps the steps in read order: the first entry is the open time and entry N includes reading the first N slices. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

- `RUN_INCREMENTAL_SLICING_BENCHMARKS` parsing and the modality filter of the incremental slicing parameters. - The incremental slicing helpers and `_track_cumulative_slice_times` on an in-memory NWB file, for both slice strategies. - `_extract_successful_results`, `_serialize_parameter_cases` and `reduce_results` on a raw results file written by asv 0.6.1 with `--record-samples`, holding an incremental, a network and a time benchmark, including a failed and an unselected parameter set; and the database reader on the reduced output. The DANDI URL lookup that importing the benchmark parameters does is stubbed in `conftest.py`, so the tests run offline. A `test` dependency group installs pytest with the database and figure dependencies, and the CI runs the tests before the benchmark smoke test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

CodyCBakerPhD · 2026-10-07T05:15:42Z

Fixed those small points as well as:

The cumulative_slice_{i:03d} key names (Past 999 steps the keys will stop sorting in order as text)
Added unit tests

…rams - `number` only applies to `time_` benchmarks; ASV calls a `track_` benchmark once per sample regardless, so the override on `IncrementalSliceBenchmark` did nothing. - The `incremental_hdf5_*_params` dicts are shared by the HDF5, Zarr and LINDI parameter sets, so they are now `incremental_*_params`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

… reducer `track_cumulative_slice_times` now returns `dict(samples=..., number=None)`, like the network tracking benchmarks. With the `--record-samples` flag `nwb_benchmarks run` always passes, ASV then writes the cumulative times to the samples column, which the existing `reduce_results` reads, so the reducer rewrite is no longer needed: - `_reduce_results.py` is back to its version on `main`, with one fix: a parameter set that failed (`null` samples) no longer drops the successful parameter sets of the same benchmark through the length-mismatch warning. The warning now covers only a structural mismatch between the parameter, result and samples lists. - `parse_parameter_case` no longer special-cases `()`, which only zero-parameter benchmarks produce and the suite has none of. - The guard in `normalize_time_and_network_results` stays: the database reader still treats every dict result as network statistics otherwise. The reducer tests now run `reduce_results` on a raw results file regenerated with asv 0.6.1, holding the wrapped incremental result, a network, a time, and an unwrapped `track_` benchmark. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

…a-rzyanr Wrap incremental slicing results as ASV samples and restore the existing reducer

Follow-up to #203 (targets its branch). The raw results fixture behind `tests/test_reduce_results.py` was written with asv 0.6.1. Main now pins asv 0.6.6 and asv-runner 0.3.1 (#205), so this PR regenerates the fixture with those versions. ## Changes - **Merges `main` into the branch.** It merges cleanly and brings in the #205 pins, so the tests in this PR run against the asv version that wrote the fixture. - **Regenerates the fixture.** It comes from the same toy suite as before: `Incremental` (B raises), `Network`, `Timed` (`--bench` selects only parameter set A) and `Unwrapped`. The suite was run with `asv run --python=same --record-samples` and trimmed to the keys the reducer reads, with the same fixed commit hash. - **Renames the fixture** `tests/data/asv_0.6.1_raw_results.json` → `asv_0.6.6_raw_results.json`, and updates the path and comment in the test. ## What changed in the fixture The structure is the same as under 0.6.1: same `result_columns`, same row lengths (12 for the wrapped benchmarks, 5 for `Unwrapped`), NaN result for the unselected `Timed` B, `null` samples for the failed `Incremental` B, and the same `true` results for the wrapped `track_` benchmarks. Only the `Timed` timing, the benchmark version hashes, `started_at` and `duration` differ. No version bump: nothing under `src/nwb_benchmarks/` changes. ## Testing - `python -m pytest tests`: 40 passed, none skipped (polars and seaborn installed, so the database-reader test ran). - `pre-commit run` on the changed files passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01At337MLAynHQXz3TZBF3Vv --- _Generated by [Claude Code](https://claude.ai/code/session_01At337MLAynHQXz3TZBF3Vv)_ Co-authored-by: Claude <noreply@anthropic.com>

…arams Only the benchmark suite reads it (`params.py` filters the parameter sets by modality, `track_incremental_slicing.py` gates the benchmark), but the parser and its warning ran in the package `__init__`, so on every import of `nwb_benchmarks`: an invalid value broke every CLI command and the warning printed on all of them. They now live in `benchmarks/params.py`, next to the modality filter, and `__init__.py` is back to its version on `main`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

CodyCBakerPhD · 2026-10-07T22:34:23Z

@oruebel I also folded the doc snippets into sphinx-tabs (for cross platform)

Then moved some code from __init__.py into one of the more official modules

If you think it still looks good / does what you want go ahead and merge

oruebel · 2026-10-07T23:55:48Z

If you think it still looks good / does what you want go ahead and merge

Thanks @CodyCBakerPhD for the fixes for the fixes. Looks good to me.

oruebel added 4 commits October 6, 2026 12:40

Add benchmarks for incremetnally slicing a file

3da3fa4

Document evironment variables

c441814

Make it easy to run the incremental slice benchmark for individual mo…

7b7e676

…dalitities via an environment variable

Fix but in result parser

da1cacc

Copilot AI balanced review requested due to automatic review settings October 6, 2026 20:14

Copilot started reviewing on behalf of oruebel October 6, 2026 20:15 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread src/nwb_benchmarks/setup/_reduce_results.py Outdated

Comment thread src/nwb_benchmarks/setup/_reduce_results.py Outdated

Comment thread src/nwb_benchmarks/benchmarks/params.py

oruebel marked this pull request as draft October 6, 2026 20:22

oruebel and others added 2 commits October 6, 2026 13:30

Add missing /track_incremental_slicing.py file

3d67b52

Merge branch 'main' into add_time_slicing_benchmark

76128ee

oruebel requested a balanced review from Copilot October 6, 2026 20:33

Copilot started reviewing on behalf of oruebel October 6, 2026 20:33 View session

oruebel marked this pull request as ready for review October 6, 2026 20:36

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread src/nwb_benchmarks/benchmarks/track_incremental_slicing.py Outdated

Comment thread src/nwb_benchmarks/setup/_reduce_results.py Outdated

Comment thread src/nwb_benchmarks/setup/_reduce_results.py Outdated

oruebel marked this pull request as draft October 6, 2026 20:41

oruebel and others added 3 commits October 6, 2026 15:56

Address review comments

b119932

Address copilot feedback

585822c

Merge branch 'main' into add_time_slicing_benchmark

cbd8af6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

oruebel marked this pull request as ready for review October 6, 2026 23:28

oruebel requested a balanced review from Copilot October 6, 2026 23:28

Copilot AI reviewed Oct 6, 2026

CodyCBakerPhD assigned oruebel Oct 7, 2026

CodyCBakerPhD self-requested a review October 7, 2026 00:58

claude added 2 commits October 7, 2026 04:29

Bump package version to 0.2.0

3b6af3b

The incremental slicing benchmarks are a new benchmark family, which AGENTS.md treats as a minor bump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015K2S7gEkSTfhpnfbXVD5YG

claude added 2 commits October 7, 2026 05:04

claude and others added 3 commits October 7, 2026 05:17

Merge pull request #207 from NeurodataWithoutBorders/claude/bold-volt…

5a0482d

…a-rzyanr Wrap incremental slicing results as ASV samples and restore the existing reducer

CodyCBakerPhD mentioned this pull request Oct 7, 2026

Regenerate the reducer test fixture with asv 0.6.6 #208

Merged

CodyCBakerPhD and others added 4 commits October 7, 2026 11:49

Merge branch 'main' into add_time_slicing_benchmark

2d26a3b

Merge branch 'main' into add_time_slicing_benchmark

6868498

CodyCBakerPhD approved these changes Oct 7, 2026

View reviewed changes

oruebel merged commit 887c784 into main Oct 7, 2026
3 checks passed

oruebel deleted the add_time_slicing_benchmark branch October 7, 2026 23:55

Navigation Menu

Repository navigation

Add time slicing benchmark - #203

Add time slicing benchmark#203
oruebel merged 20 commits into
mainfrom
add_time_slicing_benchmark

oruebel commented Oct 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

oruebel commented Oct 6, 2026

Uh oh!

CodyCBakerPhD commented Oct 7, 2026

Uh oh!

CodyCBakerPhD commented Oct 7, 2026

Uh oh!

CodyCBakerPhD commented Oct 7, 2026

Uh oh!

oruebel commented Oct 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

oruebel commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

oruebel commented Oct 6, 2026

Uh oh!

CodyCBakerPhD commented Oct 7, 2026

Uh oh!

CodyCBakerPhD commented Oct 7, 2026

Uh oh!

CodyCBakerPhD commented Oct 7, 2026

Uh oh!

oruebel commented Oct 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

oruebel commented Oct 6, 2026 •

edited

Loading