Files
goodwe-addon/goodwe_controller/DECISIONS/SAFETY-13-bounding-the-output-freeze.md
glenn schrooyenandClaude Opus 5 f156fadac0
checks / unit (pull_request) Successful in 5m34s
SAFETY-13: the bound came from the wrong half of a two-sided measurement
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
2026-09-06 18:32:03 +02:00

21 KiB
Raw Permalink Blame History

SAFETY-13 — the output freeze gets an exit, and only where it cannot end on its own

Status: decided and implemented on branch SAFETY-13, off release/1.0 @ c8dcb38. Card: SAFETY-13 — "Bound the no-start-while-saturated output block" (Trello). Nothing here reached the inverter. Every figure below is from the pure control law run in a container against a plant model, on this branch and on a pristine c8dcb38 for the before-figures. No rig, no ESP32, no Home Assistant.


0. The four things a reader should take away

  1. The card's headline case is real, and the route to it is not the obvious one. The block does not engage on the way down into an export; the slew limit moves the command off zero before the counter reaches 3. It engages from the deadband — a site resting at its target with another writer on the bus, counting saturated cycles while nothing is wrong — and 0 W is then a fixed point. §2.
  2. prev_w == 0 is not a special case, so the fix is not one either. The same run pins at −800 W against its own integrator standing at −2388 W, at a non-zero prev_w, with the same absence of an exit. Zero is only the instance where the pinned magnitude is the whole of what the battery could have done. §3.
  3. The decision is not "how long", it is "which". A divergence a limit could have produced clears itself and gets no exit — release/1.0 unchanged. A divergence a limit could not have produced may not, and gets one. The predicate is physical, not statistical — and it is not a second-writer detector; §4.1 and §8 say what it can and cannot tell apart.
  4. Releasing the output freeze does not unfreeze the loop, and that is what makes the release bounded rather than merely different. §6.
  5. The bound comes from the SLOWER half of a two-sided measurement, and the first version of this branch took the faster half. §8 is the correction and the measurement behind it; it is the reason SETTLE_CYCLES is 12 and not 9.

1. What was there, in the terms the code uses

control.py, in compute(), after the clamp and the slew limit:

if frozen:
    target = min(target, prev_w) if prev_w > 0 else max(target, prev_w)
    reason = "saturated-freeze"

"Magnitude may fall, never rise." At prev_w == 0 the magnitude is zero and max(target, 0) forbids any negative command, so a charge cannot start.

The reason is written in the file and it is a good one: frozen means the inverter's reported power has diverged from the command by more than saturation_w for saturation_cycles consecutive cycles, and at a command of zero that means the actuator is producing power nobody in this add-on asked for. Starting to push the other way into it is the configuration §1.5 of the specification rules out — "any configuration in which two writers can reach one actuator".

The defect is that it has no end. Not the error growing, not the integrator moving, not time. sat_count stops rising at its cap and the state is a fixed point.


2. Reproducing it — and correcting the route

Driven at the live install's shape: max_w 5000, slew_w 400, target_grid_w -10, saturation_w 500, saturation_cycles 3, gain 0.6. Another writer holds the pack at +700 W whatever is commanded.

The route that does NOT reach it. Start at prev_w = 0 with 2 kW of export already standing. On c8dcb38 the loop moves off zero at the slew limit before the counter reaches 3 — commands −400, −800 — so by the time frozen is true prev_w is −800 and the block that gets applied is max(target, −800). The site charges at 800 W, not 0 W, for as long as the run lasts.

The route that does. The site is resting. grid == target_grid_w, so the loop is in its deadband and the command is 0 — and saturated_now is computed before the deadband branch, so the counter climbs anyway. Three cycles later the loop is frozen at prev_w == 0 with nothing wrong. Then the export arrives:

cycle grid actual prev command integrator sat reason
0–1 −10 +700 0 0 0 1–2 deadband
2–5 −10 +700 0 0 0 3–6 saturated-freeze
6 −2000 +700 0 0 −1194 7 saturated-freeze
7… −2000 +700 0 0 −1194 8→10 saturated-freeze

On c8dcb38 the command is 0.0 on every cycle from there on. The integrator has moved to −1194 W and stopped (its own freeze holds it), the site exports 2 kW, and the battery is idle until the other writer leaves.

That is the card's sentence — "the site can sit exporting 2 kW with an idle battery indefinitely" — reproduced, with the route corrected: it is entered from the deadband, not from a correction that ran out of room.


