From da7e954b7c187660c208ee9be0398d390e57139f Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Wed, 1 Jul 2026 19:50:50 +0200 Subject: [PATCH] fix: make sim_warp_factor real-hardware-safe and sud logs forecast-reproducible sim_warp_factor was scaling DT_TASK unconditionally, including against real hardware, and was leaking into recorded sample timestamps via wall-clock time - both violate its intended meaning (a reciprocal scale on task wait time only). Clamp it to 1.0 whenever Heater.type isn't "sim", and track elapsed time via tick-counted accumulators (Sud.elapsed for SudLogTask, a matching one for ServerLogTask) instead of time.monotonic(). Sud/server logs now also carry HeaterPowers, the effective SimWarpFactor, the raw Doc, a correct Name, and ForecastAnchors (one entry per real _reanchor_forecast() trigger) - enough for utils/analyze_log.py to rebuild a SudForecastEstimator and replay the exact same splice-per-transition forecast correction offline that a live client sees, replacing the old (always-dead) forecast_*.json sidecar lookup. Two bugs found and fixed along the way: - ServerLogTask's periodic wall-clock checkpoint had no way to know a SudLogTask run had just ended, so it would immediately re-write the just-completed log with PlantParams/ForecastAnchors cleared back to empty. Gated the checkpoint on _should_sample(). - _reanchor_forecast() truncated the old forecast against real elapsed time, which can end up numerically less than the old forecast's own (very wrong) speculative timestamps once a WAIT_USER confirmation runs long - leaving a "ghost" segment instead of a clean cut. Truncate against forecast_step_starts[index] instead, which lives in the same coordinate space as the forecast being cut. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JFDeoTKBDkXRhPfnyDEoBt --- README.md | 61 ++++++++++++++++++++++++++---- server/brewpi.py | 14 +++++-- tasks/server_log.py | 58 +++++++++++++++++++++++++---- tasks/sud.py | 39 +++++++++++++++++-- tasks/sud_log.py | 44 ++++++++++++++++++++-- utils/analyze_log.py | 89 +++++++++++++++++++++++++++++++++++++++----- 6 files changed, 268 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 3ce1448..d694968 100644 --- a/README.md +++ b/README.md @@ -253,6 +253,13 @@ pip install -r client/requirements.txt ./brewpi.py --sim-warp-factor 50 # 50x speedup, e.g. for dev/testing ./brewpi.py --http-port 8888 # change the browser client's port ``` + + `--sim-warp-factor` only ever reciprocally scales the *wait* time + between task ticks (`DT_TASK`) - it never scales `--dt` itself, and has + no effect at all against real hardware (`Heater.type` other than + `"sim"`): real hardware ticks at its own physical pace regardless of + this flag, so a real run is always paced at `DT_TASK == 1.0` (true real + time) no matter what `--sim-warp-factor` is set to. 3. Either connect with the desktop GUI: ```bash @@ -480,13 +487,27 @@ moment the schedule actually reaches the next real step boundary, not just at a user confirmation: `tasks/sud.py`'s `SudTask._reanchor_forecast()`, called from `on_step_changed()` on *every* transition (a full step change and a step's own ramp→hold phase switch alike), truncates the forecast -back to right now and splices in a freshly anchored simulation of the -rest of the schedule - anchored at the real elapsed time and the real -current temperature (`TempControllerBase.get_theta_ist()` - trustworthy -here, unlike at Load, since a real run has been actively ticking for a -while by the time this runs). The corrected forecast is sent in full each -time, so the GUI's `SudForecastPlot.show_forecast()` simply redraws the -(faded) forecast line outright rather than patching it up itself. +and splices in a freshly anchored simulation of the rest of the schedule - +anchored at the real elapsed time and the real current temperature +(`TempControllerBase.get_theta_ist()` - trustworthy here, unlike at Load, +since a real run has been actively ticking for a while by the time this +runs). The corrected forecast is sent in full each time, so the GUI's +`SudForecastPlot.show_forecast()` simply redraws the (faded) forecast line +outright rather than patching it up itself. + +The truncation point is `forecast_step_starts[index]` (the *old* forecast's +own belief of where step `index` begins), not the real elapsed time itself +- those can diverge badly for exactly the `user_wait_for_continue` case +above: if a human's real confirmation takes far longer than the zero-delay +guess assumed, the old forecast's entire speculative remainder (which +raced straight through the rest of the schedule) still carries timestamps +numerically *less than* the now-much-larger real elapsed time, so cutting +against real elapsed time directly would find nothing to discard and just +append the fresh, correct simulation after it - a doubled-back, self- +overlapping curve instead of a clean cut. `forecast_step_starts[index]` +lives in the same (speculative) coordinate space as the forecast being +truncated, so it isn't fooled by how far real and speculated time have +drifted apart. Two transitions can fire in close succession (a step whose hold duration is already `0` advances right through it within a single tick, and a @@ -729,3 +750,29 @@ to `logs/log_T