Skip to content

fix: don't slow EMA smoother across zero-crossings for high alpha#373

Merged
tomquist merged 1 commit into
developfrom
claude/review-issue-371-RsR3W
May 18, 2026
Merged

fix: don't slow EMA smoother across zero-crossings for high alpha#373
tomquist merged 1 commit into
developfrom
claude/review-issue-371-RsR3W

Conversation

@tomquist
Copy link
Copy Markdown
Owner

@tomquist tomquist commented May 18, 2026

Summary

Fixes #371.

The sign-flip "catchup" branch in SmoothedPowermeter (src/astrameter/powermeter/wrappers/smoothing.py:75) was intended to react faster when the raw total crosses zero by boosting the effective alpha to alpha * 4, capped at 0.5:

catchup_alpha = self._alpha
if (raw_total > 0) != (self._value > 0):
    catchup_alpha = min(0.5, self._alpha * 4)

But min(0.5, alpha * 4) is only a boost while alpha < 0.125. For any larger configured alpha the result simply pins to 0.5, and once alpha > 0.5 the branch actively reduces the step instead of accelerating it.

Reported symptom: running close to full self-consumption with SMOOTH_TARGET_ALPHA = 1.0 (instant tracking) randomly halves the delta whenever the raw reading happens to straddle zero — so a 1 → 101 step moves the smoothed value by 100, while -1 → 99 moves it by only 50, often dragging the smoothed value back inside DEADBAND and snapping it to 0.

Fix

Raise the catchup alpha to at least the configured alpha, so the branch can only ever speed the smoother up, never slow it down:

catchup_alpha = max(self._alpha, min(0.5, self._alpha * 4))
  • For small alpha (e.g. 0.1 → 0.4) the existing behaviour is preserved — that case still has a dedicated test (test_sign_change_catchup).
  • For alpha ≥ 0.5 the branch becomes a no-op instead of a slowdown.

Test plan

  • Existing test_sign_change_catchup still passes (alpha=0.1, expected delta unchanged).
  • New test_sign_change_does_not_slow_high_alpha covers alpha=1.0 and asserts the sign-flip step is no longer halved.
  • uv run ruff format .
  • uv run ruff check .
  • uv run mypy src/
  • uv run pytest — 674 passed, 27 skipped (mosquitto-only tests).

Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed EMA smoothing responsiveness when SMOOTH_TARGET_ALPHA is configured above 0.5. The catchup coefficient logic now correctly applies the configured alpha value without artificial capping, restoring expected behavior for high smoothing settings (issue #371).
  • Tests

    • Added and updated smoothing tests to verify correct behavior with various alpha configurations.

Review Change Stack

The sign-flip "catchup" branch in SmoothedPowermeter was meant to react
faster when raw power crosses zero by boosting the effective alpha to
`alpha * 4`, capped at 0.5. But `min(0.5, alpha * 4)` is only a boost
while `alpha < 0.125`; for any larger configured alpha it just clamps to
0.5, and for alpha > 0.5 it actively reduces the step.

Users running near self-consumption with a high `SMOOTH_TARGET_ALPHA`
(e.g. 1.0 for instant tracking) therefore saw randomly halved deltas
whenever the raw reading happened to straddle zero, often dragging the
smoothed value back into the deadband and snapping it to 0.

Raise the catchup alpha to at least the configured alpha so the branch
can only ever speed the smoother up, never slow it down.

Refs #371
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e4f4d250-e2bb-4e73-9669-962ac5e2f3f1

📥 Commits

Reviewing files that changed from the base of the PR and between 700f7e1 and 109dcd0.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/astrameter/powermeter/wrappers/smoothing.py
  • src/astrameter/powermeter/wrappers/smoothing_test.py

Walkthrough

This PR fixes EMA smoothing responsiveness when crossing zero in power meter readings. The catchup logic now respects high SMOOTH_TARGET_ALPHA values instead of capping them at 0.5, ensuring consistent responsiveness regardless of sign changes.

Changes

EMA Smoothing Catchup Responsiveness

Layer / File(s) Summary
Catchup alpha clamping fix
src/astrameter/powermeter/wrappers/smoothing.py, src/astrameter/powermeter/wrappers/smoothing_test.py, CHANGELOG.md
catchup_alpha calculation now clamps with max(self._alpha, min(0.5, self._alpha * 4)) to prevent the configured alpha from being undercut during sign-flip catch-up. Existing test expectations are updated to reflect this change, a new test verifies high-alpha responsiveness (alpha=1.0) does not slow during sign changes, and the fix is documented in the changelog.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing EMA smoothing behavior for high alpha values during zero-crossings, which is the core issue addressed in the PR.
Linked Issues check ✅ Passed The PR fully addresses issue #371 by changing the catchup alpha logic to use max(self._alpha, min(0.5, self._alpha * 4)), ensuring sign-flip handling never reduces effective alpha, directly resolving the reported inconsistent smoothing around zero.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #371: the CHANGELOG entry documents the fix, the core logic change implements the solution, and tests validate both the existing behavior and the new fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/review-issue-371-RsR3W

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tomquist tomquist merged commit 8bb3e97 into develop May 18, 2026
13 checks passed
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