3. …and prev_w == 0 is not where it stops

The same tuning, same writer, but the loop starts from zero with the export already standing (the route from §2 that "does not reach it"):

c8dcb38
command after the freeze engages −800 W, on every cycle
the loop's integrator, frozen −2388 W
what ends it nothing

So the missing exit is a property of the output freeze, not of the zero case. prev_w == 0 is the instance where the pinned magnitude happens to be zero. A rule written for zero alone would have left −800 W exactly as unbounded as it found it — which is why the change below is written against the freeze and not against a special case of it.

And this is the half that escalates at two inverters. actual_w is site_batt_w, the SUM (ALLOC-01, DEV-02), so the comparison is a site command against two units' power. One unit still moving after the other has stopped is enough to put the site into the freeze with no external actor at all.

⚠️ What that claim is and is not. The escalation is stated from the code — prev_w is self.target, the site total, and actual_w is the site sum — and from the single-unit runs above. It is not measured on two inverters. Inverter B does not physically exist (NOTES.md §1.1b: on paper, from datasheet and serial), and sim/plantsim.py:216 INVERTER_B is INVERTER_A copied with measured=False. Any two-inverter number would be two copies of A.


4. The decision: which freezes get an exit, not how long the block lasts

Three answers were on the table, and the card named them: lift after N frozen cycles, exempt above some export magnitude, or keep it unbounded and write down the cost.

A plain cycle bound was rejected, because it bounds the wrong thing. The freeze is right to be permanent in one case and cannot be permanent in the other, and a single N treats them alike:

the inverter is… can the freeze clear itself?
doing less than the command, in the command's own direction — a cold pack, a curtailment, an export cap, the plant still ramping Yes. The command is pinned at something the inverter can reach, so it catches up, saturated_now goes false, and the freeze ends. Asking for more would achieve nothing anyway.
doing something outside what the command can explain — overshooting it, or the opposite sign No. Clearing the freeze requires the inverter to track our command, and the other writer is precisely what stops that.

An export-magnitude exemption was rejected for a narrower reason: it makes the block's end depend on how big the other actor's disturbance happens to be, which is not a property of the thing the block is about.

So the rule implemented is:

The output freeze applies as before, unless the inverter's reported power cannot be explained by the command not being met and the divergence has outlasted the plant that would otherwise have explained it.

4.1 The predicate is physical

unasked_power(prev_w, actual_w, saturation_w). A limit can only make the inverter do less than the command, in the command's own direction. It cannot make it overshoot the command and it cannot make it produce the opposite sign. So everything a limit can produce lies in the closed interval spanned by 0 and prev_w, widened by saturation_w for noise, ramp lag and idle draw.

⚠️ What it does NOT establish is that anything outside was put there by something else. The first version of this branch said exactly that, in the docstring, in control.py and in DOCS.md, and it is false: the predicate compares against one cycle's command while the pack carries every command before it, so our own coast-down through zero lands outside our own interval. §8 measures it. What the predicate guarantees is only its own first sentence — this cycle's command does not account for this cycle's reading — and the time bound below is what turns that into a reason to act.

prev actual interval verdict
+2000 +700 [0, 2000] a limit
−5000 −700 [−5000, 0] a limit
0 +700 [−500, +500] unasked
−800 +700 [−1300, +500] unasked

⚠️ At prev_w == 0 this predicate and the saturation detector are the same test — asserted in test_control.py over 801 readings. That is not a coincidence to tidy away: with nothing commanded there is no interval left for a limit to live in, which is exactly why the zero case is the one that strands.

⚠️ It is not a detector and nothing alarms on it. A stale, wrong-signed or noisy reading lands outside the interval too. All it decides is whether the freeze gets an exit; being wrong about it costs a bounded command (§6). 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.

4.2 The bound, and which half of the measurement it comes from

SETTLE_CYCLES = ceil(PLANT_SETTLE_DOWN_S / LOOP_TICK_S) = 12.

⚠️ PLANT_SETTLE_DOWN_S is new, and adding it is the point rather than a cost. NOTES.md §5.10 did not measure a settling time; it measured the same 1.78 kW step in both directions — "Up: ~18 s from step to deadband. Down: ~23 s from a −1784 W export excursion" (NOTES.md:1550). So the plant's settling time is a pair, and which half is the safe one depends on which way being wrong hurts:

the bound the question the safe half why
SAFETY-39's SLEW_RAIL_FRAC may this configuration be refused? 18 s, the shorter a looser bound refuses fewer working configurations
this one may the plant still be the explanation? 23 s, the longer a shorter bound stops believing the plant too early, and then it accuses

