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

Merged
jens merged 6 commits from overshoot_hold_windup into master 2026-07-06 08:37:21 +02:00
6 Commits
Author SHA1 Message Date
jensandClaude Sonnet 5 5a75d15416 docs: note grounding y_hold_min in passive-cooling capacity in pid TODO
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
2026-07-06 08:30:40 +02:00
jensandClaude Sonnet 5 6d17419067 docs: note active-cooler architecture assessment in pid TODO
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
2026-07-06 08:26:25 +02:00
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
jensandClaude Sonnet 5 11c3d92f25 docs: refresh windup writeup to past tense, link architecture diagram
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
2026-07-05 22:08:19 +02:00
jensandClaude Sonnet 5 acfb98e186 docs: add FSM states diagram screenshot
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
2026-07-05 21:59:20 +02:00
jensandClaude Sonnet 5 f69d63c31b fix: split inner-loop yi_max clamp to fix HOLD-state windup overshoot
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
2026-07-05 21:23:55 +02:00