Fix HOLD-state PID overshoot (transient windup + steady-state offset) #4

2026-07-06T08:36:56+02:00

jens commented

2026-07-06 08:36:56 +02:00

Summary

Split the inner-loop yi_max anti-windup clamp into Inner.Heat/Inner.Hold/Inner.Cool, renamed pid_hold/pid_heat/pid_cool to pid_outer/pid_inner/pid_inner_cool to match what actually runs when, fixing a transient windup overshoot during a HOLD-state disturbance.
Fixed a distinct, permanent steady-state HOLD overshoot found testing live against sude/sud_0030.json: replaced the hardcoded 0.0 floor on pid_outer_y with a configurable Outer.y_hold_min (default 0.0, backward compatible; set to -0.1 in config.json/templates/demos), letting HOLD request a gentle decline back to setpoint instead of holding flat forever.
Documented two follow-on design questions as new components/pid/TODO.md items: readiness for a real active cooler, and grounding Outer.y_hold_min in the pot's actual passive-cooling capacity instead of a fixed heuristic.

Test plan

python -m unittest discover -t . -s tests (7 tests, all passing)
Confirmed against a live re-run of sud_0030 with the fix applied

🤖 Generated with Claude Code

## Summary - Split the inner-loop `yi_max` anti-windup clamp into `Inner.Heat`/`Inner.Hold`/`Inner.Cool`, renamed `pid_hold`/`pid_heat`/`pid_cool` to `pid_outer`/`pid_inner`/`pid_inner_cool` to match what actually runs when, fixing a transient windup overshoot during a HOLD-state disturbance. - Fixed a distinct, permanent steady-state HOLD overshoot found testing live against `sude/sud_0030.json`: replaced the hardcoded `0.0` floor on `pid_outer_y` with a configurable `Outer.y_hold_min` (default `0.0`, backward compatible; set to `-0.1` in `config.json`/templates/demos), letting HOLD request a gentle decline back to setpoint instead of holding flat forever. - Documented two follow-on design questions as new `components/pid/TODO.md` items: readiness for a real active cooler, and grounding `Outer.y_hold_min` in the pot's actual passive-cooling capacity instead of a fixed heuristic. ## Test plan - [x] `python -m unittest discover -t . -s tests` (7 tests, all passing) - [x] Confirmed against a live re-run of `sud_0030` with the fix applied 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jens added 6 commits 2026-07-06 08:36:57 +02:00

fix: split inner-loop yi_max clamp to fix HOLD-state windup overshoot f69d63c31b

Renames pid_hold/pid_heat/pid_cool to pid_outer/pid_inner/pid_inner_cool
to match what actually runs when, and splits inner-loop config into
Inner.Heat/Inner.Hold/Inner.Cool so the same PID instance gets a tight
yi_max ceiling only while HOLD drives it, without capping legitimate
1.5 K/min ramps. Fixes the overshoot from docs/overshoot_hold_windup.md
where a cold-water disturbance during HOLD wound up pid_heat's integral
term with no anti-windup engagement, taking ~35s+ to unwind naturally.

Breaking config change: Hold/Heat/Cool -> Outer/Inner.{Heat,Hold,Cool}
in config.json, both .tpl templates, the pid/sud demo scripts, and
replay_sim.py's CLI flags. Adds tests/components/pid/ (stdlib unittest)
covering the Pid clamp/recovery behavior and closed-loop disturbance,
ramp, and HOLD<->HEAT transition cases.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGQhVQ2Y3yXAQTXhrxVd5u

docs: add FSM states diagram screenshot acfb98e186

Reference screenshot of the FSM-states panel from the temp controller
architecture diagram (states, thresholds, and the HOLD->HEAT no-reset
bumpless-transfer note).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGQhVQ2Y3yXAQTXhrxVd5u

docs: refresh windup writeup to past tense, link architecture diagram 11c3d92f25

overshoot_hold_windup.md read like a live plan ("not yet implemented")
even though the fix had already shipped; reworks it into a past-tense
record with corrected line numbers and test descriptions matching what
was actually built. Fixes a stale pid_heat reference in
temp_control_calibration.md.

TODO.md's windup item still claimed test coverage was "outstanding"
from before the test suite was added; corrects that and the "No
automated tests" item above it to reflect current coverage. Both docs
now link docs/fsm_states.png (durable) and the Claude Artifact URL
(session-scoped) for the cascade architecture diagram.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGQhVQ2Y3yXAQTXhrxVd5u

fix: allow gentle negative HOLD floor to fix steady-state overshoot 2d032bf9b2

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

docs: note active-cooler architecture assessment in pid TODO 6d17419067

Captures the answer to "would the cascade/FSM design still hold up with
a real active cooler instead of heat-only + passive loss": PID/plant
model already generalize (symmetric y range, sign-agnostic plant math,
existing COOL state/pid_inner_cool), but the actor-level max(0, y)
clamp in tasks/heater.py is the one thing making today's tuning
mismatches free - removing it exposes both Outer.y_hold_min and
pid_inner_cool as live, untuned negative-power paths.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M2ierBoxW3v7nUbDw3M2pE

docs: note grounding y_hold_min in passive-cooling capacity in pid TODO 5a75d15416

Captures the idea of computing the HOLD-state negative floor from the
Smith controller's internal Pot model (L*(theta_ist-theta_amb)/C) instead
of the fixed -0.1 heuristic, plus the caveat that the Normal controller
has no internal plant model to draw the same estimate from.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M2ierBoxW3v7nUbDw3M2pE

jens merged commit 1e7042d906 into master

2026-07-06 08:37:21 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jens/brewpi#4