The first version of this branch reused PLANT_SETTLE_S and got 9 — three cycles short of the plant's own measured down-ramp, on the side that produces a false positive. §8 is the measurement and the correction.

⚠️ It is no longer the reciprocal of SLEW_RAIL_FRAC, and it never should have been read as one. The two were equal only while both were being derived from the same half of the pair, and that coincidence read as a derivation. They are 9 and 12 now; test_control.py asserts the inequality so they cannot be quietly re-fused.

⚠️ It inherits SAFETY-39's caveat about LOOP_TICK_S, in the opposite direction. A meter slower than 2 s makes 12 cycles cover more than 23 s, so the freeze holds longer than the plant needs: that costs energy, never safety. A meter faster makes them cover less, so the freeze can release while the plant is still moving — bounded by §6 rather than by luck.

_SAT_COUNT_CAP moved with it, from 10 to 13, because it is written as SETTLE_CYCLES + 1 — a counter capped below the longest bound read from it would silently delete the exit. That has a consequence worth stating on its own: config.yaml accepts saturation_cycles as int(1,10), so at the old bound of 9 a legal setting could sit above the exit and delete the block. At 12 the whole accepted range is strictly below the bound, and the degenerate configuration this branch used to document no longer exists. §7.


5. What actually changed, graded against release/1.0

test_control.py's sweep still compares this law against a transcription of release/1.0 — the transcription was not taught the new rule, because an oracle taught the change compares the law against itself. What changed is the assertion: every case must be identical, or be a release with both of its conditions true, or differ in the saturation counter alone.

⚠️ The sweep had to be widened to keep grading anything. Its sat_count fixtures ran to 9; with the bound at 12 the counter never reached the exit and every release silently vanished from the table, which is what the assertion on the count caught. It runs to 12 now, at 3780 cases.

cases
total 3780
bit-identical to release/1.0 3267
releases 432
…of those, reason-only (the freeze was not binding) 405
…of those, the command changes 27
differ in the saturation counter alone 81

All 27 command changes are at prev_w == 0, all with release/1.0 commanding exactly 0.0 W, and every new command is negative — a charge start that was refused.

⚠️ The 81 are the counter's ceiling moving, and they reach nothing. _SAT_COUNT_CAP went from 10 to 13 with the bound, so above release/1.0's own ceiling the two counters differ. Nothing else may differ in those cases, and the sweep asserts that: frozen is sat_count >= saturation_cycles, config.yaml accepts that at most 10, and no other line in compute() reads the counter's value — so a counter reading 13 instead of 10 cannot change what is commanded. The 81 all sit at an sc above 10, which is a state release/1.0 could never hold at all.

The behaviour after the fix, on §2's run: the command holds at 0 W through sat_count 3–11, releases on the twelfth saturated cycle, and walks 0 → −400 → −800 → −1190 W, where it stops — the same settling point as at the old bound, three cycles later.

The genuine-limit case, driven for 30 cycles with the inverter curtailed to ±1000 W against a 4 kW import: freeze-released never appears and the command stays pinned at +2400 W. Unchanged, deliberately.


