From 6c4565659b2c8be1cc1c4dddb7ebcee30965dd4a Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Sun, 6 Sep 2026 13:56:26 +0200 Subject: [PATCH] SAFETY-13: the output freeze gets an exit, and only where it needs one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the loop is frozen the command's magnitude may fall but never rise. At prev_w == 0 there is no magnitude left to fall, so a charge cannot START. That refusal is deliberate and it is kept: the inverter reporting power we did not command means another writer is on the actuator, which is the §1.5 non-goal. What it did not have was an END - nothing cleared it but the other writer leaving. Reproduced on release/1.0 @ c8dcb38, at the live install's shape (max_w 5000, slew_w 400, target_grid_w -10), with another writer holding the pack at +700 W. The route is not the obvious one: entering from an export, the slew limit moves the command off zero before the counter reaches 3. It is entered from the DEADBAND - a site resting at its target counts saturated cycles while nothing is wrong - and 0 W is then a fixed point: a 2 kW export arriving afterwards leaves the command at exactly 0.0 W on every subsequent cycle. And prev_w == 0 is not where it stops. The same tuning pins at -800 W against its own integrator standing at -2388 W, with the same absence of an exit. So the change is written against the output freeze, not against a special case of it. THE DECISION IS WHICH FREEZES GET AN EXIT, NOT HOW LONG THE BLOCK LASTS. - A divergence a LIMIT could have produced - the inverter doing less than the command, in the command's own direction - clears itself, because the command is pinned at something the inverter can reach. No exit. release/1.0 unchanged, and asserted so: a curtailed inverter is never released over 30 cycles. - A divergence a limit could NOT have produced - overshooting the command, or the opposite sign - cannot clear itself, because clearing it needs the inverter to track OUR command. That one releases. unasked_power() is that predicate and it is physical, not statistical: a limit can only make the inverter do less, in the command's own direction, so everything a limit can produce lies in the interval spanned by 0 and prev_w widened by saturation_w. At prev_w == 0 it IS the saturation test, which is exactly why the zero case is the one that strands. The bound is not a new number. SETTLE_CYCLES = ceil(PLANT_SETTLE_S / LOOP_TICK_S) = 9, SAFETY-39's two measured constants and the reciprocal of SLEW_RAIL_FRAC: a divergence still standing after the plant's own settling time is not this plant catching up. _SAT_COUNT_CAP is written as SETTLE_CYCLES + 1; its value is unchanged at 10. RELEASING THE OUTPUT FREEZE DOES NOT UNFREEZE THE LOOP, which is what makes this bounded. The integrator freeze is untouched, so a released cycle can only move the command toward an accumulator that stopped accumulating, by at most one slew step, and never past it - swept over 107 625 released states. On the run above the command starts on the ninth saturated cycle and settles at -1190 W against a frozen integrator of -1194 W, not at the 5000 W rail. Graded against a transcription of release/1.0 over the 3024-case sweep, which was NOT taught the new rule: 2592 cases bit-identical, 432 releases, and 27 of those change the command - every one a charge from prev_w == 0 that release/1.0 refused to start. SAFETY-04's deliberate assertion is kept rather than inverted: its fixture sits below the bound, so it still asserts the block, and the release is asserted beside it on the other side of the bound. Coverage audit re-measured, same mutation form, __pycache__ purged: output freeze 2 -> 11 (2 reproduced on a pristine c8dcb38 first), the exit 4, unasked_power 6. saturation_cycles at 9 or 10 removes the block entirely, because the freeze cannot engage before its own exit is due. Not refused at startup; asserted in the tests and written in DOCS.md. Nothing was measured on two inverters: inverter B does not physically exist (NOTES.md §1.1b) and sim/plantsim.py:216 INVERTER_B is INVERTER_A copied with measured=False. The N=2 escalation is stated from the code (actual_w is site_batt_w, the SUM) and is labelled as such. 22/22 test files pass in goodwe-addon-ci, mounted :ro, bytecode purged first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/CHANGELOG.md | 45 +++ .../SAFETY-13-bounding-the-output-freeze.md | 276 ++++++++++++++++++ goodwe_controller/DOCS.md | 49 +++- goodwe_controller/app/control.py | 144 ++++++++- goodwe_controller/test_control.py | 260 ++++++++++++++++- 5 files changed, 761 insertions(+), 13 deletions(-) create mode 100644 goodwe_controller/DECISIONS/SAFETY-13-bounding-the-output-freeze.md diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index bd2141a..fd5bc9c 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -31,6 +31,51 @@ guessing a boundary; saying so is more useful than a guess. ## Unreleased +⚠️ **THE OUTPUT FREEZE NOW HAS AN EXIT, AND ONLY WHERE IT CANNOT END ON ITS OWN +([[SAFETY-13]]).** While the loop is frozen the command's magnitude may fall but +never rise; at `prev_w == 0` there is no magnitude left to fall, so a charge +cannot start. That refusal is deliberate - the inverter reporting power we did +not command means another writer is on the actuator, the §1.5 non-goal - and it +is kept. What it did not have was an END: nothing cleared it but the other +writer leaving. + +The exit is now explicit, and *which* freezes get one is the decision rather +than *how long*: + +- A divergence a **limit** could have produced - the inverter doing less than + the command, in the command's own direction - **clears itself**, because the + command is pinned at something the inverter can reach. It gets no exit, and + that is release/1.0's behaviour kept unchanged. +- A divergence a limit could **not** have produced - the inverter overshooting + the command, or producing the opposite sign - cannot clear itself, because + clearing it needs the inverter to track *our* command. That one releases once + the divergence has outlasted `PLANT_SETTLE_S`, i.e. `SETTLE_CYCLES` = 9 cycles + at the measured tick. No new constant: it is the reciprocal of SAFETY-39's + `SLEW_RAIL_FRAC`. + +⚠️ **RELEASING THE OUTPUT FREEZE DOES NOT UNFREEZE THE LOOP.** The integrator +freeze is untouched, so the command can only walk, one slew step per cycle, +toward an accumulator that stopped accumulating when the freeze engaged. +Measured at the live install's shape (`max_w 5000`, `slew_w 400`, +`target_grid_w -10`) with another writer holding the pack at +700 W and 2 kW of +export standing: the command holds at 0 W, starts on the ninth saturated cycle +and settles at **-1190 W** against a frozen integrator of -1194 W, not at the +5000 W rail. On `release/1.0` @ `c8dcb38` the same run held **0.0 W on every +cycle**, for as long as the other writer stayed. + +⚠️ **THIS IS NOT A REGRESSION FIX AND IT ESCALATES AT TWO INVERTERS.** The +unbounded block is `release/1.0` behaviour, confirmed cycle for cycle against +SAFETY-04's branch. At a roster of two, `actual_w` is the SITE SUM, so the +comparison is a site command against two units' power and one unit still moving +after the other has stopped can put the site into the block with no external +actor at all. + +Graded against `release/1.0` over the 3024-case sweep in `test_control.py`: +**2592 cases bit-identical, 432 releases, and 27 of those change the command** - +every one of them a charge from `prev_w == 0` that `release/1.0` refused to +start. `saturation_cycles` at 9 or 10 removes the block entirely; DOCS.md says +so. + ⚠️ **A PER-INVERTER POWER READING THAT HAS STOPPED FOLLOWING ITS OWN COMMAND IS NOW `None`, NOT A HELD NUMBER ([[SAFETY-37]]).** `Inverter.batt` (register `0x0518`) goes through a freshness gate: unchanged for >= 35 s *while* the diff --git a/goodwe_controller/DECISIONS/SAFETY-13-bounding-the-output-freeze.md b/goodwe_controller/DECISIONS/SAFETY-13-bounding-the-output-freeze.md new file mode 100644 index 0000000..708380d --- /dev/null +++ b/goodwe_controller/DECISIONS/SAFETY-13-bounding-the-output-freeze.md @@ -0,0 +1,276 @@ +# 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](https://trello.com/c/uZjmuF7B)). +**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 cannot clear itself and gets + one. The predicate is physical, not statistical. §4. +4. **Releasing the output freeze does not unfreeze the loop**, and that is what + makes the release bounded rather than merely different. §6. + +--- + +## 1. What was there, in the terms the code uses + +`control.py`, in `compute()`, after the clamp and the slew limit: + +```python +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. Anything +outside was put there by something else. + +| 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 is not a new number + +`SETTLE_CYCLES = ceil(PLANT_SETTLE_S / LOOP_TICK_S)` = **9**. + +`PLANT_SETTLE_S` (18.0 s) and `LOOP_TICK_S` (2.0 s) are SAFETY-39's two measured +constants, already in this file. A divergence still standing after the plant's +own settling time is not this plant catching up — so after `SETTLE_CYCLES` the +plant has run out of alibi. It is the reciprocal of `SLEW_RAIL_FRAC`: the slew +limit takes exactly this many ticks to cross the rail, for exactly this reason. + +⚠️ **It inherits SAFETY-39's caveat about `LOOP_TICK_S`, in the opposite +direction.** A meter **slower** than 2 s makes 9 cycles cover more than 18 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` is now written as `SETTLE_CYCLES + 1`. Its value is unchanged +(10, exactly as shipped); writing it against the bound stops a counter capped +below the longest bound read from it, which would silently delete the exit. + +--- + +## 5. What actually changed, graded against `release/1.0` + +`test_control.py`'s 3024-case 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. + +| | cases | +|---|---| +| total | 3024 | +| **bit-identical to `release/1.0`** | **2592** | +| releases | 432 | +| …of those, reason-only (the freeze was not binding) | 405 | +| …of those, **the command changes** | **27** | + +All 27 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. Nothing +else in the sweep moved. + +The behaviour after the fix, on §2's run: the command holds at 0 W through +`sat_count` 3–8, releases on the **ninth** saturated cycle, and walks +0 → −400 → −800 → **−1190 W**, where it stops. + +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. Swept over 107 625 released states in `test_control.py`: no case leaves the +interval between the previous command and the frozen integrator, and no case +moves further than one slew step. + +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` in the action log means "look for the second writer."** 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. + +**One configuration removes the block entirely.** `saturation_cycles` is +accepted up to 10 by `config.yaml`. At 9 or above the freeze cannot engage +before its own exit is due, so the no-start block never happens. This is +**not** refused at startup — a controller that will not boot over a tuning +nuance is worse than the nuance — but it is asserted in `test_control.py` and +written in DOCS.md so that it is a known consequence and not a surprise. + +**Nothing was measured on two inverters.** See the warning in §3. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index a4d28f8..1380344 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -473,7 +473,7 @@ behind it would trip the firmware watchdog on a system that is working fine. | `target_grid_w` | -10 | What the meter should rest at. Negative = a slight export | | `step_w` | 10 | Quantisation | | `saturation_w` | 500 | Divergence that counts as "the inverter is at a limit" | -| `saturation_cycles` | 3 | How many consecutive cycles before freezing. A cycle is one meter update or `cycle_floor_s`, whichever comes first - see the note below. **Do not set to 1** | +| `saturation_cycles` | 3 | How many consecutive cycles before freezing. A cycle is one meter update or `cycle_floor_s`, whichever comes first - see the note below. **Do not set to 1**, and **do not set it to 9 or 10** either: at 9 the freeze cannot engage before its own exit is due, so the no-start block stops existing (SAFETY-13, below) | | `integrator_max_w` | 0 | Bound on the loop's accumulator, and 0 means "same as `max_w`". Caps how much stale error can be waiting to unwind when the sign flips. **Do not raise it above `max_w`** - the output clamp already bounds what is commanded, so the only thing extra headroom buys is more cycles of wrong-direction power after every saturation event. Lowering it below `max_w` is the useful direction | | `heartbeat_s` | 10 | Refresh interval; must stay well under the firmware watchdog | | `cycle_floor_s` | 15 | The longest the loop may go without evaluating. A deadline on the gap between cycles, reset by every cycle — on a healthy meter it never elapses and changes nothing. Raise it only if your meter is slower than ~10 s; it must stay under `meter_max_age_s`, past which there is no reading to evaluate anyway | @@ -628,6 +628,53 @@ correct toward that wrong value instead of holding a stale command. It is bounde happened to a lying meter whose value merely jittered — bit-exact repetition was never a safety property. +#### When the freeze lets go, and when it deliberately never does (SAFETY-13) + +While the loop is frozen the command's **magnitude may fall but never rise**. That is +the anti-windup rule, and at `prev_w == 0` it reads as a refusal to *start*: with +nothing commanded there is no magnitude left to fall, so a charge cannot begin. That +refusal is deliberate. Commanding 0 W while the inverter reports hundreds of watts +means something is driving the actuator that this add-on did not ask, and starting to +push power the other way into it is how two controllers end up fighting over one +actuator — the configuration §1.5 of the specification rules out. + +**What was missing was an end to it.** Nothing cleared the block: not the error growing, +not the integrator moving, not time. The exit is now explicit, and *which* freezes get +one is the decision: + +| the inverter is… | can it clear itself? | what happens now | +|---|---|---| +| doing **less** than the command, in the command's own direction — a cold pack, a curtailment, an export cap | **yes.** The command is pinned at something the inverter can reach, so it catches up and the freeze clears | unchanged: frozen for as long as it lasts, with no exit | +| doing something **outside** what the command can explain — overshooting it, or the opposite sign | **no.** Clearing the freeze needs the inverter to track *our* command, and the other writer is what stops it | the freeze releases once the divergence has outlasted the plant that could otherwise have explained it | + +The second row's threshold is not a new number. `PLANT_SETTLE_S` (18 s, measured) is how +long this plant takes to settle, so a divergence still standing after that is not this +plant catching up; at the measured 2 s meter tick that is **9 cycles**, which is the +same relationship the slew limit already uses (`control.py`, `SETTLE_CYCLES`). + +**Releasing the output freeze does not unfreeze the loop.** The *integrator* freeze is +untouched, so the command is free to move only as far as an accumulator that stopped +accumulating cycles ago, one slew step per cycle. Measured on the simulator at the live +install's shape (`max_w 5000`, `slew_w 400`, `target_grid_w -10`), with another writer +holding the pack at +700 W and 2 kW of export standing: the command holds at 0 W while +the counter climbs, starts at the ninth saturated cycle, and settles at **−1190 W** +against a frozen integrator of −1194 W — not at the 5000 W rail. Before this change the +same run held 0.0 W on every cycle, for as long as the other writer stayed. + +⚠️ **The site still does not reach its target in that state, and it is not supposed +to.** What the exit buys is that the battery stops being idle while the site exports; +what it does not buy is correct regulation against an actuator somebody else is also +writing to. A standing grid error with another writer on the bus is the symptom of a +configuration this add-on refuses to support, not a tuning problem. If you see +`freeze-released` in the action log, look for the second writer. + +⚠️ **One configuration removes the block entirely, silently.** `saturation_cycles` is +accepted up to 10. Set it at or above 9 and the freeze does not engage until the bound +has already passed, so the no-start block never happens at all. This is not refused at +startup — a controller that will not boot over a tuning nuance is worse than the nuance +— but it is why the default is 3 and why the table above says **do not set to 1** at one +end and leaves you this note at the other. + #### Why `target_grid_w` is not zero The deadband is a one-way ratchet: any resting point inside it holds until diff --git a/goodwe_controller/app/control.py b/goodwe_controller/app/control.py index e41cef5..8192a03 100644 --- a/goodwe_controller/app/control.py +++ b/goodwe_controller/app/control.py @@ -16,6 +16,7 @@ the short version is in the comments below. Do not "clean this up" - each rule exists because its absence produced a specific, observed failure. """ +import math from dataclasses import dataclass # --------------------------------------------------------------------------- @@ -86,6 +87,56 @@ 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 +# --------------------------------------------------------------------------- +# SAFETY-13. HOW LONG THE PLANT STAYS A CANDIDATE EXPLANATION. +# +# The output freeze below stops the command growing while the inverter is not +# following. Two completely different things produce that divergence and the +# freeze cannot tell them apart on magnitude alone: +# +# (a) THE PLANT HAS NOT CAUGHT UP YET, or it is at a limit - a cold pack, a +# curtailment, an export cap. Asking for more is pointless, and the +# freeze is right to hold. It also CLEARS ITSELF: the command is pinned +# at something the inverter can do, so the inverter catches up to it and +# `saturated_now` goes false. +# +# (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. +# +# SETTLE_CYCLES = ceil(PLANT_SETTLE_S / LOOP_TICK_S) +# +# ⚠️ 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. +# +# ⚠️ AND 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 +# 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 + +# 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 + @dataclass(frozen=True) class Tuning: @@ -185,6 +236,48 @@ def slew_limit_w(tuning: Tuning) -> float: return SLEW_RAIL_FRAC * abs(float(tuning.max_w)) +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. + + 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? + + ⚠️ 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 + command, in the command's own direction. It cannot make it overshoot the + 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. + + Worked, at `saturation_w = 500`: + + prev +2000, actual +700 -> inside [0, 2000] a limit. False. + prev -5000, actual -700 -> inside [-5000, 0] a limit. False. + prev 0, actual +700 -> outside [-500, 500] unasked. True. + prev -800, actual +700 -> outside [-1300, 500] unasked. True. + + ⚠️ 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. + + ⚠️ 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. + """ + lo = min(0.0, float(prev_w)) - abs(float(saturation_w)) + hi = max(0.0, float(prev_w)) + abs(float(saturation_w)) + return not (lo <= float(actual_w) <= hi) + + def compute( prev_w: float, grid_w: float, @@ -232,7 +325,7 @@ def compute( # clockless; the remaining ceiling is that saturation_cycles expresses a # bound in seconds rather than an exact number of them. saturated_now = abs(prev_w - actual_w) > tuning.saturation_w - sat_count = min(sat_count + 1, 10) if saturated_now else 0 + sat_count = min(sat_count + 1, _SAT_COUNT_CAP) if saturated_now else 0 frozen = sat_count >= tuning.saturation_cycles # --- deadband ---------------------------------------------------------- @@ -346,8 +439,53 @@ def compute( # implementation: commanding 0 while the inverter reports >500 W means # something else is driving the bus, and that is not the moment to # start pushing power the other way. - target = min(target, prev_w) if prev_w > 0 else max(target, prev_w) - reason = "saturated-freeze" + # + # --- SAFETY-13: ...AND IT IS NOT THE MOMENT FOREVER EITHER ---------- + # Everything above stands. What was missing is an END. Held at + # `prev_w == 0` this is a refusal to START, and nothing in this + # function or above it clears it - not the error growing, not the + # integrator moving, not time. Measured on release/1.0 @ c8dcb38, at + # the live install's shape (max_w 5000, slew_w 400, target_grid_w -10): + # a site resting in the deadband with another writer holding the pack + # at +700 W reaches sat_count 3 with the command at 0, and 0 W is then + # a FIXED POINT - a 2 kW export arriving afterwards leaves the command + # at exactly 0.0 W on every subsequent cycle, with the integrator + # standing at -1194 W the whole time. + # + # ⚠️ AND THE ZERO CASE IS NOT SPECIAL, WHICH IS WHY THE FIX IS NOT + # SPECIAL EITHER. The same run pinned at prev_w == -800 W, against its + # own integrator standing at -2388 W, for as long as it ran. + # `prev_w == 0` is only the instance where the pinned magnitude happens + # to be zero - i.e. where the standing cost is the whole of what the + # battery could have done. + # A rule written for the zero case alone would have left the other + # instances exactly as unbounded as it found them. + # + # ⚠️ WHAT DECIDES IS NOT HOW LONG, IT IS WHETHER THE FREEZE CAN EVER + # 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. + # + # ⚠️ RELEASING IS NOT UNFREEZING. The INTEGRATOR freeze above is not + # lifted, and that is what makes this bounded rather than merely + # different: the command is free to move, but only as far as an + # integrator that stopped accumulating cycles ago, through the same + # slew limit as any other cycle. It buys back the standing error, not + # the rail. + # + # `reason` is authoritative here in a way it is not elsewhere in this + # function: nothing after this line writes it. + if unasked_power(prev_w, actual_w, tuning.saturation_w) \ + and sat_count >= SETTLE_CYCLES: + reason = "freeze-released" + else: + target = min(target, prev_w) if prev_w > 0 else max(target, prev_w) + reason = "saturated-freeze" # --- quantise ---------------------------------------------------------- # 10 W. The register is 1 W and the inverter reports at 1 W, but its diff --git a/goodwe_controller/test_control.py b/goodwe_controller/test_control.py index 47574b8..8bebc68 100644 --- a/goodwe_controller/test_control.py +++ b/goodwe_controller/test_control.py @@ -17,8 +17,9 @@ Read MUTATION.md before you report a survivor. import sys -from app.control import (LOOP_TICK_S, PLANT_SETTLE_S, SLEW_RAIL_FRAC, - Tuning, compute, slew_limit_w, +from app.control import (LOOP_TICK_S, PLANT_SETTLE_S, SETTLE_CYCLES, + SLEW_RAIL_FRAC, Tuning, compute, slew_limit_w, + unasked_power, maintenance_charge_floor, peak_at_risk) # ⚠️ SAFETY-39: `slew_w` IS EXPLICIT HERE NOW, AND IT IS release/1.0's VALUE. @@ -59,6 +60,17 @@ def check(name, cond): # A comment cannot go stale-green, because it never claims to be running. # # Measured at 389d9ec. Numbers are the lead's independent reproduction. +# ⚠️ THE OUTPUT FREEZE ROW WAS RE-MEASURED AT SAFETY-13 AND IT MOVED FROM 2 TO +# 11, because the SAFETY-13 section grades that branch too. Both figures are +# from the SAME mutation form - the whole `target = min(...)/max(...)` line +# removed, `reason` left alone - and 2 was reproduced on a pristine c8dcb38 +# tree before the new figure was taken, so the row moved because checks were +# added and not because the mutation changed shape. The two new rows below it +# are that section's own mechanisms: removing the EXIT restores release/1.0's +# unbounded freeze, and making `unasked_power()` return True gives an exit to +# the limit case as well, which is the half of the decision that has to stay +# unbounded. +# # ⚠️ THE SLEW ROWS WERE RE-MEASURED AT SAFETY-39 AND ONE OF THEM MOVED: the # slew limit was 4 and is now 5, because the SAFETY-39 section grades it too. # The new row is the RAIL DEFAULT specifically - `slew_limit_w()` reduced to @@ -75,7 +87,9 @@ def check(name, cond): # output clamp 3 # slew limit 5 # slew follows the rail (SAFETY-39), `slew_w` read raw 3 -# output freeze 2 +# output freeze 11 +# the output freeze's EXIT (SAFETY-13), freeze made unconditional again 4 +# `unasked_power()` discriminates, replaced by `return True` 6 # deadband 5 # quantisation 2 # saturation detector, `saturated_now = False` 11 @@ -282,8 +296,15 @@ check("frozen at i_w=0.0: an export push moves the integrator", # hundreds of watts means something else is driving the bus. Asserted so that # nobody "fixes" it by accident - the integrator moving is what this ticket # owns, the command rule belongs to the output freeze. +# +# ⚠️ SAFETY-13 KEPT THIS ASSERTION RATHER THAN INVERTING IT, and the fixture is +# why it survived unchanged: `sat_count=3` becomes 4, which is below +# SETTLE_CYCLES, so this is still inside the window in which the block holds. +# SAFETY-13 gave the block an EXIT, not a different opinion about its first +# cycles - see the SAFETY-13 section below, where the same state is driven past +# the bound and the release is asserted on the other side of it. check("frozen at i_w=0.0: the output freeze still blocks a charge from 0 W", - ze.target_w == 0.0) + ze.target_w == 0.0 and ze.sat_count < SETTLE_CYCLES) # Where prev_w is already charging the output freeze does NOT block, and there # the difference reaches the wire: held at 0.0 the integrator abandons the # charge mid-export. @@ -358,6 +379,192 @@ check(f"after saturation ends the command is {tight:.0f} W (<= 1000)", tight <= check(f"headroom above max_w makes that worse ({loose:.0f} W) - hence the default", loose > tight) +print("SAFETY-13: the output freeze has an exit, and only where it needs one") + +# THE SHAPE OF THIS SECTION. release/1.0's output freeze holds the command's +# magnitude down while the inverter is not following, and nothing ends it. At +# `prev_w == 0` the held magnitude is ZERO, so it reads as "refuses to start a +# charge"; that is the case the card was written about, but it is one instance +# of a rule that is unbounded at every prev_w. SAFETY-13 bounds it exactly where +# it CANNOT end on its own - see `unasked_power()` - and nowhere else. + +# --- 1. the predicate, which is the whole of the decision ------------------- +# A limit can only make the inverter do LESS than the command, in the command's +# own direction, so everything a limit can produce lies in the interval spanned +# by 0 and prev_w widened by saturation_w. These are the four cases in the +# docstring, run rather than described. +check("limit: 700 W under a +2000 W command is explainable", + not unasked_power(2000.0, 700.0, 500.0)) +check("limit: -700 W under a -5000 W command is explainable", + not unasked_power(-5000.0, -700.0, 500.0)) +check("unasked: +700 W under a 0 W command is not", + unasked_power(0.0, 700.0, 500.0)) +check("unasked: +700 W under a -800 W command is not", + unasked_power(-800.0, 700.0, 500.0)) +# The band edges belong to the limit side, because saturation_w is the same +# tolerance the detector uses - a reading the detector calls "following" must +# never be called "another writer". +check("the saturation band is inclusive on both edges", + not unasked_power(0.0, 500.0, 500.0) + and not unasked_power(0.0, -500.0, 500.0) + and unasked_power(0.0, 500.01, 500.0)) +# ⚠️ AT prev_w == 0 THE PREDICATE AND THE DETECTOR ARE THE SAME TEST, which is +# why the zero case is the one that strands: with nothing commanded there is no +# interval for a limit to live in. +same = all(unasked_power(0.0, a, 500.0) == (abs(a) > 500.0) + for a in [x * 5.0 for x in range(-400, 401)]) +check("at prev_w == 0 unasked_power IS the saturation test, over 801 readings", + same) + +# --- 2. the bound is derived, not chosen ------------------------------------ +check(f"SETTLE_CYCLES is ceil(PLANT_SETTLE_S / LOOP_TICK_S) = {SETTLE_CYCLES}", + SETTLE_CYCLES == 9 and SETTLE_CYCLES * LOOP_TICK_S >= PLANT_SETTLE_S + and (SETTLE_CYCLES - 1) * LOOP_TICK_S < PLANT_SETTLE_S) +check("...which is the reciprocal of SAFETY-39's SLEW_RAIL_FRAC", + abs(SETTLE_CYCLES - 1.0 / SLEW_RAIL_FRAC) < 1e-9) +# The counter has to be able to REACH the bound or the exit does not exist. +# Driven, not read off the constant: run a saturated state and see how high the +# counter actually goes. +c, sc = 0, 0 +while c < 50: + sc = compute(0.0, -2000.0, 900.0, T, sc).sat_count + c += 1 +check(f"the saturation counter reaches {sc}, which is above the bound", + sc >= SETTLE_CYCLES) + +# --- 3. prev_w == 0: blocked below the bound, released above it ------------- +# The same state either side of the bound. This is the pair the card asks for. +blk = compute(prev_w=0, grid_w=-2000, actual_w=-600, tuning=T, + sat_count=SETTLE_CYCLES - 2, i_w=0.0) +check("prev_w=0: below the bound the charge is still refused", + blk.target_w == 0.0 and blk.reason == "saturated-freeze" + and blk.sat_count == SETTLE_CYCLES - 1) +rel = compute(prev_w=0, grid_w=-2000, actual_w=-600, tuning=T, + sat_count=SETTLE_CYCLES - 1, i_w=0.0) +check(f"prev_w=0: at the bound it is released to {rel.target_w:.0f} W", + rel.target_w < 0.0 and rel.reason == "freeze-released" + and rel.sat_count == SETTLE_CYCLES) +# ...and the released command is the SLEW step from 0, not the rail. A release +# that jumped to the integrator in one cycle would be the windup this ticket +# was told to avoid. +check(f"the released command is exactly one slew step" + f" ({slew_limit_w(T):.0f} W)", + abs(rel.target_w) == slew_limit_w(T)) +# The mirror. Nothing here is sign-specific, so the discharge side must behave +# identically - every freeze rule in this file has a mirror, because the one +# that did not is the defect that got through review. +mblk = compute(prev_w=0, grid_w=2000, actual_w=600, tuning=T, + sat_count=SETTLE_CYCLES - 2, i_w=0.0) +mrel = compute(prev_w=0, grid_w=2000, actual_w=600, tuning=T, + sat_count=SETTLE_CYCLES - 1, i_w=0.0) +check("prev_w=0, discharging: same block, same release, mirrored", + mblk.reason == "saturated-freeze" and mrel.reason == "freeze-released" + and mrel.target_w == slew_limit_w(T)) + +# --- 4. non-zero prev_w, and the two halves are NOT the same -------------- +# (a) A divergence a LIMIT could have produced is never released, however long +# it stands. This is release/1.0's behaviour and it is kept deliberately: the +# freeze clears itself here, because the command is pinned at something the +# inverter can do. +TL = Tuning(max_w=5000.0, slew_w=400.0, target_grid_w=-10.0) +prev, i_w, sat, reasons = 0.0, 0.0, 0, set() +for _ in range(30): + d = compute(prev, 4000.0, max(-1000.0, min(1000.0, prev)), TL, sat, i_w) + prev, i_w, sat, _r = d.target_w, d.i_w, d.sat_count, reasons.add(d.reason) +check("non-zero prev_w, a curtailed inverter: never released over 30 cycles", + "freeze-released" not in reasons and prev == 2400.0) +# (b) A divergence a limit could NOT have produced is released at the bound, +# at a non-zero prev_w exactly as at zero. release/1.0 pinned this run at +# -800 W, against its own integrator standing at -2388 W, for as long as it ran. +prev, i_w, sat, rows = 0.0, 0.0, 0, [] +for _ in range(20): + d = compute(prev, -2000.0, 700.0, TL, sat, i_w) + rows.append((prev, d.target_w, d.i_w, d.reason)) + prev, i_w, sat = d.target_w, d.i_w, d.sat_count +pinned = [r for r in rows if r[3] == "saturated-freeze"] +check(f"non-zero prev_w, another writer: pinned at {pinned[0][1]:.0f} W...", + pinned and all(r[1] == -800.0 for r in pinned)) +check(f"...then released, settling at {prev:.0f} W", prev == -2390.0) + +# --- 5. THE CARD'S OWN CASE, DRIVEN END TO END ----------------------------- +# The live install's shape (max_w 5000, slew_w 400, target_grid_w -10). The site +# rests in the deadband with another writer holding the pack at +700 W - so the +# saturation counter climbs while the command is 0 and nothing is wrong - and +# then 2 kW of export arrives. On release/1.0 @ c8dcb38 the command is 0.0 W on +# every cycle from there on: 0 W is a FIXED POINT, and the export is given away +# for as long as the other writer stays. +prev, i_w, sat, cmds = 0.0, 0.0, 0, [] +for c in range(24): + d = compute(prev, -10.0 if c < 6 else -2000.0, 700.0, TL, sat, i_w) + cmds.append(d.target_w) + prev, i_w, sat = d.target_w, d.i_w, d.sat_count +check("the card's case: the block does hold at first, at 0 W", + cmds[6] == 0.0 and cmds[7] == 0.0) +started = [i for i, c in enumerate(cmds) if c != 0.0] +check(f"the card's case: the charge starts at cycle {started[0]}" + f" instead of never", bool(started)) +# ⚠️ AND IT STOPS AT THE FROZEN INTEGRATOR, NOT AT THE RAIL. This is AC 4: the +# integrator moved while the output was blocked, and the release must not turn +# that divergence into windup. It cannot, because releasing the OUTPUT freeze +# does not release the INTEGRATOR freeze - so the command can only walk, one +# slew step per cycle, to a value the integrator stopped accumulating at. +check(f"the card's case: it settles at {prev:.0f} W, at the frozen integrator " + f"({i_w:.0f} W) and not the {TL.max_w:.0f} W rail", + prev == -1190.0 and abs(prev) <= abs(i_w) and abs(prev) < TL.max_w) +steps = [abs(cmds[i] - cmds[i - 1]) for i in range(1, len(cmds))] +check(f"the card's case: no cycle moves more than the slew limit " + f"(worst {max(steps):.0f} W of {slew_limit_w(TL):.0f})", + max(steps) <= slew_limit_w(TL)) + +# --- 6. AC 4, AS A PROPERTY RATHER THAN A TRAJECTORY ----------------------- +# ⚠️ THE FIRST FORM OF THIS CHECK WAS WRONG AND THE SWEEP SAID SO, which is +# worth leaving in the file. It asserted `|target| <= |i_w|` on a released +# cycle, and the sweep produced prev_w -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. +# +# What it means: a released cycle can only move the command TOWARD the +# integrator, by at most one slew step, and never past it. So the divergence +# that built up while the output was blocked cannot arrive as a jump - it is +# spent one slew step at a time, against an integrator that stopped +# accumulating when the freeze engaged and is not released with the output. +step = float(T.step_w) +past, jumped = [], [] +for prev in [x * 250.0 for x in range(-20, 21)]: + for grid in [x * 500.0 for x in range(-12, 13)]: + for actual in [x * 500.0 for x in range(-10, 11)]: + for i0 in (-3000.0, -500.0, 0.0, 500.0, 3000.0): + d = compute(prev, grid, actual, TL, SETTLE_CYCLES, i0) + if d.reason != "freeze-released": + continue + lo, hi = min(prev, d.i_w), max(prev, d.i_w) + if not (lo - step <= d.target_w <= hi + step): + past.append((prev, grid, actual, i0, d.target_w, d.i_w)) + if abs(d.target_w - prev) > slew_limit_w(TL) + step: + jumped.append((prev, grid, actual, i0, d.target_w)) +check(f"over {41*25*21*5} released states the command stays between the" + f" previous command and the frozen integrator" + + (f" (e.g. {past[0]})" if past else ""), not past) +check("...and no released cycle moves further than one slew step" + + (f" (e.g. {jumped[0]})" if jumped else ""), not jumped) + +# --- 7. the one configuration in which the block does not exist ------------ +# ⚠️ SAID OUT LOUD BECAUSE IT IS A SILENT NO-OP OTHERWISE. `saturation_cycles` +# is accepted up to 10 by config.yaml. Set it at or above SETTLE_CYCLES and the +# freeze does not engage until the bound has already passed, so the no-start +# block never happens at all. It is not refused at startup - a controller that +# will not boot over a tuning nuance is worse than the nuance - but it must be +# a known consequence rather than a surprise. +TN = Tuning(slew_w=1000.0, saturation_cycles=SETTLE_CYCLES) +prev, i_w, sat, reasons = 0.0, 0.0, 0, set() +for _ in range(20): + d = compute(prev, -2000.0, 700.0, TN, sat, i_w) + reasons.add(d.reason) + prev, i_w, sat = d.target_w, d.i_w, d.sat_count +check(f"saturation_cycles >= {SETTLE_CYCLES} means the block never engages", + "saturated-freeze" not in reasons) + print("SAFETY-04: the i_w=None path is still release/1.0, exactly") @@ -394,8 +601,19 @@ def legacy(prev, grid, actual, t, sat_count): # "i-clamped", because the truncation happens on the integrator before the # command is derived from it. Aliased here rather than papered over - if any # OTHER reason ever diverges, this check goes red. +# ⚠️ SAFETY-13 IS A DELIBERATE DIVERGENCE FROM `legacy()`, AND THE SWEEP NOW +# GRADES THE DIVERGENCE INSTEAD OF FORBIDDING IT. `legacy()` above stays a +# transcription of release/1.0 - it is not edited to agree, for the reason the +# note on `slew_w` gives: an oracle taught the new rule compares the new law +# against itself. What changed is the assertion. Every case must either be +# IDENTICAL to release/1.0, or be a case where release/1.0 said +# "saturated-freeze" and this law says "freeze-released" with the two conditions +# that permit it both true. Anything else is a regression, and the counts below +# say how much of the sweep each answer covers. ALIAS = {"i-clamped": "clamped"} diffs = [] +released = [] +value_diffs = [] seen = set() # ⚠️ `slew_w` IS WRITTEN OUT IN ALL THREE (SAFETY-39). `legacy()` below is a # TRANSCRIPTION of release/1.0 and reads `t.slew_w` directly, as release/1.0 @@ -414,11 +632,35 @@ for tune in (Tuning(slew_w=1000.0), Tuning(slew_w=1000.0, target_grid_w=-10.0), seen.add(d.reason) got = (d.target_w, d.sat_count, d.frozen, ALIAS.get(d.reason, d.reason)) - if got != legacy(prev, grid, actual, tune, sc): - diffs.append((prev, grid, actual, sc, got, - legacy(prev, grid, actual, tune, sc))) -check(f"i_w=None reproduces release/1.0 over {3*7*9*4*4} cases, reason included" - + (f" (first diff {diffs[0]})" if diffs else ""), not diffs) + was = legacy(prev, grid, actual, tune, sc) + if got == was: + continue + explained = ( + d.reason == "freeze-released" + and was[3] == "saturated-freeze" + and d.sat_count >= SETTLE_CYCLES + and unasked_power(prev, actual, tune.saturation_w)) + if not explained: + diffs.append((prev, grid, actual, sc, got, was)) + continue + released.append((prev, grid, actual, sc)) + if got[0] != was[0]: + value_diffs.append((prev, got[0], was[0])) +TOTAL = 3 * 7 * 9 * 4 * 4 +check(f"i_w=None reproduces release/1.0 over {TOTAL} cases except where" + f" SAFETY-13 releases the freeze, reason included" + + (f" (first unexplained diff {diffs[0]})" if diffs else ""), not diffs) +check(f"{TOTAL - len(released)} of {TOTAL} cases are bit-identical to" + f" release/1.0; {len(released)} are releases", len(released) == 432) +# ⚠️ AND THE RELEASES ARE MOSTLY A LABEL. Only these change what reaches the +# wire; the rest report "freeze-released" on a command the freeze was not +# binding on anyway. Every one of them is a charge that release/1.0 refused to +# start from exactly 0 W, which is the ticket, stated as a count. +check(f"{len(value_diffs)} of those {len(released)} change the command," + f" and every one is a charge start from prev_w == 0 that was refused", + len(value_diffs) == 27 + and all(p == 0.0 and new_ < 0.0 and old_ == 0.0 + for p, new_, old_ in value_diffs)) # ...and the rename is not a quiet deletion: the signal SAFETY-03 alarms on has # to actually occur in that sweep, or its hook is dead.