admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

SAFETY-13: the bound came from the wrong half of a two-sided measurement
checks / unit (pull_request) Successful in 5m34s
Details

Browse Source 
NOTES.md 5.10 measured the 1.78 kW step BOTH ways - "Up: ~18 s from step to
deadband. Down: ~23 s" (NOTES.md:1550). SAFETY-39 takes 18 correctly: its
bound is a REFUSAL and the looser number refuses less. This branch reused
PLANT_SETTLE_S for the opposite job - how long the plant may still be the
EXPLANATION for a divergence - where the conservative half is the longer one.
ceil(23/2) = 12, not 9, and being short there does not refuse anything, it
accuses.

The false positive is real and is measured here. One inverter, one compute(),
one plant, no second writer anywhere in the harness: the command falls through
zero while the pack is still coasting the old way, unasked_power() calls our
own power "unasked", the counter reaches 9 while it is still coasting, and the
law emits "freeze-released". At the plant's own measured 18 s/23 s shape, over
272 load-step pairs, the 9-cycle bound fires in 62 and the 12-cycle bound in 2.

  PLANT_SETTLE_DOWN_S = 23.0 is added beside PLANT_SETTLE_S = 18.0, because
  the plant's settling time is a PAIR and each bound has to say which half it
  means and why.
  SETTLE_CYCLES = ceil(PLANT_SETTLE_DOWN_S / LOOP_TICK_S) = 12. It is no
  longer the reciprocal of SLEW_RAIL_FRAC; that was a coincidence of both
  reading the same half, and test_control.py now asserts the inequality.
  _SAT_COUNT_CAP moves with it, 10 -> 13. config.yaml accepts
  saturation_cycles as int(1,10), so the whole accepted range is now strictly
  BELOW the bound and the degenerate configuration this branch used to
  document no longer exists - pinned against config.yaml's own schema line.

Three shipped statements were falsified by that measurement and are rewritten,
not renumbered - at 12 the residual is smaller, not zero:

  unasked_power()'s docstring said anything outside the interval "was put
  there by something other than this loop". It was our own power: eight cycles
  after a +1990 W command the command is 0 and the pack still reads +522.3 W.
  control.py said "SO THE BOUND BELONGS TO (b) AND ONLY TO (b)". It fired in
  case (a).
  DOCS.md told an operator "if you see freeze-released, look for the second
  writer" - on a single-inverter install, hunting something that does not
  exist. Replaced by a section that gives the innocent causes first (a large
  load or PV swing; a plant slower than the one measured, 8/320 scenarios at
  23 s rising to 268/320 at 50 s) and the second writer only when the state
  STANDS.

Round 1's own results are kept and re-graded, not weakened. The release/1.0
sweep had to widen to reach the new bound (3780 cases): 3267 bit-identical,
432 releases, 27 command changes - all still charge starts from prev_w == 0 -
and 81 differing in the saturation counter alone, which nothing downstream can
read. The card's case releases on the twelfth saturated cycle instead of the
ninth and settles at the same -1190 W against the same frozen integrator.

Also corrected, because they were claims and did not reproduce: the coverage
table's "output freeze" row was 11, and the mutation form its own note
describes gives 6 on this tree, 6 on the first SAFETY-13 commit and 2 on
release/1.0; the detector row was left at 11 and is 15; the frozen = False
footnote was left at 10 and is 13. And section 6 reported 107 625 as "released
states" when it is the number of states SWEPT - 67 500 of them release, and
both are counted now.

