From 84b1607c78b5296cc37069b1c66c5320d8142dd0 Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Mon, 22 Jun 2026 17:12:19 +0200 Subject: [PATCH] Forecast a Sud schedule through its confirm steps, not just up to the first one A user_wait_for_continue step used to stop SudForecastEstimator.estimate() dead, so the GUI's forecast plot only ever showed the first segment on Load. Model the wait as a zero-delay auto-confirm instead, so the whole schedule's projected curve is visible right away; correct that assumption piecewise as real confirmations actually happen, anchored at the real elapsed time and temperature. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk --- README.md | 32 +++++----- client/brewpi_gui.py | 77 +++++++----------------- components/sud_forecast.py | 41 ++++++++----- tasks/sud.py | 117 +++++++++++++++++++++++++------------ 4 files changed, 144 insertions(+), 123 deletions(-) diff --git a/README.md b/README.md index 472fb96..92c5ddb 100644 --- a/README.md +++ b/README.md @@ -294,20 +294,24 @@ re-anchoring itself to match whatever's actually happening would no longer be a meaningful baseline to compare reality against. It's computed once, up front, and left alone. -The one exception is a step with `user_wait_for_continue`: a human's -response time genuinely can't be forecast, so `SudForecastEstimator. -estimate()` simply stops simulating there instead of assuming a delay (the -old behavior - zero delay - quietly understated the schedule). The -forecast is therefore piecewise: `tasks/sud.py`'s `SudTask` sends the first -segment (up to the first such step, or the end of the schedule if there is -none) on Load, marking it `Finished: false` if it stopped early. The *next* -segment is only computed once the real confirmation actually happens, -anchored at the real elapsed time and real current temperature at that -moment - so the unforecastable wait shows up honestly as a gap in the -timeline rather than as a guess - and appended to what's already shown. -While waiting, the GUI just repeats the forecast's last value out to "now" -(`SudForecastPlot.extend_forecast_while_waiting()`) rather than leaving a -visual gap, then snaps to the new segment the moment it arrives. +The one wrinkle is a step with `user_wait_for_continue`: a human's response +time genuinely can't be forecast. Rather than stall the whole estimate +there, `SudForecastEstimator.estimate()` models it as a zero-delay +auto-confirm and keeps simulating straight through to the schedule's actual +end - so the full projected curve is visible right away on Load, instead of +stopping at the first such step. `estimate()` also returns `confirm_points`: +where (in step index and simulated time) each of those zero-delay +assumptions was made. + +That assumption gets corrected once the real confirmation actually +happens: `tasks/sud.py`'s `SudTask._continue_forecast_after_confirm()` +looks up the relevant `confirm_points` entry, truncates the forecast right +back to that point, and splices in a freshly anchored simulation of the +remaining steps - anchored at the real elapsed time and real current +temperature, so an actual delay shows up honestly as a gap in the timeline +rather than as the assumed zero. The corrected forecast is sent in full +each time, so the GUI's `SudForecastPlot.show_forecast()` simply redraws +the dashed line outright rather than patching it up itself. Comparing the two lines: real-world divergence from the forecast - more than a transient blip during a ramp's settling - is worth investigating as diff --git a/client/brewpi_gui.py b/client/brewpi_gui.py index b8fa1d3..1fd4070 100755 --- a/client/brewpi_gui.py +++ b/client/brewpi_gui.py @@ -110,21 +110,19 @@ class RealtimePlot(FigureCanvasQTAgg): class SudForecastPlot(FigureCanvasQTAgg): """Compares a Sud schedule's actual control behavior (line, solid) - against a fixed, simulation-based forecast (line_projected, dashed) - - the same plant/controller model the real run uses + against a simulation-based forecast (line_projected, dashed) - the + same plant/controller model the real run uses (components/sud_forecast.py's SudForecastEstimator - see - show_computing()/show_forecast()). The forecast is computed once per - piecewise segment and never revisited once a run is using it - the - whole point is an honest, unmoving baseline to compare reality - against, not a prediction that keeps re-anchoring itself to match - whatever's actually happening. The one exception is a step requiring - user confirmation: a human's response time genuinely can't be - forecast, so the segment just stops there, and the next one is - computed for real (anchored at the real elapsed time and - temperature) only once the user actually confirms - see - tasks/sud.py's SudTask._continue_forecast_after_confirm(). - extend_forecast_while_waiting() keeps the dashed line visually flat - through the wait in the meantime.""" + show_computing()/show_forecast()). The forecast covers the whole + schedule from the very first Load, including through any step + requiring user confirmation - a human's response time genuinely + can't be forecast, so it's modeled as a zero-delay auto-confirm + there instead of stopping. That assumption gets corrected once the + real confirmation actually happens (anchored at the real elapsed + time and temperature) - see tasks/sud.py's SudTask. + _continue_forecast_after_confirm() - at which point show_forecast() + is called again with the corrected curve, replacing the optimistic + guess outright rather than the GUI patching it up itself.""" def __init__(self): figure = Figure(facecolor='white') @@ -143,13 +141,6 @@ class SudForecastPlot(FigureCanvasQTAgg): figure.tight_layout() - # The forecast as last received from the server (see - # show_forecast()) - kept around so extend_forecast_while_waiting() - # can repeat its last value without the caller needing to pass it - # again every tick. - self._forecast_t_min = [] - self._forecast_theta = [] - def show_computing(self, name): """Shown right after a schedule loads, while the first simulation-based forecast segment is still being computed @@ -159,8 +150,6 @@ class SudForecastPlot(FigureCanvasQTAgg): (there's nothing meaningful to fit yet), so whatever was on screen before just stays frozen underneath the title change until show_forecast() replaces it.""" - self._forecast_t_min = [] - self._forecast_theta = [] self.line.set_data([], []) self.line_projected.set_data([], []) self.ax.set_title("{} - computing forecast...".format(name), fontsize='small') @@ -168,18 +157,16 @@ class SudForecastPlot(FigureCanvasQTAgg): self.draw_idle() def show_forecast(self, t_min, theta, name, finished): - """The forecast curve (dashed) - computed once, piecewise, by the - server simulating the schedule with its real plant/controller + """The forecast curve (dashed) - computed by the server simulating + the schedule with its real plant/controller (components/sud_forecast.py) - see Window.on_sud_forecast_ - received(). Called again, with a longer t_min/theta, each time a - further piecewise segment gets appended after a real user - confirmation; finished is False while the forecast doesn't yet - cover the rest of the schedule (stopped at a step requiring - confirmation - see extend_forecast_while_waiting()). Locks the - axes to fit the forecast alone, regardless of whatever the + received(). Called again, with a corrected t_min/theta, each time + a real user confirmation replaces a zero-delay guess with one + anchored at the real elapsed time and temperature; finished is + False only in the pathological case of a step whose target can + never be reached (components/sud_forecast.py's MAX_TICKS). Locks + the axes to fit the forecast alone, regardless of whatever the actual trace (line) is currently doing.""" - self._forecast_t_min = t_min - self._forecast_theta = theta self.line_projected.set_data(t_min, theta) self._set_fixed_limits(t_min, theta) total = t_min[-1] if t_min else 0.0 @@ -188,21 +175,6 @@ class SudForecastPlot(FigureCanvasQTAgg): self.figure.tight_layout() self.draw_idle() - def extend_forecast_while_waiting(self, elapsed_min): - """Called every tick while the real Sud is waiting on user - confirmation: a human's response time can't be forecast, so - rather than guess, the dashed line just repeats its last value - out to 'now' until the next segment is appended for real (see - tasks/sud.py's SudTask._continue_forecast_after_confirm(), fired - the moment the real confirmation happens) - at which point - show_forecast() overwrites this temporary extension with the - actual next segment.""" - if not self._forecast_t_min or elapsed_min <= self._forecast_t_min[-1]: - return - last_theta = self._forecast_theta[-1] - self.line_projected.set_data(self._forecast_t_min + [elapsed_min], self._forecast_theta + [last_theta]) - self.draw_idle() - def _set_fixed_limits(self, t_list, theta_list): """Explicitly sets (and, unlike relim()+autoscale_view(), locks) the axes' view to fit exactly the given data, with matplotlib's @@ -235,8 +207,6 @@ class SudForecastPlot(FigureCanvasQTAgg): self.draw_idle() def show_no_schedule(self): - self._forecast_t_min = [] - self._forecast_theta = [] self.line.set_data([], []) self.line_projected.set_data([], []) self.clear_progress() @@ -472,13 +442,6 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow): if self.sud_state != SUD_PAUSED_STATE and self.plot_temp_ist: self.forecast_history.append((elapsed_min, self.plot_temp_ist)) self.forecast_plot.show_dynamic(self.forecast_history) - if self.sud_state == SUD_WAIT_USER_STATE: - # A human's confirm time can't be forecast - the dashed - # line just repeats its last value out to 'now' until - # the real confirmation appends the next segment (see - # tasks/sud.py's SudTask._continue_forecast_after_ - # confirm()). - self.forecast_plot.extend_forecast_while_waiting(elapsed_min) else: self.forecast_plot.clear_progress() diff --git a/components/sud_forecast.py b/components/sud_forecast.py index fa06210..c14c77f 100644 --- a/components/sud_forecast.py +++ b/components/sud_forecast.py @@ -34,18 +34,23 @@ class SudForecastEstimator: self.theta_amb = theta_amb def estimate(self, doc, start_theta=None): - """Returns (t, theta, final_state): t/theta are parallel lists of - elapsed simulated seconds and temperature for one piecewise - segment of the schedule, starting from doc['steps'][0] - either - all the way to the end (final_state is SudState.DONE), or, if a - step requires user confirmation, only up to and including that - step's hold (final_state is SudState.WAIT_USER). A real user's - confirm time can't be forecast, so the simulation simply stops - there instead of guessing zero delay or otherwise - the caller is - expected to compute the next segment for real once the user - actually confirms (see tasks/sud.py's SudTask), anchored at - whatever the real elapsed time and temperature are by then, - rather than this estimate continuing to guess at it. + """Returns (t, theta, final_state, confirm_points): t/theta are + parallel lists of elapsed simulated seconds and temperature, + covering doc['steps'] from the start all the way to the end + (final_state is SudState.DONE), or, in the pathological case of + a step whose target can never actually be reached, wherever + MAX_TICKS cut the simulation off. + + A step requiring user confirmation doesn't stop the simulation + either - a human's response time genuinely can't be forecast, + so it's modeled as zero delay (auto-confirmed the instant that + step's hold completes) rather than leaving the estimate stuck + there forever. confirm_points records every place that + assumption was made, as (step_index, t) pairs, so the caller + (tasks/sud.py's SudTask) can correct it once a real + confirmation actually happens: truncate the forecast at that + point and splice in a freshly anchored simulation of the + remaining steps in place of the optimistic guess. start_theta defaults to the configured ambient temperature - i.e. a cold start, same as the GUI's static estimate.""" @@ -54,7 +59,7 @@ class SudForecastEstimator: sud = Sud() if not sud.load(doc) or not sud.schedule: - return [0.0], [start_theta], SudState.DONE + return [0.0], [start_theta], SudState.DONE, [] pot = Pot(self.dt, self.plant_params, self.theta_amb) pot.initial(start_theta) @@ -87,10 +92,16 @@ class SudForecastEstimator: t = [0.0] theta = [pot.get_temperature()] + confirm_points = [] sud.start() ticks = 0 - while sud.state not in (SudState.DONE, SudState.WAIT_USER) and ticks < MAX_TICKS: + while sud.state != SudState.DONE and ticks < MAX_TICKS: + if sud.state == SudState.WAIT_USER: + confirm_points.append((sud.index, t[-1])) + sud.confirm() + continue + pot.process() tc.set_theta_ist(pot.get_temperature()) tc.process() @@ -105,4 +116,4 @@ class SudForecastEstimator: theta.append(pot.get_temperature()) ticks += 1 - return t, theta, sud.state + return t, theta, sud.state, confirm_points diff --git a/tasks/sud.py b/tasks/sud.py index e4f0862..6c3deb5 100644 --- a/tasks/sud.py +++ b/tasks/sud.py @@ -1,4 +1,5 @@ import asyncio +import bisect from tasks import ATask from ws.message import MsgIo from utils.value import ChangedFloat @@ -29,18 +30,29 @@ class SudTask(ATask): # build a SudTask without one still work; the server always passes # one. self.forecast_estimator = forecast_estimator - # Accumulated forecast across all piecewise segments computed so - # far (see send_forecast()/_continue_forecast_after_confirm()) - - # T in simulated seconds, Theta in degrees, parallel lists, both - # growing monotonically as segments get appended; never re-walked - # from scratch once a run is in progress, except for the one - # genuinely unforecastable case (a real user confirmation). + # The forecast as last computed/corrected (see send_forecast()/ + # _continue_forecast_after_confirm()) - T in simulated seconds, + # Theta in degrees, parallel lists, both monotonically growing as + # corrections get spliced in. Covers the whole schedule from the + # very first Load, including through steps requiring user + # confirmation - those are simulated as a zero-delay auto-confirm + # (see components/sud_forecast.py's SudForecastEstimator. + # estimate()) rather than left unforecast. self.forecast_t = [] self.forecast_theta = [] - # Whether the most recently computed segment stopped at a step - # requiring user confirmation rather than reaching the - # schedule's actual end - see _continue_forecast_after_confirm(). - self.forecast_waiting_for_confirm = False + # Whether the forecast above actually reaches the schedule's real + # end (sent as 'Finished') - False only in the pathological case + # of a step whose target can never be reached (see + # components/sud_forecast.py's MAX_TICKS). + self.forecast_finished = True + # Where each user-confirmation step's zero-delay assumption sits + # in the forecast timeline above, keyed by the schedule's + # (absolute) step index - see SudForecastEstimator.estimate()'s + # confirm_points. Consulted by _continue_forecast_after_confirm() + # to know where to cut the optimistic guess loose and splice in a + # freshly anchored simulation once that confirmation actually + # happens for real. + self.forecast_confirm_marks = {} msg_handler.set_recv_handler(self.recv) def apply_plant_params(self, step): @@ -119,12 +131,18 @@ class SudTask(ATask): asyncio.create_task(self.send({'Elapsed': value})) async def send_forecast(self, doc): - """Computes and sends the first piecewise segment of doc's - forecast - from the very start, up to either the schedule's end - or its first step requiring user confirmation (see - components/sud_forecast.py's SudForecastEstimator.estimate()). - Always a fresh start: resets any segments accumulated for - whatever was loaded before.""" + """Computes and sends the full forecast for doc, start to finish - + including through every step requiring user confirmation, which + is modeled as a zero-delay auto-confirm rather than left + unforecast (see components/sud_forecast.py's + SudForecastEstimator.estimate()) - so the whole schedule's + projected curve is visible right away instead of stopping at the + first one. Always a fresh start: discards whatever forecast was + accumulated for the previously loaded schedule. + + Those zero-delay assumptions get corrected piecewise as real + confirmations actually happen - see + _continue_forecast_after_confirm().""" if self.forecast_estimator is None: return # Runs the simulation in a worker thread - it's CPU-bound and can @@ -132,34 +150,53 @@ class SudTask(ATask): # otherwise stall every other task (heater, sensor, ...) for that # whole window. loop = asyncio.get_event_loop() - t, theta, final_state = await loop.run_in_executor(None, self.forecast_estimator.estimate, doc) + t, theta, final_state, confirm_points = await loop.run_in_executor(None, self.forecast_estimator.estimate, doc) self.forecast_t = t self.forecast_theta = theta - self.forecast_waiting_for_confirm = (final_state == SudState.WAIT_USER) + self.forecast_finished = (final_state == SudState.DONE) + self.forecast_confirm_marks = dict(confirm_points) await self._send_forecast() async def _send_forecast(self): await self.send({'Forecast': { 'T': self.forecast_t, 'Theta': self.forecast_theta, - 'Finished': not self.forecast_waiting_for_confirm, + 'Finished': self.forecast_finished, }}) - async def _continue_forecast_after_confirm(self): - """Computes and appends the next piecewise segment once a real - user confirmation has actually happened, rather than guessing at - the delay - see SudForecastEstimator.estimate()'s docstring for - why. Anchored at the real elapsed time (self.sud.elapsed), so the - unforecastable wait shows up honestly as a gap in the forecast - rather than as zero delay, and at the real current temperature - (more trustworthy than whatever the now-superseded previous - segment's simulation predicted it would be by now).""" - if self.forecast_estimator is None or not self.forecast_waiting_for_confirm: + async def _continue_forecast_after_confirm(self, confirmed_index): + """Corrects the optimistic, zero-delay guess send_forecast() (or a + previous call to this method) made at confirmed_index's + user-confirmation step, now that the confirmation has actually + happened for real - see SudForecastEstimator.estimate()'s + docstring for why that assumption can't just be trusted as-is. + Truncates the forecast right back to that point and splices in a + freshly anchored simulation of the rest of the schedule, anchored + at the real elapsed time (self.sud.elapsed) - so an actual delay + shows up honestly as a gap rather than as the assumed zero - and + the real current temperature (more trustworthy than whatever the + now-superseded guess predicted it would be by now). + + No-op if confirmed_index was never part of a computed forecast in + the first place (e.g. the estimator isn't configured).""" + if self.forecast_estimator is None: return + mark_t = self.forecast_confirm_marks.pop(confirmed_index, None) + if mark_t is None: + return + # Drop the now-stale tail (everything beyond the confirmation + # point) - both the curve itself and any confirm marks that fell + # within it, since they're about to be replaced by fresh ones. + cut = bisect.bisect_right(self.forecast_t, mark_t) + self.forecast_t = self.forecast_t[:cut] + self.forecast_theta = self.forecast_theta[:cut] + self.forecast_confirm_marks = {idx: t for idx, t in self.forecast_confirm_marks.items() if t <= mark_t} + schedule = self.sud.schedule index = self.sud.index if not (0 <= index < len(schedule)): - self.forecast_waiting_for_confirm = False + self.forecast_finished = True + await self._send_forecast() return doc = { 'Name': self.sud.name, @@ -171,17 +208,22 @@ class SudTask(ATask): start_theta = self.tc.get_theta_ist() real_elapsed = self.sud.elapsed loop = asyncio.get_event_loop() - t, theta, final_state = await loop.run_in_executor(None, self.forecast_estimator.estimate, doc, start_theta) - # Bridge the gap between where the previous segment's own - # simulated time left off and the real elapsed time the confirm - # actually happened at, holding flat at its last temperature - - # then append the new segment, offset to start exactly there. + t, theta, final_state, confirm_points = await loop.run_in_executor(None, self.forecast_estimator.estimate, doc, start_theta) + # Bridge the gap between the confirmation point and the real + # elapsed time it actually happened at, holding flat at its last + # temperature - then append the new segment, offset to start + # exactly there. if self.forecast_t and self.forecast_t[-1] < real_elapsed: self.forecast_t.append(real_elapsed) self.forecast_theta.append(self.forecast_theta[-1]) self.forecast_t.extend(real_elapsed + seconds for seconds in t) self.forecast_theta.extend(theta) - self.forecast_waiting_for_confirm = (final_state == SudState.WAIT_USER) + # confirm_points' indices/times are relative to this sub-schedule + # (starting fresh at doc['steps'][0]) - rebase both onto the real + # schedule's absolute indices and the master forecast timeline. + self.forecast_confirm_marks.update( + (index + local_index, real_elapsed + local_t) for local_index, local_t in confirm_points) + self.forecast_finished = (final_state == SudState.DONE) await self._send_forecast() async def recv(self, data): @@ -189,8 +231,9 @@ class SudTask(ATask): if 'Start' in pair[0]: self.sud.start() elif 'Confirm' in pair[0]: + confirmed_index = self.sud.index self.sud.confirm() - asyncio.create_task(self._continue_forecast_after_confirm()) + asyncio.create_task(self._continue_forecast_after_confirm(confirmed_index)) elif 'Pause' in pair[0]: self.sud.pause() elif 'Stop' in pair[0]: