jensandClaude Sonnet 4.6
2845051350
Drop the now-stale "config.json.templ/.sim still carry dead Model.kn/Plant.kn keys" note (they were deleted), note that Pot now always starts at theta_amb (no separate initial-temp param) and that dropping the gain (efficiency) factor entirely is a partial realism regression, not just a fix. Point the config-validation item at the gain key that's still dead in config.json.sim. Also update demo_sud.py to match the Pot params Pot.__init__ actually reads. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
60 lines
3.4 KiB
Markdown
60 lines
3.4 KiB
Markdown
# components/plant design backlog
|
|
|
|
Findings from reviewing `pot.py`'s water-bath model against real-plant
|
|
behavior (sim/real control mismatch reported by user), refreshed against
|
|
the current `delay`-line rewrite of `Pot`.
|
|
|
|
- [x] **Heat-loss term has wrong units.** Fixed by the `Pot` rewrite:
|
|
`process()` now computes `p_loss = self.L * self.M * (self.temp -
|
|
self.theta_amb)` in Watts and applies `self.temp += (self.delay.get()
|
|
- p_loss) / (self.M * self.C) * self.dt` — the same `/(M*C)`
|
|
normalization for both the gain and loss terms, instead of the old
|
|
mismatched `leak = 1/self.M * self.L * (...)/ self.theta_amb`.
|
|
|
|
- [x] **Thermal lag modeled as single-pole lag, not transport delay.**
|
|
Fixed: `Pot` no longer uses `HeatDiffusion`'s `alpha=dt/Td`
|
|
exponential smoothing (that module was deleted). It now delays the
|
|
input power through `plant/delay.py`'s `Delay` (true dead time)
|
|
before integrating it, matching what this item recommended.
|
|
|
|
- [x] **`kn` is loaded but never applied.** Resolved differently than
|
|
originally proposed: `kn` was removed from `Pot` entirely rather than
|
|
wired up as plant-side measurement noise. Sensor noise is now
|
|
modeled one layer up, in `TempSensorSim` (`temp_offset`/`variance`,
|
|
wired from `brewpi.py`'s `TempSensorFactory.create(..., temp_offset=
|
|
-0.15, variance=0.01)`), not in the plant model. The stale
|
|
`Model.kn`/`Plant.kn` keys this item flagged have since been deleted
|
|
from `config.json.templ`/`config.json.sim` too.
|
|
|
|
- [x] **`theta_amb` is hardcoded, not configured.** Fixed: added a
|
|
top-level `ambient_temperature` key to `config.json`(`.templ`/`.sim`);
|
|
`brewpi.py` reads it once and passes it to both the real `Pot` and
|
|
(via a new `theta_amb` param) `temp_controller_smith.py`'s internal
|
|
models, so the Smith predictor's model matches the real plant's
|
|
assumed ambient instead of silently defaulting to 20. `Pot` also
|
|
dropped its separate `params['theta']` initial-temperature field —
|
|
it now always starts at `theta_amb` (so the same config value governs
|
|
both the assumed ambient *and* the pot's starting temperature; there's
|
|
no longer a way to start a sim at a temperature other than ambient).
|
|
Real ambient temperature still isn't modeled as a *drifting*
|
|
disturbance (it's a static config value, not a live sensor reading,
|
|
though `Pot.set_ambient_temperature()` now exists as a hook for that),
|
|
so robustness to ambient drift remains untested.
|
|
|
|
- [ ] **No actuator/process realism.** Partially improved (`process()`
|
|
clamps `self.temp` to `min(100, ...)`, won't simulate past boiling)
|
|
and partially regressed: `Pot` also dropped its `gain` (heater
|
|
efficiency) factor entirely — `process()` now applies 100% of
|
|
commanded power with no loss, whereas before `params['gain']` could
|
|
model e.g. an 80% efficient transfer. `Model.gain`/`Plant.gain` are
|
|
now dead keys in `config.json.sim` (already absent from `.templ`) —
|
|
see `components/pid/TODO.md`'s config-validation item. Still no
|
|
heater power saturation enforced inside `Pot` itself (handled ad hoc
|
|
in demo harnesses), no evaporation/lid heat loss, and no varying
|
|
thermal mass `M` as volume changes (e.g. boil-off, adding
|
|
water/grain) — `M` is a fixed constant for the whole simulation.
|
|
|
|
- [x] **Debug `print()`s left in `Pot.process()`.** Fixed: the
|
|
unconditional `p_loss`/`theta_amb`/`temp` prints (leftover from the
|
|
heat-loss-units fix) were removed.
|