Files
brewpi/tests/components/pid/test_temp_controller_closed_loop.py
jensandClaude Sonnet 5 2d032bf9b2 fix: allow gentle negative HOLD floor to fix steady-state overshoot
Testing the windup-fix rework live against sude/sud_0030.json surfaced a
distinct bug: a HOLD overshoot from grain-fill-in cooling never decayed -
process_pid()'s pid_outer_y floor clamped to exactly 0.0, so pid_inner
fought the pot's own ambient loss to hold the overshot temperature flat
instead of declining back to setpoint (see docs/overshoot2.png).

Replace the hardcoded 0.0 floor with a configurable Outer.y_hold_min
(default 0.0, backward compatible), set to -0.1 in config.json, both
.tpl templates, and the demo scripts - small enough to avoid
reintroducing the bb5af3c limit cycle while letting HOLD request a
gentle decline matching passive ambient cooling.

Adds TestHoldOvershootRecoversToSetpoint and documents the finding in
docs/overshoot_hold_windup.md's Follow-up section and
components/pid/TODO.md. Confirmed against a live sud_0030 re-run, not
just the unit test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M2ierBoxW3v7nUbDw3M2pE
2026-07-06 08:19:10 +02:00

200 lines
7.4 KiB
Python

import unittest
from components.pid.temp_controller_smith import TempController
from components.pid.temp_controller_fsm import States
from components.plant.pot import Pot
DT = 1.0
AMBIENT = 20.0
PLANT_PARAMS = {"M": 27.96, "C": 3403.43, "L": 0.2, "Td": 17}
OUTER_PARAMS = {"kp": 0.6, "ki": 0.0, "kd": 0.0, "kt": 0.0}
OUTER_PARAMS_HOLD_COOL = {"kp": 0.6, "ki": 0.0, "kd": 0.0, "kt": 0.0, "y_hold_min": -0.1}
INNER_HEAT_PARAMS = {"kp": 0.08, "ki": 0.02, "kd": 0.0, "kt": 1.5}
INNER_HOLD_CLAMPED = {"kp": 0.08, "ki": 0.02, "kd": 0.0, "kt": 1.5, "yi_max": 0.3}
INNER_HOLD_UNCLAMPED = dict(INNER_HEAT_PARAMS)
def make_controller(inner_hold_params, outer_params=None):
ctrl = TempController(DT)
ctrl.set_params({
"beta": 0.9,
"Outer": dict(outer_params if outer_params is not None else OUTER_PARAMS),
"Inner": {
"Heat": dict(INNER_HEAT_PARAMS),
"Hold": dict(inner_hold_params),
"Cool": dict(INNER_HEAT_PARAMS),
},
})
ctrl.set_model_plant_params(PLANT_PARAMS)
ctrl.set_ambient_temperature(AMBIENT)
ctrl.set_enabled(True)
return ctrl
def make_plant(initial_temp):
plant = Pot(DT)
plant.set_plant_params(PLANT_PARAMS)
plant.set_ambient_temperature(AMBIENT)
plant.initial(initial_temp)
return plant
def tick(ctrl, plant, temp_soll, heatrate_soll):
plant.process()
ctrl.set_theta_ist(plant.get_temperature())
ctrl.set_theta_soll(temp_soll)
ctrl.set_heatrate_soll(heatrate_soll)
ctrl.process()
power = 3500.0 * max(0.0, ctrl.get_power())
plant.set_power(power)
ctrl.set_model_power(power)
class TestHoldDisturbanceOvershoot(unittest.TestCase):
"""Reproduces the cold-water-during-HOLD incident from
docs/overshoot_hold_windup.md and checks that Inner.Hold's yi_max
measurably reduces the resulting overshoot."""
def _run_disturbance(self, inner_hold_params):
ctrl = make_controller(inner_hold_params)
plant = make_plant(30.0)
# Settle at HOLD, 30 degC.
for _ in range(60):
tick(ctrl, plant, 30.0, 1.0)
self.assertEqual(ctrl.state, States.HOLD)
# Cold-water disturbance: knock plant.temp down ~0.5 degC.
plant.temp -= 0.5
peak = plant.get_temperature()
for _ in range(600):
tick(ctrl, plant, 30.0, 1.0)
peak = max(peak, plant.get_temperature())
# Disturbance must stay within HOLD the whole time, matching
# the incident (diff never reached HoldHeat).
self.assertEqual(ctrl.state, States.HOLD)
return peak
def test_clamped_overshoot_smaller_than_unclamped(self):
peak_clamped = self._run_disturbance(INNER_HOLD_CLAMPED)
peak_unclamped = self._run_disturbance(INNER_HOLD_UNCLAMPED)
overshoot_clamped = peak_clamped - 30.0
overshoot_unclamped = peak_unclamped - 30.0
self.assertLess(overshoot_clamped, overshoot_unclamped)
class TestHoldOvershootRecoversToSetpoint(unittest.TestCase):
"""Reproduces the sud_0030 grain-fill-in incident: a HOLD overshoot
(post-recovery temp_ist above temp_soll, well under HoldCool's
threshold so the FSM never leaves HOLD) must coast back down to
setpoint rather than sitting in a permanent flat-clamp dead zone -
see docs/overshoot_hold_windup.md and the Outer.y_hold_min floor in
temp_controller_base.py's process_pid()."""
def _run_overshoot(self, outer_params):
ctrl = make_controller(INNER_HOLD_CLAMPED, outer_params)
plant = make_plant(30.0)
for _ in range(60):
tick(ctrl, plant, 30.0, 1.0)
self.assertEqual(ctrl.state, States.HOLD)
# Overshoot above setpoint, small enough to stay inside HOLD
# (matches the ~0.3-0.4 degC band observed in the real trace).
plant.temp += 0.4
min_temp = plant.get_temperature()
for _ in range(900):
tick(ctrl, plant, 30.0, 1.0)
self.assertEqual(ctrl.state, States.HOLD)
min_temp = min(min_temp, plant.get_temperature())
return plant.get_temperature(), min_temp
def test_negative_floor_recovers_flat_clamp_does_not(self):
final_flat, _ = self._run_overshoot(OUTER_PARAMS)
final_floored, min_floored = self._run_overshoot(OUTER_PARAMS_HOLD_COOL)
# Old behaviour (y clamped to exactly 0.0): pid_inner fights the
# pot's own ambient loss to hold the overshot temperature flat -
# it stays measurably above setpoint within this window.
self.assertGreater(final_flat - 30.0, 0.05)
# New behaviour (small negative floor): the outer loop can ask
# for a gentle decline, so the overshoot recovers much closer to
# setpoint than the flat clamp manages in the same window.
self.assertLess(abs(final_floored - 30.0), 0.15)
# Guard against reintroducing the violent limit cycle the original
# [0, 1] clamp was added to break (bb5af3c): recovery should coast
# down smoothly, undershooting by no more than the injected
# disturbance itself (0.4 degC) - not the ~1 degC+ swings of a
# runaway power on/off cycle.
self.assertGreater(min_floored, 30.0 - 0.4)
class TestRealRampStillReachesTarget(unittest.TestCase):
"""Guards against reintroducing the rejected flat-clamp regression:
Inner.Heat has no yi_max, so a genuine ramp must still be able to
reach its commanded heat rate."""
def test_ramp_reaches_commanded_heatrate(self):
ctrl = make_controller(INNER_HOLD_CLAMPED)
plant = make_plant(20.0)
heatrate_soll = 1.5
temp_soll = 40.0
max_heatrate_ist = 0.0
reached_heat = False
for _ in range(600):
tick(ctrl, plant, temp_soll, heatrate_soll)
if ctrl.state == States.HEAT:
reached_heat = True
max_heatrate_ist = max(max_heatrate_ist, ctrl.get_heatrate_ist())
self.assertTrue(reached_heat)
self.assertGreater(max_heatrate_ist, 1.4)
class TestHoldHeatHoldTransitionIsBumpless(unittest.TestCase):
"""Per-state yi_max swap must not itself introduce a discontinuity in y
at a HOLD<->HEAT transition - the same Pid instance/state carries over,
only the yi_max ceiling changes going forward."""
def test_no_bump_at_transitions(self):
ctrl = make_controller(INNER_HOLD_CLAMPED)
plant = make_plant(20.0)
y_prev = None
state_prev = None
max_bump = 0.0
for _ in range(1000):
tick(ctrl, plant, 40.0, 1.5)
y = ctrl.get_power()
if state_prev is not None and ctrl.state != state_prev and \
{state_prev, ctrl.state} <= {States.HOLD, States.HEAT}:
max_bump = max(max_bump, abs(y - y_prev))
y_prev = y
state_prev = ctrl.state
# A "bump" here would be a y-step far larger than what one tick of
# normal PID evolution produces elsewhere in the same run - not
# literally zero, since heatrate_err itself keeps evolving tick to
# tick regardless of the state transition. Note: a HEAT->HOLD
# transition can retroactively clamp yi if a sustained ramp pushed
# it past Inner.Hold's yi_max before HeatHold's threshold fired,
# producing a small (not literally bumpless) step - bounded well
# below the full-freeze/thaw magnitude this replaces.
self.assertLess(max_bump, 0.1)
if __name__ == '__main__':
unittest.main()