SAFETY-40: the integrator bound is not a BUG SIGNAL; numbers from the clamp tick
The clamp alarm gave `i_clamped` (normal anti-windup) the over-rating BUG SIGNAL text, printed `Decision.i_w` as "the loop asked for", and printed `self.target` (after slew/freeze) as "was clamped to". - clamp.loop_clamp_note(): i_clamped says "the loop's integrator reached its N W anti-windup bound" with this tick's command; clamped keeps BUG SIGNAL with the requested command and the clamp's own result. - Decision carries i_bound_w / requested_w / clamped_w, set at the clamp. - i_clamped still counts and notifies (same id, cadence, SAFETY-41 escalation) but as a WARNING, amber row, plain title. A bug has its own notification budget and its text is not overwritten within an episode. - test_clamp.py part 9: red on the old code per defect; 5 mutations killed. - DOCS.md alarm section (and its stale 30 s cadence), DECISIONS/SAFETY-40. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
59415ea83f
commit
fd14569a71
@@ -73,6 +73,7 @@ An alarm wired only to this module's `ClampResult.engaged` would therefore be
|
||||
silent on the exact configuration the whole ticket is about. The winding is
|
||||
visible one layer up, at the integrator bound, and that is why `Decision` now
|
||||
carries `i_clamped` / `clamped` BOOLEANS and the caller ORs them in here.
|
||||
They alarm in different words: see `loop_clamp_note()` (SAFETY-40).
|
||||
|
||||
AND WHY BOOLEANS RATHER THAN `Decision.reason`. `reason` is a single string that
|
||||
later stages overwrite: a cycle that clamps the integrator and is then slew
|
||||
@@ -200,9 +201,8 @@ REASON_CODE = "OUTPUT_CLAMP"
|
||||
# ceiling rests on one number a test author chose; if that number is ever
|
||||
# revisited, this constant moves with it.
|
||||
#
|
||||
# ⚠️ THIS IS A STOP-GAP AND DOES NOT MAKE THE ALARM CORRECT. The message still
|
||||
# calls the integrator bound a BUG SIGNAL and still pairs a request from one
|
||||
# moment with a `self.target` read at another - SAFETY-40. Nothing here is
|
||||
# ⚠️ THIS WAS A STOP-GAP; the wording and the numbers were SAFETY-40's, fixed
|
||||
# in `loop_clamp_note()` below. Nothing here is
|
||||
# suppressed: every engagement is still counted, the action-log row is written on
|
||||
# every engaged cycle regardless, and the rising edge of every episode alarms
|
||||
# unconditionally, so no alarm is DROPPED - only delayed.
|
||||
@@ -234,6 +234,31 @@ CLEAR_CYCLES = 3
|
||||
SUSTAINED_CYCLES = 3
|
||||
|
||||
|
||||
BUG_SIGNAL = ("This is a BUG SIGNAL, not a limit being used: something "
|
||||
"upstream computed a setpoint the actuator cannot deliver.")
|
||||
|
||||
|
||||
def loop_clamp_note(d) -> str:
|
||||
"""The loop-side alarm text for one `control.Decision`, or "".
|
||||
|
||||
SAFETY-40: the integrator bound is anti-windup doing its job and says so;
|
||||
only a command over the rating is a BUG SIGNAL. Every number is the
|
||||
decision's own, i.e. from the tick that clamped.
|
||||
"""
|
||||
if d.clamped:
|
||||
return (f"{REASON_CODE}: the loop computed {d.requested_w:+.0f} W, over "
|
||||
f"the {abs(d.clamped_w):.0f} W rating, and it was clamped to "
|
||||
f"{d.clamped_w:+.0f} W; the loop's command this cycle was "
|
||||
f"{d.target_w:+.0f} W. {BUG_SIGNAL}")
|
||||
if d.i_clamped:
|
||||
return (f"{REASON_CODE}: the loop's integrator reached its "
|
||||
f"{d.i_bound_w:+.0f} W anti-windup bound; the loop's command "
|
||||
f"this cycle was {d.target_w:+.0f} W. Brief "
|
||||
f"engagements after a large load or solar step are normal. If it "
|
||||
f"persists, check that the battery is following its commands.")
|
||||
return ""
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ClampResult:
|
||||
"""What the guard did, with the RAW value preserved.
|
||||
@@ -342,23 +367,51 @@ class ClampWatchdog:
|
||||
# spoken yet - the two the escalation clause needs.
|
||||
self.run_len = 0
|
||||
self.spoken = False
|
||||
# SAFETY-40. Did this episode include a genuine over-rating (`bug`), and
|
||||
# has THAT spoken? A bug has its own budget, so anti-windup notices can
|
||||
# never delay it; with bug=True throughout, cadence is as before.
|
||||
self.bug = False
|
||||
self.bug_spoken = False
|
||||
self.last_bug_alarm: datetime | None = None
|
||||
|
||||
def note(self, engaged: bool, reason: str, now: datetime) -> bool:
|
||||
def _due(self, last, now) -> bool:
|
||||
return last is None or (now - last).total_seconds() >= REASSERT_S
|
||||
|
||||
def _speak(self, now, bug) -> bool:
|
||||
self.last_alarm = now
|
||||
self.spoken = True
|
||||
if bug:
|
||||
self.last_bug_alarm = now
|
||||
self.bug_spoken = True
|
||||
return True
|
||||
|
||||
def note(self, engaged: bool, reason: str, now: datetime,
|
||||
bug: bool = True) -> bool:
|
||||
"""Advance one cycle. True when the caller should raise the alarm.
|
||||
|
||||
`now` is the caller's clock, the same discipline as
|
||||
`SignWatchdog.evaluate()` and `FreezeWatchdog.evaluate()`.
|
||||
`SignWatchdog.evaluate()` and `FreezeWatchdog.evaluate()`. `bug` False
|
||||
is the integrator bound (SAFETY-40): counted and notified like the
|
||||
rest, but it never replaces a bug's text within an episode.
|
||||
"""
|
||||
if engaged:
|
||||
self.engagements += 1
|
||||
self.clean_cycles = 0
|
||||
self.reason = reason
|
||||
self.run_len += 1
|
||||
if not self.active:
|
||||
rising = not self.active
|
||||
if rising:
|
||||
self.active = True
|
||||
self.since = now
|
||||
self.episodes += 1
|
||||
self.spoken = False
|
||||
self.bug = False
|
||||
self.bug_spoken = False
|
||||
if bug or not self.bug:
|
||||
self.reason = reason
|
||||
self.bug = self.bug or bug
|
||||
if bug and not self.bug_spoken and self._due(self.last_bug_alarm, now):
|
||||
return self._speak(now, True)
|
||||
if rising:
|
||||
# ⚠️ SAFETY-41. THE RISING EDGE IS RATE-LIMITED TOO, and it did
|
||||
# not used to be. It fired unconditionally, so a condition that
|
||||
# engages and clears repeatedly raised one notification PER
|
||||
@@ -384,11 +437,8 @@ class ClampWatchdog:
|
||||
# still written on every engaged cycle, and a genuine runaway -
|
||||
# which by definition keeps engaging - is picked up by the
|
||||
# re-assert path within REASSERT_S. Delay, not loss.
|
||||
if (self.last_alarm is None
|
||||
or (now - self.last_alarm).total_seconds() >= REASSERT_S):
|
||||
self.last_alarm = now
|
||||
self.spoken = True
|
||||
return True
|
||||
if self._due(self.last_alarm, now):
|
||||
return self._speak(now, self.bug)
|
||||
return False
|
||||
# ⚠️ THE ESCALATION. An episode that has not yet spoken because the
|
||||
# budget was spent by an EARLIER, unrelated episode speaks anyway
|
||||
@@ -396,15 +446,10 @@ class ClampWatchdog:
|
||||
# engages every cycle and reaches it; the scattered one-cycle
|
||||
# railing that produced the live flood never does.
|
||||
if not self.spoken and self.run_len >= SUSTAINED_CYCLES:
|
||||
self.last_alarm = now
|
||||
self.spoken = True
|
||||
return True
|
||||
return self._speak(now, self.bug)
|
||||
# Still engaged: re-assert on the slow cadence, not every cycle.
|
||||
if (self.last_alarm is None
|
||||
or (now - self.last_alarm).total_seconds() >= REASSERT_S):
|
||||
self.last_alarm = now
|
||||
self.spoken = True
|
||||
return True
|
||||
if self._due(self.last_alarm, now):
|
||||
return self._speak(now, self.bug)
|
||||
return False
|
||||
|
||||
self.run_len = 0
|
||||
@@ -420,5 +465,6 @@ class ClampWatchdog:
|
||||
# above. The episode ends; the notification budget does not reset
|
||||
# with it.
|
||||
self.reason = ""
|
||||
self.bug = False
|
||||
self.clean_cycles = 0
|
||||
return False
|
||||
|
||||
@@ -340,6 +340,12 @@ class Decision:
|
||||
# the comment at the bound below, and clamp.py.
|
||||
i_clamped: bool = False
|
||||
clamped: bool = False
|
||||
# SAFETY-40. The alarm's numbers, taken at the clamp so they are one tick's:
|
||||
# the bound the integrator reached, and the command before/after the
|
||||
# rating clamp. `target_w` is this tick's emitted command.
|
||||
i_bound_w: float = 0.0
|
||||
requested_w: float = 0.0
|
||||
clamped_w: float = 0.0
|
||||
|
||||
|
||||
def slew_limit_w(tuning: Tuning) -> float:
|
||||
@@ -563,6 +569,7 @@ def compute(
|
||||
# winding, "clamped" is a command that came out over the rating anyway.
|
||||
reason = "i-clamped"
|
||||
i_w = bounded
|
||||
i_bound_w = bounded if i_clamped else 0.0
|
||||
want = i_w
|
||||
|
||||
# ⚠️ Maintenance shaping (charge-only, cheap-window floor) used to live
|
||||
@@ -574,6 +581,7 @@ def compute(
|
||||
# An early draft applied a floor after the clamp and let demand escape it.
|
||||
target = max(-tuning.max_w, min(tuning.max_w, want))
|
||||
clamped = target != want
|
||||
clamped_w = target
|
||||
if clamped:
|
||||
reason = "clamped"
|
||||
|
||||
@@ -740,7 +748,8 @@ def compute(
|
||||
target = round(target / step) * step
|
||||
|
||||
return Decision(float(target), sat_count, frozen, reason, float(i_w),
|
||||
i_clamped, clamped)
|
||||
i_clamped, clamped, float(i_bound_w), float(want),
|
||||
float(clamped_w))
|
||||
|
||||
|
||||
def maintenance_charge_floor(
|
||||
|
||||
@@ -49,7 +49,8 @@ from .circulation import (
|
||||
circulating_w, opposing,
|
||||
)
|
||||
from .clamp import (
|
||||
REASON_CODE as CLAMP_REASON, ClampWatchdog, clamp_to_rating, is_commandable,
|
||||
BUG_SIGNAL as CLAMP_BUG_SIGNAL, REASON_CODE as CLAMP_REASON, ClampWatchdog,
|
||||
clamp_to_rating, is_commandable, loop_clamp_note,
|
||||
)
|
||||
from . import freshness
|
||||
from .freeze import (
|
||||
@@ -994,7 +995,8 @@ class Controller:
|
||||
# the counterpart to `clamp_last` moving onto the Inverter: the integrator
|
||||
# that wound is the SITE integrator, of which there is exactly one, while
|
||||
# the value a clamp bit is a value written to ONE device.
|
||||
self.loop_clamped: tuple[bool, str] = (False, "")
|
||||
# (engaged, note, over_rating) - SAFETY-40 splits the last one out.
|
||||
self.loop_clamped: tuple[bool, str, bool] = (False, "", False)
|
||||
self.stopping = False
|
||||
# SAFETY-61. time.monotonic() at the end of the last run_control() pass.
|
||||
self.loop_pass_mono = None
|
||||
@@ -3271,11 +3273,12 @@ class Controller:
|
||||
# it - so a string test is silent on the two cases a winding loop
|
||||
# most often presents as. These flags are set where the clamp
|
||||
# happens and nothing downstream touches them.
|
||||
# SAFETY-40: every number comes from THIS decision, at clamp time,
|
||||
# never from `self.target` after slew/freeze/arbiter rewrote it.
|
||||
self.loop_clamped = (
|
||||
decision.i_clamped or decision.clamped,
|
||||
(f"the loop asked for {decision.i_w:+.0f} W"
|
||||
+ (" (integrator bound)" if decision.i_clamped else "")
|
||||
+ (" (output bound)" if decision.clamped else "")),
|
||||
loop_clamp_note(decision),
|
||||
decision.clamped,
|
||||
)
|
||||
# SITE. compute() regulates `grid_w` - the P1 measurement - to
|
||||
# zero. Its subject is the meter, and §6.2's architecture is ONE
|
||||
@@ -3299,7 +3302,7 @@ class Controller:
|
||||
# A loop that is not running cannot be winding. Clearing this here
|
||||
# rather than letting the last value stand is what makes the alarm
|
||||
# de-assert when control is stopped.
|
||||
self.loop_clamped = (False, "")
|
||||
self.loop_clamped = (False, "", False)
|
||||
|
||||
# SAFETY-28. A new resolution is a new tick, and the previous record is
|
||||
# stale from this line onwards. Incremented BEFORE resolve() so that the
|
||||
@@ -3716,20 +3719,17 @@ class Controller:
|
||||
clamped_units = [inv for inv in self.inverters
|
||||
if inv.clamp_last is not None and inv.clamp_last.engaged]
|
||||
engaged_write = bool(clamped_units)
|
||||
engaged_loop, loop_note = self.loop_clamped
|
||||
engaged_loop, loop_note, loop_bug = self.loop_clamped
|
||||
engaged = engaged_write or engaged_loop
|
||||
if engaged:
|
||||
where = (", ".join(f"{inv.name}: {inv.clamp_last.describe()}"
|
||||
for inv in clamped_units) if engaged_write
|
||||
else f"{CLAMP_REASON}: {loop_note}, above the "
|
||||
f"{self.site_max_w:.0f} W rating, and "
|
||||
f"was clamped to {self.target:+.0f} W")
|
||||
reason = (f"{where}. This is a BUG SIGNAL, not a limit being used: "
|
||||
f"something upstream computed a setpoint the actuator "
|
||||
f"cannot deliver.")
|
||||
# SAFETY-40: only an over-rating is a bug; the integrator bound is not.
|
||||
bug = engaged_write or loop_bug
|
||||
if engaged_write:
|
||||
reason = (", ".join(f"{inv.name}: {inv.clamp_last.describe()}"
|
||||
for inv in clamped_units) + ". " + CLAMP_BUG_SIGNAL)
|
||||
else:
|
||||
reason = ""
|
||||
fired = self.clamp_wd.note(engaged, reason, datetime.now(timezone.utc))
|
||||
reason = loop_note if engaged else ""
|
||||
fired = self.clamp_wd.note(engaged, reason, datetime.now(timezone.utc),
|
||||
bug=bug)
|
||||
|
||||
# --- THE ACTION LOG, AND IT IS COALESCED PER EPISODE ------------------
|
||||
# Every engagement is recorded, but consecutive ones are recorded by
|
||||
@@ -3760,6 +3760,18 @@ class Controller:
|
||||
self.log_event(row)
|
||||
if not fired:
|
||||
return
|
||||
if not self.clamp_wd.bug:
|
||||
# SAFETY-40: the integrator bound is anti-windup, not a bug - a
|
||||
# WARNING and a plainly titled notice, same id and cadence.
|
||||
_LOG.warning("SAFETY-03 - %s Engagements so far: %d.",
|
||||
self.clamp_wd.reason, self.clamp_wd.engagements)
|
||||
await self.hass.call("persistent_notification", "create", {
|
||||
"notification_id": "goodwe_output_clamp",
|
||||
"title": "GoodWe: the control loop is at its integrator bound",
|
||||
"message": (f"{self.clamp_wd.reason}\n\nNo command exceeded "
|
||||
f"the rating and the controller is STILL RUNNING."),
|
||||
})
|
||||
return
|
||||
_LOG.error(
|
||||
"SAFETY-03 - %s Silently clipping this is how a winding integrator "
|
||||
"reached 14 768 W on this project before: the clamp makes every "
|
||||
@@ -4893,16 +4905,21 @@ class Controller:
|
||||
out.append({"ok": False, "warn": "NOT COMMANDABLE" not in note,
|
||||
"text": note})
|
||||
|
||||
# SAFETY-03. Red while a clamp is engaged, and the engagement COUNT is
|
||||
# shown even after it clears - a loop that wound once and stopped is
|
||||
# still a bug, and a row that vanishes with the condition takes the
|
||||
# SAFETY-03. Red while an over-rating clamp is engaged (amber for the
|
||||
# integrator bound, SAFETY-40), and the engagement COUNT is shown even
|
||||
# after it clears - a row that vanishes with the condition takes the
|
||||
# evidence with it. Reported only when there is something to report:
|
||||
# unlike the freeze detector this guard has no "not covered" state, it
|
||||
# runs on every write on every transport.
|
||||
if self.clamp_wd.active:
|
||||
if self.clamp_wd.active and self.clamp_wd.bug:
|
||||
out.append({"ok": False, "warn": False,
|
||||
"text": "OUTPUT CLAMP ENGAGED - " + self.clamp_wd.reason
|
||||
+ f" Engagements: {self.clamp_wd.engagements}."})
|
||||
elif self.clamp_wd.active:
|
||||
# SAFETY-40: the integrator bound alone is amber, not red.
|
||||
out.append({"ok": False, "warn": True,
|
||||
"text": "LOOP AT INTEGRATOR BOUND - " + self.clamp_wd.reason
|
||||
+ f" Engagements: {self.clamp_wd.engagements}."})
|
||||
elif self.clamp_wd.engagements:
|
||||
out.append({"ok": False, "warn": True,
|
||||
"text": f"output clamp: not engaged now, but it has "
|
||||
|
||||
Reference in New Issue
Block a user