Skip to content

fix(vital_signs): use circular variance for wrapped phases#595

Merged
ruvnet merged 1 commit into
ruvnet:mainfrom
akhilesharora:fix/vital-signs-circular-phase-variance
May 17, 2026
Merged

fix(vital_signs): use circular variance for wrapped phases#595
ruvnet merged 1 commit into
ruvnet:mainfrom
akhilesharora:fix/vital-signs-circular-phase-variance

Conversation

@akhilesharora

Copy link
Copy Markdown
Contributor

The heartbeat feature in process_frame was computed as arithmetic mean + arithmetic variance on phases that come from atan2() and are wrapped to (-pi, pi]. Two phases that straddle +/-pi are physically aligned but the linear formula treats them as ~2*pi apart, so you get ~pi^2 (~9.87) instead of ~1e-6. That noise rides into the heart-rate FFT buffer every time a subcarrier's phase crosses the wrap.

Swapped the inline math for a standard circular variance (1 - R, where R is the mean resultant length). The value is bounded [0, 1] instead of [0, ~pi^2] now, but the heartbeat FFT cares about relative spectral peaks, not absolute scale.

Tests:

cargo test -p wifi-densepose-sensing-server --no-default-features --lib vital_signs
test result: ok. 15 passed; 0 failed

4 new tests: one drives process_frame end-to-end and inspects the heartbeat_buffer; the other three hit the helper with wraparound, opposite, and identical inputs.

Related to the jumpy vitals in #519 and the +/-15 bpm jitter mentioned in #485. #485 wraps VitalSignDetector with an external pipeline but keeps it as the low-confidence fallback - this fix is complementary.

Closes #593

process_frame computed arithmetic mean + variance on phase values from
atan2(), which are wrapped to (-pi, pi]. Phases close across the +/-pi
discontinuity produced ~pi^2 variance instead of ~1e-6, feeding wrap
noise into the heart-rate FFT buffer.

Replace inline math with a standard circular variance helper
(1 - mean resultant length). Add 4 unit tests, one through the
production path of process_frame.

Closes ruvnet#593

@ruvnet ruvnet 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.

Reviewed end-to-end:

  • Math is correct: 1 - R where R = sqrt(sin_sum^2 + cos_sum^2) / n is the standard Mardia circular variance.
  • Bounded to [0, 1] instead of [0, ~pi^2], which is fine because the heartbeat FFT cares about relative spectral peaks (confirmed in the PR description).
  • The regression test test_phase_variance_handles_wraparound is particularly nice — it asserts both the new bound AND that the old buggy formula on the same input produces ~pi^2, which makes a regression to the linear computation explicitly fail.
  • test_process_frame_handles_wrapped_phases is the end-to-end check that anyone can run against main to observe the bug and against this branch to observe the fix. Excellent reproducer.
  • Identical-phase and opposite-phase edge cases covered.
  • Scoped to a single file. Well-commented. Closes #593.

Approving. Merging with squash.

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.

vital_signs.rs: linear stats on wrapped (atan2) phase values

2 participants