Nothing here reached the inverter. 22/22 test files pass in goodwe-addon-ci,
tree rsynced to a plain path, mounted :ro, python3 -B, __pycache__ purged and
printed first (0 dirs / 0 .pyc; the worktree carries none).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
glenn schrooyen
2026-09-06 18:32:03 +02:00
co-authored by Claude Opus 5
parent 6c4565659b
commit f156fadac0
5 changed files with 648 additions and 150 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/control.py
+130 -37
View File
@@ -66,6 +66,17 @@ from dataclasses import dataclass
# validate.py refuse the impossible case only, never "close
# to the rail".
#
# ⚠️ AND THAT CHOICE IS LOCAL TO THIS BOUND. §5.10 MEASURED THE STEP IN BOTH
# DIRECTIONS, so "the plant's settling time" is a PAIR and not a number, and
# which half is the safe one depends on which way being wrong hurts. Taking
# 18 for a REFUSAL is right: the looser number refuses less. Taking 18 for a
# question of the form "how long does the plant remain a candidate
# explanation" is WRONG, because the looser number stops believing the plant
# too early - and there the answer is a false accusation. SAFETY-13 asks that
# second question and so reads PLANT_SETTLE_DOWN_S below, deliberately not
# this constant. Anything else added here has to say which of the two it
# means and why.
#
# LOOP_TICK_S is the METER, not this loop. `run_control()` runs a cycle on
# a changed meter value or on `cycle_floor_s`, whichever comes
# first, so the tick is the meter's own update rate. 2.0 s is
@@ -87,6 +98,13 @@ PLANT_SETTLE_S = 18.0
LOOP_TICK_S = 2.0
SLEW_RAIL_FRAC = LOOP_TICK_S / PLANT_SETTLE_S # 0.111..., i.e. 9 ticks
# The other half of the same measurement: the descent of the same 1.78 kW step,
# ~23 s from a -1784 W export excursion back into the deadband (goodwe/NOTES.md
# §5.10, the same run, the same day, the same plug). It is slower than the ramp
# up, which is the ordinary asymmetry of a pack that is pushed one way and
# coasts the other. Nothing above this line reads it; SAFETY-13 below does.
PLANT_SETTLE_DOWN_S = 23.0
# ---------------------------------------------------------------------------
# SAFETY-13. HOW LONG THE PLANT STAYS A CANDIDATE EXPLANATION.
#
@@ -100,42 +118,78 @@ SLEW_RAIL_FRAC = LOOP_TICK_S / PLANT_SETTLE_S # 0.111..., i.e. 9 ticks
# at something the inverter can do, so the inverter catches up to it and
# `saturated_now` goes false.
#
# ⚠️ AND (a) INCLUDES THE CASE `unasked_power()` BELOW CANNOT SEE AS (a):
# the command has fallen THROUGH ZERO and the pack is still coasting in
# the old direction, so the reading has the sign the command used to have
# and not the one it has now. That is this loop's own power, arriving
# late. It is the reason the time bound is what does the work here and
# the predicate alone is not enough.
#
# (b) SOMETHING ELSE IS DRIVING THE ACTUATOR - the §1.5 non-goal, two writers
# on one actuator. The freeze CANNOT clear itself here, because clearing
# it needs the inverter to track OUR command and the other writer is what
# stops it. Nothing ends this but the other writer going away.
#
# ⚠️ SO THE BOUND BELONGS TO (b) AND ONLY TO (b), and this constant is what
# separates them in TIME rather than by guessing. Case (a) is bounded by the
# plant: PLANT_SETTLE_S is how long this plant takes to settle, measured, and
# after that the plant has run out of alibi. A divergence still standing after
# PLANT_SETTLE_S is not this plant catching up.
# ⚠️ SO THE BOUND IS ONLY EVER A REASON TO STOP BELIEVING (a). It does not
# establish (b) and nothing below claims it does: what it says is that the
# plant has run out of alibi, and (b) is then the only explanation this loop
# has left - not the only one there is. Everything downstream of the release is
# written to be safe when that inference is wrong, because sometimes it is.
#
# SETTLE_CYCLES = ceil(PLANT_SETTLE_S / LOOP_TICK_S)
# Case (a) is bounded by the plant, and the plant was measured:
#
# ⚠️ THE SAME TWO MEASURED NUMBERS SAFETY-39 USES, AND DELIBERATELY NOT A NEW
# ONE. It is the reciprocal of SLEW_RAIL_FRAC - the slew limit takes exactly
# this many ticks to traverse the rail, for exactly this reason. A new constant
# here would be a second thing to re-derive when the plant is re-measured, and
# a number that outlives its derivation is the defect SAFETY-39 was about.
# SETTLE_CYCLES = ceil(PLANT_SETTLE_DOWN_S / LOOP_TICK_S)
#
# ⚠️ AND IT INHERITS SAFETY-39's CAVEAT ABOUT LOOP_TICK_S, in the opposite
# ⚠️ THE **DOWN** NUMBER, AND THE DIRECTION IS THE WHOLE POINT. §5.10 measured
# the same step both ways - 18 s up, 23 s down - and SAFETY-39 takes 18 because
# 18 is the LOOSER bound for a REFUSAL. This is the opposite job. Here the
# question is "how long may the plant still be the explanation", and the
# conservative answer is the LONGER of the two: believe the plant for as long
# as the plant has ever been observed to take. Deriving this from 18 gives 9,
# which is three cycles SHORT of the plant's own measured down-ramp - and being
# short here does not refuse anything, it ACCUSES. Measured, single inverter,
# no second writer anywhere, 1280 closed-loop scenarios: at 9 the exit fires in
# 113 of them and at 12 in 24. The direction of that error is why the pair is
# now two named constants instead of one.
#
# ⚠️ AND IT IS NO LONGER THE RECIPROCAL OF SLEW_RAIL_FRAC. It was, at 9, and
# that coincidence read as a derivation - two different questions that happened
# to share an answer while both were being asked of the same half of a two-
# sided measurement. They are 9 and 12 now. If they ever coincide again it is
# arithmetic, not agreement.
#
# ⚠️ IT INHERITS SAFETY-39's CAVEAT ABOUT LOOP_TICK_S, in the opposite
# direction, which is why it is repeated rather than cross-referenced. A meter
# SLOWER than 2 s makes these 9 cycles cover MORE than 18 s, so the freeze
# SLOWER than 2 s makes these 12 cycles cover MORE than 23 s, so the freeze
# holds longer than the plant needs: that costs energy - see the case in
# DOCS.md - and never safety. A meter FASTER than 2 s makes them cover LESS,
# so the freeze can release while the plant is genuinely still moving. That one
# is bounded twice over and by neither this constant nor luck: the INTEGRATOR
# freeze is not released with the output freeze, so the command can only reach
# the integrator's frozen value, and the output clamp bounds that in turn.
SETTLE_CYCLES = math.ceil(PLANT_SETTLE_S / LOOP_TICK_S) # 9
#
# ⚠️ AND A PLANT SLOWER THAN THE ONE THAT WAS MEASURED DOES THE SAME THING,
# which no constant derived from a measurement can fix. In the same 1280
# scenarios, holding the bound at 12 and lengthening only the plant's down-ramp
# past the measured 23 s, the exit starts firing again on a single inverter:
# 8/320 at 23 s, 59/320 at 34 s, 268/320 at 50 s. A re-measured plant means a
# re-derived constant here, exactly as for the two above it.
SETTLE_CYCLES = math.ceil(PLANT_SETTLE_DOWN_S / LOOP_TICK_S) # 12
# The saturation counter's ceiling. It has always been 10 and still is; it is
# written against SETTLE_CYCLES rather than as a literal so that a counter
# capped BELOW the longest bound read from it - which would silently make that
# bound unreachable, i.e. delete the exit condition - cannot happen by editing
# one of the two.
_SAT_COUNT_CAP = SETTLE_CYCLES + 1 # 10
# The saturation counter's ceiling, one above the longest bound read from it. A
# counter capped BELOW that bound would silently make the bound unreachable -
# i.e. delete the exit condition - so it is written against SETTLE_CYCLES and
# not as a literal, and it moves when SETTLE_CYCLES moves.
#
# ⚠️ IT WAS 10, WHICH IS ALSO config.yaml's MAXIMUM FOR `saturation_cycles`,
# AND THAT COLLISION IS GONE. At a bound of 9 the largest accepted
# `saturation_cycles` (10) sat ABOVE the exit, so a legal configuration could
# push the freeze's engagement past its own release and the no-start block
# stopped existing. At 12 every accepted value 1..10 is strictly below the
# bound, so the freeze always engages before its exit is due and no accepted
# configuration can delete the block. `test_control.py` pins that at the schema
# edge rather than in prose, because it is a property of two files agreeing.
_SAT_COUNT_CAP = SETTLE_CYCLES + 1 # 13
@dataclass(frozen=True)
@@ -237,11 +291,11 @@ def slew_limit_w(tuning: Tuning) -> float:
def unasked_power(prev_w: float, actual_w: float, saturation_w: float) -> bool:
"""True when `actual_w` cannot be explained by the command not being met.
"""True when `actual_w` cannot be explained by THIS cycle's command not being met.
SAFETY-13. The one question the saturation magnitude test cannot answer:
is the inverter FAILING TO REACH what we asked, or is it doing something we
never asked for at all?
is the inverter FAILING TO REACH what we asked, or is it doing something
the command in front of it does not account for?
⚠️ THE ANSWER IS PHYSICAL, NOT STATISTICAL, WHICH IS WHY IT IS A PREDICATE
AND NOT A THRESHOLD. A limit can only make the inverter do LESS than the
@@ -249,8 +303,28 @@ def unasked_power(prev_w: float, actual_w: float, saturation_w: float) -> bool:
command, and it cannot make it produce power of the OPPOSITE sign. So the
whole set of readings a limit can produce is the closed interval spanned by
0 and `prev_w`, widened by `saturation_w` for measurement noise, ramp lag
and the inverter's own idle draw. Anything outside that interval was put
there by something other than this loop.
and the inverter's own idle draw.
⚠️ WHAT IT DOES NOT SAY - AND AN EARLIER VERSION OF THIS DOCSTRING DID -
IS THAT ANYTHING OUTSIDE THE INTERVAL WAS PUT THERE BY SOMETHING OTHER THAN
THIS LOOP. THAT IS FALSE AND IT WAS MEASURED FALSE. This predicate has no
memory: `prev_w` is one cycle's command, while the plant carries every
command before it. Drive ONE inverter, with nothing else on the bus, from a
discharge down through zero, and the pack is still coasting in the old
direction after the command has crossed. Measured in the container, one
`compute()` against one plant: eight cycles after a +1990 W command the
command is 0 and the pack still reads +522.3 W, so
`unasked_power(0, 522.3, 500)` is True - and every watt of that 522.3 was
asked for by this loop. In 1280 single-inverter closed-loop scenarios with
no second writer in them at all, the exit built on this predicate fires in
24 of them.
So what it guarantees is exactly the sentence at the top: this cycle's
command does not account for this cycle's reading. What it CANNOT do is
tell "somebody else is writing" from "we are". That gap is the reason
SETTLE_CYCLES is derived from the plant's slower direction, why the caller
checks both, and why the reason string it produces is a place to start
looking rather than a finding.
Worked, at `saturation_w = 500`:
@@ -262,16 +336,19 @@ def unasked_power(prev_w: float, actual_w: float, saturation_w: float) -> bool:
⚠️ AT `prev_w == 0` THIS IS EXACTLY `saturated_now`, and that is not a
coincidence to be tidied away - it is the reason the freeze's zero case is
the one the card was written about. With nothing commanded there is no
interval left for a limit to live in, so every saturated cycle at zero is a
cycle in which the actuator is being driven by someone else.
interval left for a LIMIT to live in, so at zero this test cannot be
answered by "the inverter is not reaching what we asked" at all. It can
still be answered by "the inverter has not finished leaving what we asked
for LAST time", which is the coast-down above and is why zero needs the
time bound as much as any other `prev_w` does.
⚠️ IT IS NOT A DETECTOR AND MUST NOT BE ALARMED ON. A saturated inverter
whose reading is stale, wrong-signed or simply noisy lands outside the
interval too. All this predicate decides is whether the output freeze gets
an EXIT; being wrong about it costs at most a bounded command that the
frozen integrator and the output clamp both still bound. SAFETY-30's
`unit_sign_disagree_w` is the thing that alarms on sign, and it is
deliberately a different test with a different threshold.
interval too, and so does our own ramp, above. All this predicate decides
is whether the output freeze gets an EXIT; being wrong about it costs at
most a bounded command that the frozen integrator and the output clamp both
still bound. SAFETY-30's `unit_sign_disagree_w` is the thing that alarms on
sign, and it is deliberately a different test with a different threshold.
"""
lo = min(0.0, float(prev_w)) - abs(float(saturation_w))
hi = max(0.0, float(prev_w)) + abs(float(saturation_w))
@@ -465,11 +542,27 @@ def compute(
# END ON ITS OWN. See SETTLE_CYCLES and `unasked_power()` at the top:
# a divergence a LIMIT could produce clears itself once the inverter
# catches up to the pinned command, so it needs no exit and gets none -
# unbounded, exactly as shipped. A divergence a limit CANNOT produce is
# another writer, which by construction stops our command ever being
# tracked, so it can only end when the other writer leaves. That is the
# one this releases, and only after the divergence has outlasted the
# plant that would otherwise have explained it.
# unbounded, exactly as shipped. A divergence a limit CANNOT produce
# does not clear itself by being waited on, so it gets one.
#
# ⚠️ AND THAT IS NOT THE SAME SENTENCE AS "THE OTHER WRITER". An
# earlier version of this comment said the two conditions below fire
# only in case (b); they do not, and the sweep says so. Our own plant
# coasting through a command that has just crossed zero satisfies
# `unasked_power()` too, because that predicate compares against ONE
# cycle's command and the pack remembers more than one - measured, a
# single inverter with nothing else on the bus reaching this branch in
# 24 of 1280 closed-loop scenarios. `SETTLE_CYCLES` is what keeps that
# rare rather than routine (113 of the same 1280 at the old bound of
# 9), and it does it by believing the plant for as long as the plant
# has ever been measured to take. It is a bound on how long we wait,
# not a proof about what we found.
#
# ⚠️ SO THE RELEASE IS WRITTEN TO BE SURVIVABLE WHEN IT IS WRONG, and
# that is the paragraph below, not this one: what a false release costs
# is one slew step per cycle toward a frozen integrator. If it ever has
# to cost less than that, the thing to change is what happens after the
# release, not the confidence of the sentence before it.
#
# ⚠️ RELEASING IS NOT UNFREEZING. The INTEGRATOR freeze above is not
# lifted, and that is what makes this bounded rather than merely