Skip to content

Add channel rereferencing across language bindings - #847

Draft
Daniiiil1 wants to merge 1 commit into
brainflow-dev:masterfrom
Daniiiil1:codex/brainflow-rereference-channels
Draft

Add channel rereferencing across language bindings#847
Daniiiil1 wants to merge 1 commit into
brainflow-dev:masterfrom
Daniiiil1:codex/brainflow-rereference-channels

Conversation

@Daniiiil1

Copy link
Copy Markdown

Summary

  • add a reference operation to the C++, C#, Java, Julia, MATLAB, Node.js, Python, Rust, and Swift DataFilter APIs
  • compute the sample-wise mean of selected reference channels before mutating data, so overlapping reference and target channel lists are safe
  • validate empty matrices, empty channel lists, rectangular inputs where applicable, and out-of-range indices
  • demonstrate the operation in each language's existing downsampling example; the R example uses the Python binding as the R package already does
  • add Julia, Rust, and Swift unit coverage, including overlapping channel lists and invalid arguments

Motivation

EEG pipelines commonly need to re-reference multiple channels against the mean of one or more reference electrodes. BrainFlow currently requires users to implement that operation themselves in every language. This adds one consistent operation following the API proposed in #619.

For input matrix data, the implementation first computes

reference_signal[t] = mean(data[channel, t] for channel in reference_channels)

and then applies

data[channel, t] -= reference_signal[t]

to every channel in channels_to_reference. The reference signal is computed completely before any target row is changed.

Closes #619.

Validation

  • Python: 640 randomized matrix/channel configurations matched an independent NumPy calculation at rtol=1e-13; invalid lists and indices were also checked
  • Node.js: npm run build passed; direct runtime behavior and invalid-argument checks passed
  • C#: complete brainflow.sln Release build passed with 0 warnings and 0 errors
  • Swift: the BrainFlow target and swift-downsampling product both build successfully
  • repository diff check: git diff --check passed

The draft PR lets the full BrainFlow CI validate the remaining platform-specific C++, Java, Julia, MATLAB, R, Rust, and Swift test paths.

@Andrey1994 Andrey1994 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing this across all supported bindings. The core algorithm looks correct, in particular computing the complete reference signal before mutating the target channels, which makes overlapping target/reference lists safe.

There are a few things I think we should address before taking this out of draft:

  1. Duplicate channel indices currently have inconsistent semantics across bindings. Most implementations subtract the reference repeatedly when channels_to_reference contains the same channel more than once, while MATLAB's indexed assignment appears to update that row only once. Duplicates in reference_channels also implicitly give that channel additional weight. Please either reject duplicates in both lists or explicitly define and test identical behavior in every binding. I would prefer rejecting them.

  2. Regression coverage is too limited for nine independent implementations. At present, dedicated tests are added only for Julia, Rust, and Swift; the C++ implementation itself is not covered. Please add authoritative C++ coverage for the calculation, overlapping lists, empty/out-of-range arguments, and the chosen duplicate semantics. The independently implemented bindings should also have at least smoke/parity coverage—the modified downsampling examples do not verify the result.

  3. The branch needs to be updated and CI rerun. GitHub currently reports the PR as non-mergeable, and the Valgrind and CppCheck runs were cancelled. We should have an up-to-date, complete CI result before marking it ready.

Non-blocking: I would prefer a separate rereferencing example instead of changing every downsampling example. Combining unrelated operations makes those examples less focused. It would also be useful to document explicitly that MATLAB returns a referenced copy while the other APIs mutate their input.

The overall API direction matches #619, and the overlap handling itself looks good.

Andrey1994 commented Aug 18, 2026

Copy link
Copy Markdown
Member

Update: the workflow hardening has been moved into a separate draft PR built directly on the current master: #857. No CI workflow changes need to be added to this feature branch.

The two cancelled checks are infrastructure failures, not failures in the rereferencing code:

  • CppCheck run 30890111327 and Valgrind run 30890111514 both stalled in apt-get update against azure.archive.ubuntu.com and were cancelled at GitHub's six-hour job limit.
  • Both already used the current stable runner: ubuntu-latest resolved to Ubuntu 24.04.4, image 20260810.271.1. Changing to ubuntu-24.04 would select the same image; ubuntu-26.04 is still preview.
  • Identical CppCheck and Valgrind workflows passed on the identical 24.04 image for other recent PRs, confirming a transient Azure mirror problem.

Both cancelled jobs have been re-run (attempt 3) and are currently queued. PR #857 addresses recurrence by using the official fallback mirrors, bounding package-install time, consolidating Valgrind's APT setup, and updating the Node-based actions to v7.

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.

add method to rereference channels

2 participants