Skip to content

Commit 1e3730f

Browse files
tomquistclaude
andauthored
Fix false saturation detection during load sign reversal (#301)
* Fix false saturation detection during load sign reversal When the grid target flips sign (e.g. solar excess causes discharge→charge), batteries ramping through zero were falsely detected as saturated because abs(actual) < min_target during the reversal. This caused ping-pong rotation between batteries. The fix treats sign disagreement between target and actual as a direction reversal (not saturation) and decays the score instead of accumulating it. https://claude.ai/code/session_01TowsqcPQ2jQMqo7PXDCYSm * Remove changelog entry for saturation fix https://claude.ai/code/session_01TowsqcPQ2jQMqo7PXDCYSm * Add unit tests for sign-reversal guard boundary cases - actual=0 does not trigger the guard (no sign to disagree) - target=0 does not trigger the guard (handled by low-target decay) - After reversal ends (actual crosses zero), accumulation resumes https://claude.ai/code/session_01TowsqcPQ2jQMqo7PXDCYSm --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1c74d33 commit 1e3730f

3 files changed

Lines changed: 157 additions & 1 deletion

File tree

src/b2500_meter/ct002/balancer.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,15 @@ def update(
173173
else:
174174
state.saturation_grace_until = 0.0
175175
state.saturation_grace_started_at = 0.0
176-
if target_abs < self._min_target:
176+
# Detect sign reversal: target says one direction, actual is still
177+
# in the opposite direction. The battery is healthy but ramping to
178+
# the new direction — not saturated. Treat like low-target (decay).
179+
target_sign = 1 if last_target > 0 else (-1 if last_target < 0 else 0)
180+
actual_sign = 1 if actual > 0 else (-1 if actual < 0 else 0)
181+
sign_reversing = (
182+
target_sign != 0 and actual_sign != 0 and target_sign != actual_sign
183+
)
184+
if target_abs < self._min_target or sign_reversing:
177185
prev = state.saturation_score
178186
if prev > 0:
179187
decayed = prev * self._decay_factor

tests/test_balancer.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,98 @@ def test_get(self):
159159
state = BalancerConsumerState(saturation_score=0.7)
160160
assert tracker.get(state) == 0.7
161161

162+
def test_sign_reversal_decays_instead_of_accumulating(self):
163+
"""When target and actual have opposite signs (direction reversal),
164+
saturation should decay rather than accumulate."""
165+
tracker = self._make_tracker(alpha=0.15, min_target=20, decay_factor=0.995)
166+
state = BalancerConsumerState()
167+
168+
# Battery discharging at 100W, tracking well
169+
for _ in range(5):
170+
tracker.update(state, last_target=100.0, actual=95.0)
171+
assert state.saturation_score < 0.01
172+
173+
# Target flips to charge (-100W), but actual is still +80W (ramping)
174+
for _ in range(20):
175+
tracker.update(state, last_target=-100.0, actual=80.0)
176+
# Score must NOT have accumulated — should have stayed near zero
177+
assert state.saturation_score < 0.01, (
178+
f"Saturation score {state.saturation_score:.3f} should not "
179+
f"accumulate during sign reversal"
180+
)
181+
182+
def test_sign_reversal_existing_score_decays(self):
183+
"""Pre-existing saturation score decays during sign reversal."""
184+
tracker = self._make_tracker(decay_factor=0.9)
185+
state = BalancerConsumerState(saturation_score=0.5)
186+
187+
# Target positive, actual negative (battery reversing)
188+
for _ in range(10):
189+
tracker.update(state, last_target=100.0, actual=-50.0)
190+
assert state.saturation_score < 0.5, (
191+
f"Score should decay during sign reversal, got {state.saturation_score:.3f}"
192+
)
193+
194+
def test_same_sign_low_output_still_accumulates(self):
195+
"""When signs agree but output is below min_target, saturation
196+
should still accumulate (genuine saturation)."""
197+
tracker = self._make_tracker(alpha=0.15, min_target=20)
198+
state = BalancerConsumerState()
199+
200+
# Target +100W but actual only +5W (same sign, truly saturated)
201+
for _ in range(20):
202+
tracker.update(state, last_target=100.0, actual=5.0)
203+
assert state.saturation_score > 0.4, (
204+
f"Saturation score {state.saturation_score:.3f} should accumulate "
205+
f"when signs agree but output is low"
206+
)
207+
208+
def test_actual_zero_during_reversal_does_not_trigger_guard(self):
209+
"""actual=0 should NOT activate the sign-reversal guard (actual
210+
has no sign), so saturation accumulates normally."""
211+
tracker = self._make_tracker(alpha=0.15, min_target=20)
212+
state = BalancerConsumerState()
213+
214+
# Target is -100W (charge) but actual is exactly 0
215+
for _ in range(20):
216+
tracker.update(state, last_target=-100.0, actual=0.0)
217+
assert state.saturation_score > 0.4, (
218+
f"Saturation score {state.saturation_score:.3f} should accumulate "
219+
f"when actual is zero (no sign to disagree)"
220+
)
221+
222+
def test_target_zero_does_not_trigger_guard(self):
223+
"""target=0 should NOT activate the sign-reversal guard."""
224+
tracker = self._make_tracker(alpha=0.15, min_target=20, decay_factor=0.995)
225+
state = BalancerConsumerState(saturation_score=0.5)
226+
227+
# Target is 0, actual is positive — target_abs < min_target so
228+
# the low-target decay path handles this, not the sign guard.
229+
for _ in range(10):
230+
tracker.update(state, last_target=0.0, actual=50.0)
231+
# Score should have decayed via the low-target path
232+
assert state.saturation_score < 0.5
233+
234+
def test_sign_reversal_then_same_sign_resumes_accumulation(self):
235+
"""After a sign reversal period, once actual crosses zero to match
236+
target sign, saturation tracking resumes normally."""
237+
tracker = self._make_tracker(alpha=0.15, min_target=20)
238+
state = BalancerConsumerState()
239+
240+
# Phase 1: sign reversal — target negative, actual positive
241+
for _ in range(10):
242+
tracker.update(state, last_target=-100.0, actual=50.0)
243+
score_after_reversal = state.saturation_score
244+
assert score_after_reversal < 0.01
245+
246+
# Phase 2: actual crosses zero but is small (same sign, low output)
247+
for _ in range(20):
248+
tracker.update(state, last_target=-100.0, actual=-5.0)
249+
assert state.saturation_score > 0.4, (
250+
f"Saturation score {state.saturation_score:.3f} should accumulate "
251+
f"once signs agree again"
252+
)
253+
162254

163255
class TestLoadBalancerLifecycle:
164256
def _make_balancer(self, **kwargs):

tests/test_efficiency_e2e.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,3 +698,59 @@ async def test_saturation_recovery_after_swap(self):
698698
)
699699
finally:
700700
await h.stop()
701+
702+
async def test_load_sign_reversal_does_not_cause_false_saturation(self):
703+
"""When load flips sign (discharge->charge), active battery must not
704+
be falsely detected as saturated while it ramps to the new direction.
705+
706+
Reproduces the real-world ping-pong observed when solar production
707+
changes cause the grid target to flip sign.
708+
"""
709+
h = _SimHarness(
710+
num_batteries=2,
711+
base_load=[200.0, 0.0, 0.0],
712+
min_efficient_power=150,
713+
efficiency_rotation_interval=9999, # no timed rotation
714+
efficiency_fade_alpha=1.0,
715+
efficiency_saturation_threshold=0.4,
716+
saturation_stall_timeout_seconds=60.0,
717+
)
718+
await h.start()
719+
try:
720+
# Let system settle with one battery discharging
721+
await h.step_until(lambda: h.active_battery_count() == 1)
722+
await h.step(10)
723+
724+
active_idx = h.active_battery_indexes()[0]
725+
other_idx = 1 - active_idx
726+
assert h.battery_powers()[active_idx] > 100
727+
728+
# Flip load sign: go from 200W discharge to -200W (solar excess)
729+
h.load_model.base_load[0] = -200.0
730+
731+
# Step through the sign reversal -- the active battery must NOT
732+
# be swapped out due to false saturation.
733+
# 60 steps at 0.3s/step = 18s, plenty for old bug to trigger.
734+
rotations = 0
735+
for _ in range(60):
736+
await h.step()
737+
powers_after = h.battery_powers()
738+
# Detect rotation: other battery becomes sole active
739+
if (
740+
abs(powers_after[other_idx]) > 15
741+
and abs(powers_after[active_idx]) < 15
742+
):
743+
rotations += 1
744+
745+
# The battery should have reversed direction (now charging)
746+
assert h.battery_powers()[active_idx] < -50, (
747+
f"Active battery should be charging after sign reversal. "
748+
f"Powers: {h.battery_powers()}"
749+
)
750+
# No false-saturation rotation should have occurred
751+
assert rotations == 0, (
752+
f"Battery was falsely rotated {rotations} time(s) during "
753+
f"sign reversal. Powers: {h.battery_powers()}"
754+
)
755+
finally:
756+
await h.stop()

0 commit comments

Comments
 (0)