6. Why the release cannot become windup (the card's fourth criterion)

The integrator freeze is not lifted with the output freeze. The command is derived from i_w, then clamped, then slew-limited from prev_w — so a released cycle can only move the command toward an accumulator that stopped accumulating when the freeze engaged, by at most one slew step, and never past it. test_control.py sweeps 107 625 states, of which 67 500 release: no released case leaves the interval between the previous command and the frozen integrator, and none moves further than one slew step.

⚠️ 107 625 was previously reported as the number of released states. It is the number of states swept, and the assertion only ever graded the subset that released. Both numbers are counted and printed now.

On §2's run that shows up as the settling point: −1190 W against a frozen integrator of −1194 W, not the 5000 W rail. The tech lead's note on SAFETY-04 — that the carried i_w and release/1.0's recomputed value land on the same slew-limited command — is preserved: the i_w=None path is still graded against the transcription, cycle for cycle, everywhere except the 432 releases.

⚠️ The first form of this check was wrong and the sweep said so, which is left in the test file. It asserted |target| <= |i_w| on a released cycle and produced prev −5000, i_w −3000, target −4600: the command was walking back from −5000 toward the integrator and had not arrived. It was moving the right way and the assertion still failed, because "never exceeds the integrator" is not what no-windup means here.


7. What this does NOT buy, said plainly

The site still does not reach its target while another writer is on the bus. It settles at the frozen integrator, not at the meter's target. What the exit buys is that the battery stops being idle while the site exports; it does not buy correct regulation against an actuator somebody else is also writing to, and nothing in a rules engine can. A standing grid error with a second writer present is the symptom of a configuration this add-on refuses to support.

freeze-released is a place to start looking, not a finding. It says the divergence outlasted the plant's own measured settling time, and that is all it says — §8. DOCS.md now tells the operator the innocent causes first (a large load or PV swing, a plant slower than the one that was measured) and the second writer only when the state stands. It is carried on Decision.reason, which nothing after the freeze overwrites. No new alarm was added: SAFETY-41 removed one that narrated normal anti-windup, and this state is not yet known to be worth waking anybody for.

No accepted configuration removes the block any more. config.yaml accepts saturation_cycles as int(1,10); the bound is 12, so every accepted value is strictly below it and the freeze always engages before its exit is due. This paragraph used to say the opposite — at a bound of 9, settings of 9 and 10 deleted the block silently and the branch documented that as a known consequence. Moving the bound closed it by construction. test_control.py pins it against config.yaml's own schema line rather than a transcription of it.

Nothing was measured on two inverters. See the warning in §3.


8. ⚠️ ROUND 2: the bound was derived from the wrong half, and it accused

What was wrong. SETTLE_CYCLES was ceil(PLANT_SETTLE_S / LOOP_TICK_S) = 9. PLANT_SETTLE_S is 18 s, the up-ramp of NOTES.md §5.10's kettle step; the same measurement recorded ~23 s coming back down (NOTES.md:1550). SAFETY-39 takes 18 correctly — its bound is a refusal, and the looser number refuses less. This bound asks the opposite question, and the looser number stops believing the plant three cycles too early.

Why that is not a tuning quibble. The predicate has no memory. prev_w is one cycle's command; the pack carries every command before it. A command that falls through zero — the ordinary end of a load or PV step — leaves the pack coasting in the old direction, so the reading has the sign the command used to have and lands outside the interval spanned by 0 and the new prev_w. unasked_power() calls that "unasked". With the bound three cycles short of the plant's own down-ramp, the counter reaches it while the coast-down is still going, and the law emits freeze-released on one inverter with nothing else on the bus.

Measured, in the container, N = 1. One compute(), one plant model, one command; no second writer anywhere in the harness. The plant is a first-order lag whose two time constants come from the two measured settling times (tau = t/3, since a first-order step is 95 % done at 3 tau) with sim/plantsim.py's measured 800 W/s ramp cap. It is a model, and it is labelled one wherever it is used.

bound 9 bound 12
1280 closed-loop scenarios (a grid of plant pairs, slews, saturation_cycles, load steps) 113 fire 24 fire
272 load-step pairs at the measured 18 s / 23 s plant, slew_w 400 62 fire 2 fire

The single case now pinned in test_control.py: a 4 kW export falling to 2.5 kW. At cycle 34 the command is −310 W, the pack is still at −1343.4 W coasting down from −4860 W a few cycles earlier, unasked_power(−310, −1343.4, 500) is True, sat_count is 9, and the law says freeze-released. At 12 the same run never releases.

What the fix does not fix. The bound makes the false positive rare on the plant that was measured. It cannot make it impossible, because it is a measurement and the plant can be slower than the measurement. Same sweep, bound held at 12, lengthening only the plant's down-ramp: 8 of 320 scenarios at the measured 23 s, 59 of 320 at 34 s, 268 of 320 at 50 s. That residual is why the three statements below were corrected rather than merely re-numbered.

Three shipped statements were falsified by this and are now rewritten:

  1. unasked_power()'s docstring — "Anything outside that interval was put there by something other than this loop." It was our own power. The docstring now says what the predicate does guarantee (this cycle's command does not account for this cycle's reading) and states the coast-down as its known blind spot.
  2. control.py, in compute() — "SO THE BOUND BELONGS TO (b) AND ONLY TO (b)". It fired in case (a). The comment now says the bound is a reason to stop believing (a), not a proof of (b), and that the release is written to be survivable when the inference is wrong.
  3. DOCS.md — "If you see freeze-released in the action log, look for the second writer." This is the one that reaches a person, and on a single-inverter install it sent them hunting something that does not exist. Replaced by a section that gives the innocent causes first and the second writer only when the state stands.