From 5a75d1541641fb0b68cc80be040be551d2f845bc Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Mon, 6 Jul 2026 08:30:40 +0200 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_01M2ierBoxW3v7nUbDw3M2pE --- components/pid/TODO.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/components/pid/TODO.md b/components/pid/TODO.md index 8be35a9..62a7c9b 100644 --- a/components/pid/TODO.md +++ b/components/pid/TODO.md @@ -155,6 +155,33 @@ git history for `temp_controller.py`/`temp_controller_smith.py`). and re-checking the `HoldCool`/`CoolHold` thresholds in `temp_controller_fsm.py:14-17`), not just wiring. +- [ ] **`Outer.y_hold_min` is a fixed heuristic constant, not grounded in + the pot's actual passive-cooling capacity.** Related to the active-cooler + item above. `Pot.process()` (`components/plant/pot.py:86-89`) already + computes `p_loss = L * M * (temp - amb)` unconditionally every tick, + independent of `y` - passive cooling isn't actually *driven by* the + negative half of `y` today, it's a physics term that happens regardless + of what the controller commands. `-0.1` (see the steady-state overshoot + fix above) was picked by estimating a typical passive loss rate by hand + for one plant/ambient combination; it doesn't adapt to a different pot, + water mass, or `theta_ist - theta_amb` delta. + `TempController` (Smith, `temp_controller_smith.py:12-16,22-32`) already + carries its own internal `Pot` model with `L`/`M`/`C` and ambient + temperature set (needed for the Smith prediction) - it could expose a + `heatrate_loss_max(theta_ist) = L * (theta_ist - theta_amb) / C * 60` + (K/min) from that model and use it to size `y_hold_min` (and bound + `pid_inner_cool`'s target) dynamically per-tick, instead of a fixed + config constant - the "cooling path" would always ask for exactly what + passive loss can actually deliver, no more and no less. Also a natural + stepping stone toward the active-cooler item above: same "ceiling" + concept, just a bigger number once real cooling power exists. + Caveat: `temp_controller.py` ("Normal", non-Smith) has no internal plant + model at all - `set_model_plant_params()`/`set_ambient_temperature()` + are no-ops in `TempControllerBase` and only overridden in the Smith + subclass - so this only works for Smith out of the box; Normal would + need either its own lightweight loss estimate (e.g. from observed + `heatrate_ist` decay while power is ~0) or keep the fixed fallback. + - [ ] **`kalman.py` is now dead code in production.** Neither `temp_controller.py` nor `temp_controller_smith.py` uses `Kalman` anymore; the only remaining references are