Skip to content

Round before casting preprocessed traces to integer dtypes#4653

Open
CodeBlackwell wants to merge 1 commit into
SpikeInterface:mainfrom
CodeBlackwell:fix-integer-dtype-rounding
Open

Round before casting preprocessed traces to integer dtypes#4653
CodeBlackwell wants to merge 1 commit into
SpikeInterface:mainfrom
CodeBlackwell:fix-integer-dtype-rounding

Conversation

@CodeBlackwell

Copy link
Copy Markdown

Several preprocessors cast their float result straight to an integer dtype with astype(), which truncates toward zero instead of rounding. Over many samples that biases the output (toward zero for centered data, downward otherwise). filter.py already rounds before the same cast — this brings whiten, common_reference and highpass_spatial_filter in line with it. It also drops the offset.astype(dtype) cast in ScaleRecording, which was silently zeroing fractional offsets for integer dtypes.

Added tests covering the integer-rounding paths.

Noticed while looking at #1972; this addresses the integer-cast rounding, not the zscore/int_scale std question in that issue.

…er dtype

whiten, common_reference and highpass_spatial_filter cast their float
result straight to an integer dtype with astype(), which truncates toward
zero instead of rounding and biases the output; filter.py already rounds
before the same cast. Round on the integer-dtype path in those three, and
stop casting the ScaleRecording offset to the integer dtype (which zeroed
out fractional offsets). Adds tests covering the integer-rounding paths.
@CodeBlackwell CodeBlackwell marked this pull request as ready for review July 7, 2026 19:13
@alejoe91 alejoe91 added the preprocessing Related to preprocessing module label Jul 8, 2026
@zm711

zm711 commented Jul 8, 2026

Copy link
Copy Markdown
Member

We've been discussing this a bit and two questions:

  1. we aren't sure if we see much value in the tests that were added. We trust the implementation of round vs int from numpy and python. At minimum we would remove the monkey patching test.
  2. Could you give us some profiling of this change. We've actually had a discussion about rounding in the past, but the issue is that np.round needs to allocate at least an equalize sized array and we are concerned that even with our chunking mechanism that the memory costs may really explode as recording files are between 10GB-1TB depending on when the memory is freed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preprocessing Related to preprocessing module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants