Add activity index calculation from accelerometer data (fixes #642) - #859
Add activity index calculation from accelerometer data (fixes #642)#859samerzumot wants to merge 3 commits into
Conversation
…ow-dev#642) - Implement get_activity_index in C++ core using multi-axis epoch variance - Add get_activity_index bindings across C++, Python, Java, C#, TypeScript, Rust, Swift, Julia, and MATLAB - Add automated test activity_index.py
|
Thanks for working on this. I found several blockers that should be addressed before merge:
Confirmed Rust CI failure: https://github.com/brainflow-dev/brainflow/actions/runs/32657395247/job/100051356398 References:
|
…iod validation, and baseline noise variance support
|
Thanks for the detailed review and guidance, @Andrey1994! I have addressed all 5 blockers: 1. Rust Compilation Fix
2. Complete Bai et al. (2016) / BIOBSS Formula
3. Scope/API design and Issue #642Between rescoping this as a variability helper or implementing the complete AI contract, **I chose to implement the complete AI contract in Here is my reasoning (Let me know what you think):
4. Input Validation Consistency & Safety
5. CI Workflow & Test Suite
|
Andrey1994
left a comment
There was a problem hiding this comment.
[P1] The Node.js package does not compile.
functions.types.ts:450–451 declares getActivityIndex with six arguments, but the updated call passes ten. The declaration is missing samplingRate and the three noise variance parameters. Both Ubuntu and macOS CI fail with TS2554: Expected 6 arguments, but got 10, so the new Python test is subsequently skipped. Please update the declaration to match the native signature. CI failure.
[P2] Invalid samples silently become zero activity.
In data_handler.cpp:1784–1785, std::max(0.0, NaN) returns zero. I reproduced this: a valid signal produces AI ≈ 0.57735, but replacing one sample with NaN produces AI = 0.0 with a success status. Passing noise_var_x=NaN has the same effect. This makes invalid data indistinguishable from inactivity. Please reject non-finite inputs or explicitly preserve an invalid-result indication.
| from brainflow.exit_codes import BrainFlowError, BrainFlowExitCodes | ||
|
|
||
|
|
||
| def test_activity_index(): |
There was a problem hiding this comment.
make the tests easier and smaller, its too big now, also tests here are meant to be more like examples for docs, keep them simple, check for example tests for signal filtering
- update Node.js DataHandlerFunctions.getActivityIndex signature to match 10 parameters - add std::isfinite checks to reject NaN/Inf in accelerometer samples and noise variances - simplify activity_index.py to a concise BoardShim demonstration test - remove leftover PNG artifacts from repository root - add Activity Index test step to Windows CI workflow
|
Thanks for the review @Andrey1994! I've addressed your feedback with the following changes:
|
Andrey1994
left a comment
There was a problem hiding this comment.
The previous TypeScript signature and non-finite-input bugs are fixed, and Unix/Windows CI now pass. Two blockers remain:
- The rewritten
activity_index.pyis only a smoke/demo script and contains no assertions. It no longer tests noise subtraction, known numerical output, aggregation semantics, or invalid arguments, despite those being the core behavior added by this PR. Please keep the example small, but add focused regression coverage in the appropriate test suite (or a few compact assertions here). - Clang Format is currently failing in
src/data_handler/data_handler.cpp.
The API example also currently describes a different output granularity from what its call produces.
| accel_channels = BoardShim.get_accel_channels(board_id) | ||
| print(f'Accel channels: {accel_channels}, Sampling rate: {sampling_rate}') | ||
|
|
||
| # demo activity index calculation per 1-second epoch |
There was a problem hiding this comment.
[P2] This says “per 1-second epoch,” but omitting period sets it to the full integer-second length of the recording, so this call returns one value containing the sum of all 1-second AIs. Either pass period=sampling_rate to produce per-second outputs, or describe this as an aggregate over the complete recording.
get_activity_indexin C++ core using multi-axis epoch varianceget_activity_indexbindings across C++, Python, Java, C#, TypeScript, Rust, Swift, Julia, and MATLABactivity_index.py