perf(float32): make the data path float32 at its source - #344
Merged
Conversation
Phase 1 of moving minian off float64. Two source-of-truth changes; the rest of the pipeline infers dtype from its inputs and follows along. - io.load_videos default dtype np.float64 -> np.float32. The footage is 8-bit, so every loaded value is represented exactly in float32; this just stops inflating it 8x in memory and on disk. Callers that pass dtype explicitly (including the demo notebook and the tests) are unaffected. - pipeline notebook: Y.astype(float) -> Y.astype(np.float32) at the Y_fm_chk save, the single spot where the demo movie was promoted to float64. This is the ~69 GB-vs-34 GB cube behind the slow save/rechunk; everything downstream (Y_fm_chk, Y_hw_chk, seed init, CNMF prep) is now float32. All other notebook casts were already float32. Scope is deliberately limited to the data source. The CNMF gufunc output_dtypes (cnmf.py) still emit float64, and sklearn/cvxpy upcast their solves internally, so numeric results move only as much as a float32 input warrants. The pipeline notebook test (test_pipeline_notebook) validates that cell count (286 +/-10) and the C/S/A sums stay within their tolerances. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
daharoni
marked this pull request as draft
June 26, 2026 08:22
daharoni
marked this pull request as ready for review
June 26, 2026 08:41
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 1 of moving minian off
float64: switch the data path tofloat32at its source. Because dask/xarray/numpy infer output dtype from their inputs, flipping the source is enough for the bulk of the pipeline to follow.Two changes:
io.load_videosdefaultnp.float64->np.float32. The footage is 8-bit, so every loaded value is represented exactly infloat32(no precision loss at load), while halving memory and on-disk size. Callers that passdtypeexplicitly - including the demo notebook (uint8) and every test - are unaffected.Y.astype(float)->Y.astype(np.float32)at theY_fm_chksave. This was the single spot promoting the demo movie tofloat64- the ~69 GB-vs-34 GB cube behind the slow save/rechunk in feat(pipeline-nb): expose rechunker mem_limit as a tunable parameter #342. Everything downstream (Y_fm_chk,Y_hw_chk, seed init, CNMF prep) is nowfloat32; all other notebook casts were alreadyfloat32.Scope (deliberately limited)
This PR only changes the data source. The CNMF gufunc
output_dtypesincnmf.pystill emitfloat64, and sklearnLassoLars/ cvxpy solvers upcast internally, so numeric results shift only as much as afloat32input warrants. Tightening those (gufuncoutput_dtypes, internalnp.zerosdtypes, double accumulators) is a validated follow-up (Phase 2).Validation
test_pipeline_notebookruns this notebook end-to-end in CI and checks cell count (286 ±10) and the C/S/A sums (rel 5e-2 / 1e-1 / 5e-2) - tolerances built for exactly this kind of numeric drift. That's the gate for the notebook change. Theiodefault change is covered bytest_pre_processing(which passesdtypeexplicitly, so behavior is unchanged there).Note
Targets
master, which is the v2 development branch, so theload_videosdefault dtype change (a public-API default change suited to a major version) is appropriate here. It's also backward-compatible for 8-bit-sourced data, since those values are exact in float32.🤖 Generated with Claude Code