diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index bd2141a..426ad1c 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -31,6 +31,72 @@ 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 - may not clear itself at all, so + it gets one. It releases once the divergence has outlasted the plant's own + measured settling time: `SETTLE_CYCLES` = **12** cycles at the measured tick. + +⚠️ **THE BOUND COMES FROM THE SLOWER OF THE TWO DIRECTIONS THE PLANT WAS +MEASURED IN.** `NOTES.md` §5.10 timed the same 1.78 kW step both ways - ~18 s +up, ~23 s down - so `PLANT_SETTLE_DOWN_S` (23 s) joins `PLANT_SETTLE_S` (18 s) +as a named constant. SAFETY-39 uses 18 correctly, because its bound is a +*refusal* and the looser number refuses less; this bound asks how long the plant +may still be the *explanation*, and there the longer number is the safe one. A +bound of 9 derived from 18 fires the exit on a **single inverter with nothing +else on the bus** - measured in the container, 62 of 272 load-step pairs at the +plant's own measured shape, against 2 at 12. + +⚠️ **`freeze-released` IS A PLACE TO START LOOKING, NOT A FINDING.** It says the +divergence outlasted the plant's measured settling time. It is **not** proof of +a second writer: a large load or PV swing can leave our own power coasting +outside our own command's interval, and a plant slower than the one that was +measured does the same. DOCS.md now gives the innocent causes first and the +second writer only when the state *stands*. + +⚠️ **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 twelfth 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. ⚠️ That escalation is read from the code and from single-unit +runs; **it is not measured on two inverters.** Inverter B does not physically +exist yet and `sim/plantsim.py`'s `INVERTER_B` is `INVERTER_A` copied with +`measured=False`, so any two-inverter figure would be two copies of A. + +Graded against `release/1.0` over the 3780-case sweep in `test_control.py`: +**3267 cases bit-identical, 432 releases, 27 of those change the command** - +every one a charge from `prev_w == 0` that `release/1.0` refused to start - and +**81 differ in the saturation counter alone**, because its ceiling moved with +the bound from 10 to 13. That ceiling change also removes a degenerate setting: +`config.yaml` accepts `saturation_cycles` up to 10, which is now strictly below +the release bound, so **no accepted configuration can stop the block engaging**. +At the old bound of 9 a setting of 9 or 10 deleted it silently. + ⚠️ **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..742a7aa --- /dev/null +++ b/goodwe_controller/DECISIONS/SAFETY-13-bounding-the-output-freeze.md @@ -0,0 +1,396 @@ +# 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 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: + +```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. + +⚠️ **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. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index a4d28f8..b02013a 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**. Every value this file accepts (1-10) is below the freeze's own release bound of 12 cycles, so no accepted setting can stop the block engaging - see 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,94 @@ 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 | **not necessarily.** If another writer is on the actuator, clearing the freeze needs the inverter to track *our* command and the other writer is what stops it, so nothing ends it | the freeze releases once the divergence has outlasted the plant that could otherwise have explained it — see *what `freeze-released` actually tells you*, below, because this row is also where our own ramp lands | + +The second row's threshold is the plant's own measured settling time — **the slower of +the two directions it was measured in.** The 2026-08-22 step test timed the same 1.78 kW +step both ways: about 18 s to settle going up, about 23 s coming back down. 23 s is the +one that belongs here, because the question this bound answers is *how long may the plant +still be the explanation*, and the safe answer is to keep believing the plant for as long +as the plant has ever been seen to take. At the measured 2 s meter tick that is +**12 cycles** (`control.py`, `SETTLE_CYCLES`). + +⚠️ **The first version of this feature used 18 s, and 9 cycles was wrong in the direction +that accuses.** Nine cycles is three short of the plant's own down-ramp, so an ordinary +load step on a **single** inverter — the command falling through zero while the pack is +still coasting the old way — could produce `freeze-released` with nothing else on the bus +at all. Driven in the container against a model of the measured plant, one inverter and +no second writer anywhere: over 272 load-step pairs the 9-cycle bound produced a release +in 62 of them and the shipped 12-cycle bound in 2. + +**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 on the **twelfth** saturated cycle, walks 0 → −400 → −800, 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. + +#### What `freeze-released` in the action log actually tells you + +**It says: the inverter's power did not match the command for longer than this plant has +ever been measured to take to settle.** That is all it says. It is a reason to go +looking; it is not a finding, and in particular it is **not** proof that a second writer +exists. + +The test behind it compares the reading against **one cycle's** command, and the pack +carries more than one cycle of history. So our own power, arriving late, can look like +somebody else's: + +- **A big, fast load step is the ordinary innocent cause.** The command falls through + zero while the pack is still coasting in the old direction, and for a few cycles the + reading has the sign the command *used* to have. Measured, one inverter, nothing else + on the bus: eight cycles after a +1990 W command the command was 0 and the pack still + read +522.3 W. +- **A plant slower than the one that was measured does it too.** The 12-cycle bound is + derived from a 23 s down-ramp. A colder or larger pack that takes longer than that will + produce releases on its own, and no constant read off the 2026-08-22 measurement can + prevent it. In the same container sweep, holding the bound at 12 and lengthening only + the plant's down-ramp, single-inverter releases went from 8 of 320 scenarios at the + measured 23 s to 59 of 320 at 34 s and 268 of 320 at 50 s. + +**So, in order:** if you see `freeze-released` **once, during or just after a large load +or PV swing**, that is most likely the loop's own ramp and needs nothing. If you see it +**standing** — cycle after cycle, at a steady load, with a standing grid error the +battery is not correcting — then look for the second writer: another controller, a +scheduler in the inverter's own app, or an installer tool left connected. That is the +configuration §1.5 of the specification rules out, and it is the one this exit exists to +stop the loop stalling on forever. + +⚠️ **No `saturation_cycles` you can set removes the block.** This file accepts 1-10, and +all of those are below the 12-cycle release bound, so the freeze always engages before +its own exit is due. (The first version of this feature had the bound at 9, where a +setting of 9 or 10 *did* silently delete the block; moving the bound to 12 closed that +by construction rather than by warning about it.) + #### 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..180f58a 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 # --------------------------------------------------------------------------- @@ -65,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 @@ -86,6 +98,99 @@ 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. +# +# 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. +# +# ⚠️ 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 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. +# +# Case (a) is bounded by the plant, and the plant was measured: +# +# SETTLE_CYCLES = ceil(PLANT_SETTLE_DOWN_S / LOOP_TICK_S) +# +# ⚠️ 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 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. +# +# ⚠️ 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, 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) class Tuning: @@ -185,6 +290,71 @@ 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 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 + 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 + 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. + + ⚠️ 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`: + + 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 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, 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)) + return not (lo <= float(actual_w) <= hi) + + def compute( prev_w: float, grid_w: float, @@ -232,7 +402,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 +516,69 @@ 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 + # 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 + # 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..976c1b4 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_DOWN_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,34 @@ 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 SAFETY-13 ROWS WERE RE-MEASURED IN ROUND 2 AND THREE OF THEM WERE +# WRONG AS FIRST WRITTEN. Every figure below marked (S13) was taken by deleting +# the mechanism, purging __pycache__, running this file in the CI image and +# counting `FAIL` lines, on THIS tree; the release/1.0 comparison figures were +# taken the same way on a pristine c8dcb38 worktree. +# +# - `output freeze` was written as 11. The mutation form its own note +# describes - the whole `target = min(...)/max(...)` line removed, `reason` +# left alone - gives 6 on this tree AND 6 on the first SAFETY-13 commit, +# against 2 on release/1.0. 11 is not reproducible by that form, or by +# removing the line and its `reason` (4), or by swapping min/max (12). The +# row is 6. +# - `saturation detector, saturated_now = False` was left at 11 while the +# SAFETY-13 section added checks that grade it. It is 15. +# - the footnote's `frozen = False` figure was left at 10 for the same +# reason. It is 13. +# +# The rows below `output freeze` are this ticket's own mechanisms: removing the +# EXIT restores release/1.0's unbounded freeze; 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; deriving SETTLE_CYCLES from +# PLANT_SETTLE_S (18 s, the UP ramp) instead of PLANT_SETTLE_DOWN_S (23 s) is +# the round-1 defect itself; and pinning `_SAT_COUNT_CAP` back at release/1.0's +# literal 10 caps the counter below the bound and deletes the exit silently. +# +# ⚠️ ONLY THE ROWS THIS BRANCH MOVES WERE RE-MEASURED. Everything not marked +# (S13) still carries 389d9ec's figures and was not re-run here. +# # ⚠️ 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,10 +104,14 @@ 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 (S13) 6 +# the output freeze's EXIT (S13), freeze made unconditional again 4 +# `unasked_power()` discriminates, replaced by `return True` (S13) 6 +# SETTLE_CYCLES from PLANT_SETTLE_S, not _DOWN_S (S13) 9 +# `_SAT_COUNT_CAP` pinned back at release/1.0's 10 (S13) 6 # deadband 5 # quantisation 2 -# saturation detector, `saturated_now = False` 11 +# saturation detector, `saturated_now = False` (S13) 15 # saturation duration (AC 2), fires instantly 2 # sat counter reset on a good cycle 6 # target_grid_w bias 3 @@ -86,7 +119,8 @@ def check(name, cond): # # The detector figure is for the `saturated_now = False` form specifically; # disabling it further down as `frozen = False` is a weaker mutation and gives -# 10. Reproduce the same form or the number will not match. +# 13 on this tree (10 on release/1.0). Reproduce the same form or the number +# will not match. # # ⚠️ IT HAS FOUND A DEAD MECHANISM TWICE, BOTH THE SAME WAY: a clamp standing in # for the mechanism under test. Deleting the integrator freeze once failed @@ -282,8 +316,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 +399,348 @@ 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, and from WHICH HALF of the measurement -------- +# ⚠️ THE DIRECTION IS THE ASSERTION. goodwe/NOTES.md 5.10 measured the same +# 1.78 kW step both ways: ~18 s up, ~23 s down. SAFETY-39 takes the SHORTER +# one because its bound is a REFUSAL and the looser number refuses less. This +# bound asks the opposite question - how long the plant may still be the +# explanation for a divergence - and for that the conservative half is the +# LONGER one. The first version of this file derived it from 18, got 9, and +# that is three cycles short of the plant's own measured down-ramp, in the +# direction that ACCUSES rather than refuses (section 8 below drives it). +check(f"SETTLE_CYCLES is ceil(PLANT_SETTLE_DOWN_S / LOOP_TICK_S) =" + f" {SETTLE_CYCLES}", + SETTLE_CYCLES == 12 + and SETTLE_CYCLES * LOOP_TICK_S >= PLANT_SETTLE_DOWN_S + and (SETTLE_CYCLES - 1) * LOOP_TICK_S < PLANT_SETTLE_DOWN_S) +check("...from the LONGER of the two measured settle times, so it is strictly" + " above what the shorter one would have given", + PLANT_SETTLE_DOWN_S > PLANT_SETTLE_S + and SETTLE_CYCLES > -(-PLANT_SETTLE_S // LOOP_TICK_S)) +# ...and it is NOT SLEW_RAIL_FRAC's reciprocal any more. It was, while both +# were being derived from the same half of the pair, and that coincidence read +# as a derivation. Asserted as an INEQUALITY so the two cannot silently be +# re-fused into one number again. +check("the slew's rail crossing and this bound are now different numbers", + SETTLE_CYCLES > 1.0 / SLEW_RAIL_FRAC) +# 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. +# ⚠️ AND THE COUNT BELOW IS THE RELEASED STATES, NOT THE SWEPT ONES. The first +# version of this check named 41*25*21*5 - the size of the sweep - while +# grading only the subset that actually releases, so the number in the message +# was not the number the assertion was about. It is counted now. +step = float(T.step_w) +swept, past, jumped = 0, [], [] +n_released = 0 +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): + swept += 1 + d = compute(prev, grid, actual, TL, SETTLE_CYCLES, i0) + if d.reason != "freeze-released": + continue + n_released += 1 + 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"of {swept} states swept, the {n_released} that release keep the command" + f" between the previous command and the frozen integrator" + + (f" (e.g. {past[0]})" if past else ""), not past and n_released > 0) +check("...and no released cycle moves further than one slew step" + + (f" (e.g. {jumped[0]})" if jumped else ""), not jumped) + +# --- 7. NO ACCEPTED CONFIGURATION CAN DELETE THE BLOCK --------------------- +# ⚠️ THIS CHECK REPLACES ITS OWN OPPOSITE, AND THE REASON IS WORTH KEEPING. +# While the bound was 9, `saturation_cycles` - which config.yaml accepts up to +# 10 - could be set AT OR ABOVE it, and then the freeze did not engage until +# its own exit was already due: the no-start block silently stopped existing, +# and this file asserted that as a known consequence. Moving the bound to 12 +# put the whole accepted range strictly below it, so the degenerate +# configuration is gone rather than documented. +# +# It is pinned against config.yaml ITSELF and not a transcription, because the +# property is "two files agree" and a transcription is what lets them drift. +# Regex, not PyYAML: the add-on image ships no PyYAML (test_validate.py says +# the same thing at more length). +import os # noqa: E402 +import re # noqa: E402 + +_CFG = os.path.join(os.path.dirname(os.path.abspath(__file__)), "config.yaml") +with open(_CFG, encoding="utf-8") as _fh: + _m = re.search(r"^ saturation_cycles: int\((\d+),(\d+)\)\s*$", + _fh.read(), re.M) +check("config.yaml's saturation_cycles range really was read", _m is not None) +_sc_lo, _sc_hi = int(_m.group(1)), int(_m.group(2)) +check(f"every accepted saturation_cycles ({_sc_lo}..{_sc_hi}) is strictly" + f" below the bound ({SETTLE_CYCLES}), so no accepted configuration can" + f" push the freeze's engagement past its own exit", + _sc_hi < SETTLE_CYCLES) +# Driven at the schema's edge rather than argued: the block still engages, and +# it engages before the exit is due. +TN = Tuning(slew_w=1000.0, saturation_cycles=_sc_hi) +prev, i_w, sat, reasons = 0.0, 0.0, 0, set() +first_freeze = None +for _ in range(20): + d = compute(prev, -2000.0, 700.0, TN, sat, i_w) + reasons.add(d.reason) + if d.reason == "saturated-freeze" and first_freeze is None: + first_freeze = d.sat_count + prev, i_w, sat = d.target_w, d.i_w, d.sat_count +check(f"at the largest accepted saturation_cycles ({_sc_hi}) the block still" + f" engages, at sat_count {first_freeze}, below the bound", + "saturated-freeze" in reasons and first_freeze is not None + and first_freeze < SETTLE_CYCLES) + +# --- 8. THE BOUND'S DIRECTION, DRIVEN ON ONE INVERTER ---------------------- +# ⚠️ WHY THIS SECTION EXISTS. The exit's two conditions are read as "another +# writer is on the actuator". They are not that, and the first shipped form of +# this file said they were. `unasked_power()` compares a reading against ONE +# cycle's command while the pack carries every command before it, so a command +# that has fallen THROUGH ZERO - or simply fallen faster than the pack coasts - +# leaves our own power outside our own interval. With the bound derived from +# the 18 s UP measurement that is not a corner case: it is what a normal load +# step does. +# +# The plant here is a MODEL and is labelled one. It is a first-order lag whose +# two time constants come from the two measured settling times (a first-order +# step is 95 % done at 3 tau, so tau = t / 3), with the ramp cap +# sim/plantsim.py measured on inverter A. It is not the rig and it is not the +# house; what it is for is that the two settling times ENTER the model in the +# same direction they enter the constant, so the check moves if either does. +# +# ⚠️ AND THERE IS NO SECOND WRITER IN THIS SECTION. One `compute()`, one +# plant, one command. Every `freeze-released` below is therefore false. +import math # noqa: E402 + + +class _Plant: + """One inverter, obeying our command and nothing else. A MODEL.""" + + RAMP_W_S = 800.0 # sim/plantsim.py INVERTER_A, measured=True + + def __init__(self, up_s, down_s): + self.a_up = 1.0 - math.exp(-LOOP_TICK_S / (up_s / 3.0)) + self.a_dn = 1.0 - math.exp(-LOOP_TICK_S / (down_s / 3.0)) + self.w = 0.0 + + def tick(self, cmd): + a = self.a_up if abs(cmd) > abs(self.w) else self.a_dn + cap = self.RAMP_W_S * LOOP_TICK_S + self.w += max(-cap, min(cap, a * (cmd - self.w))) + + +from app import control as _control # noqa: E402 + +TS8 = Tuning(max_w=5000.0, slew_w=400.0, target_grid_w=-10.0, + saturation_w=500.0, saturation_cycles=3) + + +def _drive(bound, load1, load2, cycles=60, switch=25): + """Run one closed loop at an arbitrary bound. -> (release cycles, rows).""" + was_b, was_c = _control.SETTLE_CYCLES, _control._SAT_COUNT_CAP + # ⚠️ BOTH, OR THE PROBE LIES: `_SAT_COUNT_CAP` is what `compute()` caps the + # counter with, so moving the bound without it caps the counter BELOW the + # bound and the exit quietly becomes unreachable - which reads as "no false + # positives". That is the hazard `_SAT_COUNT_CAP`'s own comment names. + _control.SETTLE_CYCLES, _control._SAT_COUNT_CAP = bound, bound + 1 + try: + p = _Plant(PLANT_SETTLE_S, PLANT_SETTLE_DOWN_S) + prev, i_w, sat, rows = 0.0, 0.0, 0, [] + for c in range(cycles): + load = load1 if c < switch else load2 + d = compute(prev, load - p.w, p.w, TS8, sat, i_w) + rows.append((c, prev, p.w, d.target_w, d.sat_count, d.reason)) + prev, i_w, sat = d.target_w, d.i_w, d.sat_count + p.tick(prev) + return [r for r in rows if r[5] == "freeze-released"], rows + finally: + _control.SETTLE_CYCLES, _control._SAT_COUNT_CAP = was_b, was_c + + +_SHORT = int(-(-PLANT_SETTLE_S // LOOP_TICK_S)) # what 18 s would give: 9 + +# A 4 kW export falling to 2.5 kW. Nothing exotic; no second writer. +_bad, _rows = _drive(_SHORT, -4000.0, -2500.0) +check(f"a bound derived from the SHORTER measurement ({_SHORT} cycles) fires" + f" the exit on ONE inverter with nothing else on the bus" + + (f" - {len(_bad)} cycle(s), first at cycle {_bad[0][0]}" if _bad else ""), + bool(_bad)) +if _bad: + _c, _prev, _act, _tgt, _sat, _ = _bad[0] + check(f"...and what it called unasked is our own coast-down: command" + f" {_prev:.0f} W, pack still at {_act:.1f} W, and the pack was" + f" commanded {min(r[1] for r in _rows[:_c]):.0f} W a few cycles" + f" earlier", + unasked_power(_prev, _act, TS8.saturation_w) + and abs(_act) > abs(_prev) + and min(r[1] for r in _rows[:_c]) < _act) +_good, _ = _drive(SETTLE_CYCLES, -4000.0, -2500.0) +check(f"the shipped bound ({SETTLE_CYCLES} cycles, the LONGER measurement)" + f" does not fire on the same run", not _good) +# ...and not on any of these, either. The whole grid of load steps the same +# plant can be given, at the tuning every headline figure in this ticket is +# quoted from. +_L = [x * 500.0 for x in range(-8, 9)] +_fired = {} +for _b in (_SHORT, SETTLE_CYCLES): + _fired[_b] = sum(1 for _a in _L for _z in _L + if _a != _z and _drive(_b, _a, _z)[0]) +_pairs = len(_L) * (len(_L) - 1) +check(f"over {_pairs} load-step pairs on the measured plant, one inverter and" + f" no second writer: the short bound fires in {_fired[_SHORT]} of them," + f" the shipped bound in {_fired[SETTLE_CYCLES]}", + _fired[SETTLE_CYCLES] < _fired[_SHORT]) +# ⚠️ AND THE SHIPPED FIGURE IS NOT ZERO IN GENERAL. It is small here because +# the plant in this model is the one that was measured. A plant slower than +# that brings the false release back, and no constant derived from a +# measurement can fix that - it is why DOCS.md tells the operator what +# `freeze-released` is evidence OF, rather than what it proves. + print("SAFETY-04: the i_w=None path is still release/1.0, exactly") @@ -394,8 +777,37 @@ 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"} +# ⚠️ SAFETY-13 ROUND 2 MOVED THE COUNTER'S CEILING, SO THE SWEEP NOW HAS A +# THIRD ANSWER. `_SAT_COUNT_CAP` is `SETTLE_CYCLES + 1`, which was 10 - the +# same literal release/1.0 used - and is now 13. Above release/1.0's own +# ceiling the two counters therefore differ, and NOTHING ELSE MAY: `frozen` is +# `sat_count >= saturation_cycles` and config.yaml accepts that at most 10, so +# a counter reading 13 instead of 10 cannot change it, and no other line in +# `compute()` reads the counter's value. That is asserted rather than asserted- +# by-omission, in `cap_diffs` below. +# +# ⚠️ AND THE SWEEP HAS TO REACH THE BOUND OR IT GRADES NOTHING. `sc` runs to 12 +# now; at 9 the counter tops out one short of the exit and every release +# silently disappears from this table, which is exactly what happened when the +# bound moved and is why the count is asserted and not merely printed. 12 is a +# state release/1.0 could never hold - its counter stopped at 10 - so +# `legacy()` is being fed one input outside its own reachable domain, on +# purpose, and `cap_diffs` is where that shows up. +LEGACY_SAT_CAP = 10 # release/1.0's literal, transcribed in legacy() diffs = [] +released = [] +value_diffs = [] +cap_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 @@ -409,16 +821,69 @@ for tune in (Tuning(slew_w=1000.0), Tuning(slew_w=1000.0, target_grid_w=-10.0), for prev in (-2000.0, -500.0, -100.0, 0.0, 100.0, 500.0, 2000.0): for grid in (-6000.0, -1000.0, -500.0, -14.0, 0.0, 14.0, 500.0, 1000.0, 6000.0): for actual in (-2000.0, 0.0, 600.0, 2000.0): - for sc in (0, 2, 3, 9): + for sc in (0, 2, 3, 9, 12): d = compute(prev, grid, actual, tune, sc) # i_w defaults to None 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 + # everything that reaches the wire, without the counter + wire_same = (got[0], got[2], got[3]) == (was[0], was[2], was[3]) + if wire_same: + # the ceiling moved and nothing else did. Only legal + # above release/1.0's own cap, and only with both + # counters standing at their own ceilings. + if (sc >= LEGACY_SAT_CAP + and was[1] == LEGACY_SAT_CAP + and got[1] == _control._SAT_COUNT_CAP): + cap_diffs.append((prev, grid, actual, sc, got[1], was[1])) + continue + diffs.append((prev, grid, actual, sc, 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 * 5 +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) +# ⚠️ EXACT COUNTS, NOT INEQUALITIES. A weaker form of this line ("some +# releases happened") was written while the bound was moving, and the coverage +# audit caught it immediately: a `unasked_power()` that stops discriminating +# went from 6 checks to 5. The three numbers partition the sweep, so any one of +# them moving is a change somebody has to explain. +check(f"{TOTAL - len(released) - len(cap_diffs)} of {TOTAL} cases are" + f" bit-identical to release/1.0; {len(released)} are releases;" + f" {len(cap_diffs)} differ in the saturation counter alone", + len(released) == 432 and len(cap_diffs) == 81 + and TOTAL - len(released) - len(cap_diffs) == 3267) +# ⚠️ THE COUNTER-ONLY DIFFERENCES ARE NOT A LOOPHOLE, so they are pinned twice: +# they may only occur at an `sc` above release/1.0's ceiling, and the largest +# `saturation_cycles` config.yaml accepts is below BOTH ceilings, so no +# accepted configuration can see the difference at all. +check(f"the {len(cap_diffs)} counter-only differences all sit above" + f" release/1.0's cap of {LEGACY_SAT_CAP}, where no accepted" + f" saturation_cycles can read them", + all(r[3] >= LEGACY_SAT_CAP for r in cap_diffs) and _sc_hi <= LEGACY_SAT_CAP) +# ⚠️ 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.