Skip to content

Register bwm_lfp as an official, opt-in agent dataset - #26

Merged
oliche merged 5 commits into
mainfrom
feat/issue-12-register-bwm-lfp-dataset
Aug 16, 2026
Merged

Register bwm_lfp as an official, opt-in agent dataset#26
oliche merged 5 commits into
mainfrom
feat/issue-12-register-bwm-lfp-dataset

Conversation

@oliche

@oliche oliche commented Jul 25, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds an opt-in bwm_lfp dataset (standard-compression tier only): scripts/download_datasets.py --lfp downloads, sha1-verifies, and registers lf_compressed_all_bwm.h5, authoring schema.yaml/provenance.yaml/manifest.json so it resolves through the same data_locations/schema-based registry as bwm_ephys/bwm_behavior.
  • lfpack is an optional lfp extra, not a core dependency; skills (ibl-neuropixel, ibl-load) tell the agent to offer uv sync --extra lfp on demand rather than assume it's installed.
  • Adds docs/bwm/lfp.md, a scripts/validate_bwm_lfp_release.py release validator, and CHANGELOG.md/CHANGELOG_DATA.md entries.

Test plan

  • uv run ruff check .
  • uv run pytest -q (one pre-existing, unrelated failure confirmed present on main too — an environment quirk from local data_locations.local.yaml, not touched by this change)
  • Manually exercised download_lfp_file/_write_lfp_sidecars and validate_bwm_lfp_release.py against a faked dataset dir
  • Run the real --lfp download (~14 GB) and validator against the actual archive before merging

Closes/references #12.

oliche and others added 3 commits July 25, 2026 17:10
Adds standard-compression-tier-only LFP dataset support: a `--lfp` flag on
scripts/download_datasets.py that downloads, sha1-verifies, and registers
lf_compressed_all_bwm.h5, authoring schema.yaml/provenance.yaml/manifest.json
so it plugs into the same data_locations/schema.yaml dataset registry as
bwm_ephys/bwm_behavior. lfpack is an optional `lfp` extra rather than a core
dependency. Includes docs, skill routing for LFPackReader, a release
validator, and tests.

References #12.
ruff 0.16.0 changed its default rule set, surfacing ~368 pre-existing
violations across unrelated files and breaking CI; cap the dev extra until
those are cleaned up separately.

Also add tool.uv.required-environments so the resolver can produce a
universal lock across macOS arm64, Linux x86_64 (CI), and Windows without
choking on the win32-only PyQt5-Qt5 constraint.
The SHA1 hash in the original commit was incorrect. The actual file on S3
has hash b84edd4b98602bec96279c3a9e42170c65dd48a6, not
2aa88d3ba52cabc89ccd345b846eecaa02cfebc6.

Generated by Mistral Vibe.
Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
@oliche
oliche marked this pull request as ready for review July 26, 2026 06:43
@oliche
oliche requested a review from rossant July 26, 2026 06:43
@rossant

rossant commented Jul 30, 2026

Copy link
Copy Markdown
Member

@oliche I reviewed this and pushed commit 560f471 to address two merge-readiness edge cases:

  • reuse an already verified LFP file and regenerate its sidecars, avoiding an unnecessary ~15.5 GB re-download after an interrupted/partial registration;
  • make the release validator reject empty or structurally invalid sidecar mappings instead of skipping their checks.

I added regression tests for both paths. Local validation is clean: ruff check . passes and pytest -q reports 96 passed. Could you review the follow-up commit and confirm it matches the intended download/release workflow?

@rossant

rossant commented Jul 30, 2026

Copy link
Copy Markdown
Member

@oliche I completed a direct audit of the published bwm_lfp 1.0.0 object and pushed follow-up commit e1aa10d for your review.

Dataset evidence:

  • exact release object verified: 15,546,004,168 bytes, SHA1 b84edd4b98602bec96279c3a9e42170c65dd48a6
  • all 699 recordings open successfully
  • locked signal audit: 24 held-out PIDs x 3 chunks (72 chunks), with valid shapes/dtypes, finite samples/PSDs, and max bin_channels=4 error 9.31e-10 V
  • actual channel distribution is 695 recordings with 384 channels plus four NP2.4 recordings with 96 channels, rather than 384 for every recording
  • the seven missing-sync recordings expose recording-relative sr.times with sr.t0 = NaN, so those times cannot be treated as session clock
  • saturation muting occurs before lossy compression; marked decompressed samples are not necessarily exact zeros. Saturation exceeds 1% in 121 recordings and 10% in 16 (maximum 23.9%)
  • three saturation stop boundaries extend only 0.4-3.2 ms beyond the last decimated timestamp due to outward rounding; saturation_mask clips safely

The commit corrects the schema/docs/runtime guidance and extends release validation to measure and enforce {96: 4, 384: 695} from the HDF5 itself. The enhanced validator passes 21 checks on the real object; the isolated full test suite passes 97 tests. Please review whether the NP2.4 and saturation wording matches your intended upstream semantics.

@oliche

oliche commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Thanks, LGTM !

I linked the commit to int-brain-lab/lfpack#8, as I'll need to update both the dataset, the SHA1 and this documentation when I'll fix the synchronization for the 7 datasets !

@rossant

rossant commented Jul 31, 2026

Copy link
Copy Markdown
Member

Got it. In any case, great work! Very exciting that we could get the LFP under 15 GB!

@oliche
oliche merged commit 9a0d2f7 into main Aug 16, 2026
1 check passed
@oliche
oliche deleted the feat/issue-12-register-bwm-lfp-dataset branch August 16, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants