Skip to content

Add optional C preprocessing extension for deterministic FlowData transforms - #41

Draft
whitews with Copilot wants to merge 3 commits into
developfrom
copilot/implement-c-extension-preprocessing
Draft

whitews with Copilot wants to merge 3 commits into
developfrom
copilot/implement-c-extension-preprocessing

Conversation

Copilot AI commented Sep 19, 2026 •

Copy link
Copy Markdown

FlowData preprocessing was still using NumPy channel-slice expressions for log/gain conversion, which made exact-array fixture tests sensitive to environment-dependent floating-point behavior. This change moves preprocessing behind a shared implementation with an optional C extension, while preserving the existing API and the Python/NumPy fallback path.

  • Design

    • Added flowio._preprocessing_ext, a C extension that processes a contiguous float64 2-D event array in a single row/column traversal using per-channel decades, log0, range, and gain arrays.
    • Added flowio._preprocessing as the shared wrapper:
      • prefers the extension when available
      • falls back to the existing NumPy behavior when the extension is unavailable
    • FlowData.as_array() now routes through _apply_preprocessing() so preprocessing logic is centralized and reusable.
  • Behavior preserved

    • Timestep scaling still happens before channel preprocessing.
    • Per-channel semantics are unchanged:
      • decades > 0: pow(10.0, decades[col] * value / range[col]) * log0[col]
      • gain correction: divide after log conversion when gain != 1.0 and gain != 0.0
      • decades == 0 channels keep their value except for gain correction
    • Output remains float64.
    • Public API stays unchanged; as_array() and write_fcs() continue to use the same preprocessing path.
  • Packaging / install behavior

    • Integrated extension builds with setuptools using the existing pyproject.toml build backend.
    • Added NumPy as a build requirement for source/editable installs.
    • Added setup.py for extension configuration without requiring users to hand-write one.
    • Extension build is optional: source checkouts and environments without a working compiler still import and run via the fallback.
  • Reproducibility scope

    • Applied conservative FP compiler settings to the extension only:
      • GCC/Clang: -ffp-contract=off, -fno-fast-math, and -mno-fma on x86/x86_64
      • MSVC: /fp:strict
    • This reduces FMA/contraction/reassociation variability in FlowIO’s preprocessing path, but does not claim bit-identical transcendental pow() results across every OS, libm, compiler, or architecture.
  • Tests / utilities

    • Added focused preprocessing tests for:
      • extension vs NumPy fallback parity
      • per-channel decades/log0/range/gain handling
      • timestep behavior through the shared preprocessing path
      • clean skipping when the optional extension is unavailable
    • Kept the existing exact fixture assertions unchanged.
    • Added a small local benchmark utility for comparing fallback vs extension performance without introducing CI timing flakiness.
# shared preprocessing path
tmp_events = self._apply_preprocessing(tmp_events)

# extension kernel semantics
if (decades[col] > 0.0) {
    value = pow(10.0, decades[col] * value / ranges[col]) * log0[col];
}
if (gains[col] != 1.0 && gains[col] != 0.0) {
    value = value / gains[col];
}

Co-authored-by: whitews <1298022+whitews@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement C extension for FlowIO preprocessing math Add optional C preprocessing extension for deterministic FlowData transforms Sep 19, 2026
Copilot AI requested a review from whitews September 19, 2026 03:01
@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 63.76812% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.79%. Comparing base (05232ba) to head (54b0719).
⚠️ Report is 46 commits behind head on develop.

Files with missing lines Patch % Lines
src/flowio/_preprocessing.py 45.45% 24 Missing ⚠️
src/flowio/flowdata.py 96.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #41      +/-   ##
===========================================
- Coverage    93.00%   89.79%   -3.21%     
===========================================
  Files            7        8       +1     
  Lines          486      549      +63     
===========================================
+ Hits           452      493      +41     
- Misses          34       56      +22     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@whitews

whitews commented Sep 19, 2026

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

…c-extension-preprocessing

# Conflicts:
#	src/flowio/flowdata.py

Co-authored-by: whitews <1298022+whitews@users.noreply.github.com>

Copilot AI commented Sep 19, 2026

Copy link
Copy Markdown
Author

@copilot resolve the merge conflicts in this pull request

Resolved and merged develop into this branch in c76f628.

@whitews whitews left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Let's separate out the preprocessing benchmark as an initial commit so we get a baseline of the current performance. Can you make a separate PR for the benchmark off of the current develop branch?

Copilot AI commented Sep 19, 2026

Copy link
Copy Markdown
Author

@whitews I've opened a new pull request, #42, to work on those changes. Once the pull request is ready, I'll request review from you.

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