Files
goodwe-addon/goodwe_controller/DECISIONS/OPT-09-soc-aware-weights.md
T
glenn schrooyenandClaude Opus 5 cab267c4f3
checks / unit (pull_request) Successful in 6m10s
OPT-09: the 6500 W window is a bound the ratings never let the site reach
Round-2 non-blocking findings, both fixed.

⚠️ THE WINDOW FIGURE WAS ARITHMETIC NOBODY HAD RUN. `min_w * (w_A + w_B) / w_B`
= 6500 W is correct about `_fill()`'s PROPORTIONAL regime and is not a
reachable steady state: the healthy pack's own 5000 W rating binds first.

SWEPT on part 8b's own fixture, reproduced rather than transcribed:

    load   equal   OPT-09   site target   A      B
    1000   199/200   0/200       1080    997     83
    2000   199/200   0/200       2170   2003    167
    3000   199/200   0/200       3250   3000    250
    4000   199/200   4/200       4340   4006    334
    4500   199/200   8/200       4890   4514    376
    4900   199/200  41/200       5320   4911    409   <- largest suppressed
    5000   199/200 195/200       8000   5000   3000   <- A at its rating
    6000   199/200 195/200       8000   5000   3000

So the alarm returns BELOW the formula bound, not above it, and the largest
site target reached with the wedged unit still under the 500 W threshold is
5320 W - not 6500. The equal-weights "suppressed below 1000 W" bound is
unreachable for the same reason: the equal arm names the wedged unit 199/200 at
EVERY load swept, 1000 W included.

The formula and its check are kept, because the mechanism they state is right
and it is the mechanism that generalises to other ratings. What was wrong was
the sentence around them, in four places, implying the site passes through the
bound. Each now carries the sweep and says the rating binds first.

⚠️ THIS NUMBER CAME FROM THE REVIEW, NOT FROM THE BRANCH, and was adopted in
good faith by the implementer. The reviewer found it in round 2 and said so.
That is worth recording: a figure arriving from a trusted reviewer got less
scrutiny than one written by the author, and PROC-01's eight fabricated numbers
in two days is now nine. The rule that catches it is the same either way - run
it before you ship it.

Also: the DECISIONS header's base sha, stale for the third time (464d398 ->
3368231 -> 3d29993, moved by two rebases and caught by review each time). It now
records that history, and the header is why `test_translations.py`'s counts are
computed rather than typed.

SAFETY-43 is now named by id where the follow-up is referenced, so a reader can
follow the thread from the disclosure to its closure.

Suite: 25 files, all pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
2026-09-06 23:26:22 +02:00

307 lines
18 KiB
Markdown

# OPT-09 - the weights parameter, and what it is honestly for
**Status:** implemented. **Branch:** `OPT-09`. **Base:** `release/1.0` @ `3d29993` (rebased onto the UX-02 merge; it read `3368231`, and before that `464d398` - each rebase moved it and review caught it each time, which is why the counts in `test_translations.py` are now computed rather than typed)
(`git merge-base HEAD 3368231`; the branch was rebased and this line used to name
the pre-rebase base, `464d398`).
## 0. What was actually wrong
`allocator.plan()` has taken a `weights` argument since ALLOC-01, `_fill()`
consumes it in two passes, and **nothing had ever passed one**. Confirmed on the
base commit:
arbiter.py:535 def resolve(claims, *, units=None, weights=None, live=None, ...)
main.py:2422 resolve(claims, units=self.units, live=self.live_units(), ...)
So `weights` was None on every tick of every install, `plan()` turned that into
1.0 per unit, and the site target was split **50/50 regardless of what either
pack could do**. The card was filed as "per-pack arbitrage sizing"; the architect
pass found the sharper content, and it is the sharper content that this branch is
mostly about.
## 1. Two instruments, and they are not interchangeable
`allocator.py` states the rule this card is built on, and it is mechanical rather
than advisory:
> WEIGHTS EXPRESS PREFERENCE. ENVELOPES EXPRESS PERMISSION. A weight of 0 is
> "please do not use this unit", and it is honoured while any other unit has
> headroom - but it is NOT a constraint.
`_fill()` runs a deliberate **last-resort pass** over the zero-weight units when
the preferred ones saturate, so that a preference can never manufacture a
standing error at the meter. A pack at its own cut-out that is only *weighted*
to zero is therefore still commanded to discharge the moment its sibling fills
up. So this branch ships both:
| | question | mechanism |
|---|---|---|
| `packshare.envelope()` | MAY this pack be asked, at all, in this direction? | a `pack` UNIT claim at `P_SAFETY`, `hi = 0` at the cut-out, `lo = 0` at 100 % |
| `packshare.weights()` | GIVEN that both may, which should carry more? | energy headroom in kWh, handed to `resolve()` |
## 2. ⚠️ What the weights CANNOT do
ARCH-01's convex-combination argument - the gain at the meter is
`sum(a_i * g_i)`, so it lies in `[min g_i, max g_i]` for **any** split -
guarantees that a bad split cannot **destabilise** the site. It says nothing
about whether the site can **regulate**. A unit at its energy limit has
`g_i = 0`; the combination is still inside the interval and the load it was
asked to carry simply does not move.
Measured, `test_packshare.py` part 8, two units rated 2000 W each, B sitting on
its own floor and therefore answering 0 W to any discharge command. The plant is
modelled in that file (`lag_s = 0`, obeys within one cycle, bounded by its
rating) and the loop is the real `compute()`:
| load | arm | site target | shares | grid | unmet |
|---|---|---|---|---|---|
| 1500 W | shipped | +3000 W | A +1500, B +1500 | 0 W | 0 W |
| 1500 W | weights | +1500 W | A +1500, B +0 | 0 W | 0 W |
| 1500 W | envelope | +1500 W | A +1500, B +0 | 0 W | 0 W |
| 3000 W | shipped | +4000 W | A +2000, B +2000 | +1000 W | 0 W |
| 3000 W | weights | +4000 W | A +2000, B +2000 | +1000 W | 0 W |
| 3000 W | envelope | +4000 W | A +2000, B **+0** | +1000 W | **+2000 W** |
Three things are worth reading off that table, and the second and third are the
uncomfortable ones:
1. At a load the healthy pack can cover, the weights alone deliver the whole
improvement. The shipped arm reaches the same meter reading only by winding
the site target to **twice the load** and standing a +1500 W command on a
battery that answers nothing, permanently saturation-frozen. That is not a
working site, it is a site with no headroom left for the next load step.
2. At a load it cannot cover, **the weights stop working entirely**. The
weighted arm and the shipped arm command the identical +2000 W into the empty
pack, because `_fill()`'s last-resort pass hands the remainder to the
zero-weighted unit. Same shares, same watts, same error.
3. **And the envelope does not rescue the delivery either - it names the
shortfall.** Same 2000 W delivered, same 1000 W standing at the meter. What
changes is that the empty pack is commanded 0 W instead of 2000 W, and 2000 W
of site target is *reported* as unplaceable instead of appearing to have been
placed.
> **Weights are a comfort choice for STABILITY and NOT for DELIVERY.** Nothing in
> this add-on shifts a unit's load to its sibling once the sibling is at its
> rating, and weighting a failing unit down is not a remedy for it.
## 2b. ⚠️ The card's own premise is only half true
The card opens with *"a unit that stops delivering its share (empty, full,
curtailed, wedged) parks the site with a standing grid error and no alarm."*
After this branch: the **ask** stops, and while the sibling has headroom the site
regulates to zero error instead of winding to twice the load. The **standing grid
error does not stop** once the sibling is at its rating - §2's table measures
1000 W standing in all three arms - and there is **still no alarm** for it, only
a report. SAFETY-42 will not alarm on a pack at its own floor; that is one of its
acceptance criteria and its evidence is that the benign and the wedged case are
indistinguishable in the power domain.
So the two cards together mean: an empty pack limiting the site is now REPORTED
(`unmet_w`, an action-log line) and still, deliberately, NOT ALARMED. Whether it
should be alarmed is a decision - and the alarm it would need is about a site's
stored ENERGY, not about an inverter's delivery, so it is not this card's and not
SAFETY-42's. Recorded rather than quietly closed against a premise this card only
half meets.
## 3. Which floor, and which ceiling - the two places this could have become policy
**The floor is `bms_soc_floor` alone, never `peakguard.pack_floor_pct()`.** That
function's conjunction with `peak_soc_floor` is right for the peak guard and
wrong here: SAFETY-25 decided (2026-09-02, Option A) that `peak_soc_floor` bounds
the peak guard **only**, ordinary self-consumption discharges straight past it,
and closing an envelope at it would turn an operator preference into a hard stop
on the grid-following loop - the memo's **deferred Option B**, built by accident.
`delivery.py`'s `account()` made the same choice one step out, for the same
reason. Graded: a pack at 15 % with a 10 % cut-out is not closed, though a 20 %
EMS floor would have closed it.
**The ceiling is 100 %, and the coarseness is the honest answer.** There is no
per-pack charge ceiling anywhere in this add-on - `delivery.py` says so at length
and calls it a card, and it is why SAFETY-42 is armed on discharge shares only.
The only other number available is `arbitrage_soc_ceiling`, which is one
site-wide **trade policy**; filing it as a `P_SAFETY` envelope on the loop would
be the mistake above in the charge direction. So the envelope closes only where
the ask is physically impossible, and everything between is left to the weights -
which is the right instrument for a graded preference and is honest about being
only a preference.
## 4. N=1, and the tripwire this had to argue past
`actuator_for()` routes **every** source to the site at one inverter, so a `pack`
interval filed there would bound the SITE TARGET rather than a share. `main.py`
therefore does not file it at all below a roster of two - the same guard, for the
same reason, that SAFETY-30's `sign_site` carries. AC 4 ("at N=1 allocation is
byte-identical") is that `if`.
`test_peakguard.py` part 10f pins the claim-source set against `main.py`'s own
AST precisely so that a state-of-charge-conditioned claim cannot arrive quietly.
`pack` is state-of-charge-conditioned, so the argument is written at that set
rather than the count edited: the number is not an option, the claim is
unit-scoped so it cannot bound the site target, it is not filed at N=1, and part
10f's own note names this as the revisit trigger - *"under ALLOC-01 the reserve
becomes a PER-INVERTER INTERVAL AT THE ALLOCATOR rather than a ninth claim
here"*. Stage 1 is how an interval reaches the allocator and stage 1's input is
claims, so a unit-scoped claim is the shape that sentence asks for.
**What N=1 still does not get, said plainly:** nothing stops the loop discharging
into the BMS cut-out at one inverter. That is unchanged, it is SAFETY-25's
deferred option, and the BMS is what stops it.
## 5. SAFETY-42 - the same hazard from the opposite side
The seam is **the state of charge** - and they overlap in one direction, which
is what the rest of this section is about:
* **OPT-09 stops the site ASKING** a pack whose own SoC says it cannot deliver.
* **SAFETY-42 alarms** when a pack that said it could does not.
A wedged unit reads a healthy SoC, so **no envelope closes** - that half is
absolute, and it is why this card cannot stop the ask on a wedged box. A pack at
its floor is `account()`'s explained case, so SAFETY-42 stays silent - and after
this card that ask does not arrive in the first place, which removes the benign
arm from the detector's input rather than teaching it a new discriminator.
⚠️ **BUT THE WEIGHT DOES MOVE, AND IT NARROWS SAFETY-42's DETECTION WINDOW. THIS
IS A DETECTION REGRESSION AT N=2 AND IT IS NOT FIXED HERE.** An earlier draft of
this section said "no envelope closes and no weight moves - OPT-09 cannot see it
at all". The second clause was **false**, and it was load-bearing for the claim
that the two cards do not overlap. A weight is a function of the pack's ENERGY
HEADROOM, not of what the pack delivers, so a wedged unit's weight moves whenever
its headroom differs from its sibling's - which is the ordinary case this card
exists to exploit. And `delivery.delinquent()` (`app/delivery.py:484`)
names a unit only while its `short_by()` gap exceeds `min_w =
Tuning.saturation_w` - the comparison is `app/delivery.py:531`, and it is
**500 W in absolute watts**. Shrinking a unit's share shrinks its gap. Shrink the
share far enough and the gap drops below 500 W while the unit is still delivering
nothing, and the alarm stops firing.
Measured on the closed loop - real `compute()`, real `plan()`, real
`delinquent()`, modelled plant, B wedged at 15 % against its own 10 % floor so
nothing closes and `account()` explains nothing, i.e. a SAFETY-42 **true
positive** by that card's own definition:
load 1500 W equal weights (shipped) named on 199/200 cycles
shares {a:1500, b:1500} grid +0 W named=['inverter-b']
load 1500 W OPT-09 weights named on 0/200 cycles
shares {a:1505, b: 125} grid -5 W named=(nobody)
⚠️ That plant is modelled in `test_packshare.py`, and **inverter B does not
exist** (expected ~2026-09-12): it is two copies of A driven with different
numbers, and B's 4600 W / 20.48 kWh / 10 % floor are from the datasheet and the
serial. The claim is about the arithmetic of the split and the threshold, which
needs no hardware, and not about a GW5000S-BP.
At a load the healthy pack can cover - the regime where nothing else tells the
operator anything, `unmet_w` is 0, the meter is satisfied and no action-log line
fires - a wedged inverter goes from named on essentially every cycle to **never
named**. The suppression window widens from `site target < 2 * min_w` = 1000 W to
`site target < min_w * (w_A + w_B) / w_B`, which on that fixture is **6500 W** of
a 9600 W site; above it - once the healthy pack is at its own rating and
`_fill()`'s last-resort pass hands the wedged unit a big share again - the alarm
returns. ⚠️ BOTH BOUNDS ARE ARITHMETIC ABOUT `_fill()`'s PROPORTIONAL REGIME AND NEITHER IS A REACHABLE STEADY STATE. Swept on this fixture, the healthy pack's own 5000 W rating binds FIRST: suppressed 0/200 up to a 3000 W load, 4/200 at 4000 W, 41/200 at 4900 W (site target 5320 W, the largest reached with the wedged unit still under 500 W), then 195/200 at 5000 W once `_fill()` pins A at its cap and hands B 3000 W. So the alarm returns BELOW the 6500 W formula bound, not above it. The equal-weights 1000 W bound is unreachable for the same reason - the equal arm names the wedged unit 199/200 at every load swept, 1000 W included.
`test_packshare.py` part 8b grades all of it and **pins the current,
unflattering behaviour**, so that the day it changes a named check moves. The
closure - `delinquent()` measured against the ask rather than a flat 500 W - is
card **SAFETY-43**.
So the seam, stated accurately: **OPT-09 stops the ask AND narrows SAFETY-42's
detection window for a low-headroom unit.** The two cards do not merely meet from
opposite sides; this one takes something away from the other one. Closing the gap
means making `delinquent()` relative to the ask rather than absolute, which is
SAFETY-42's code and a design change - it is filed as its own card, deliberately
not done here, and this paragraph is its input. Until then this is a **disclosed
residual**, in the same voice as SAFETY-13's "2 of 272" and SAFETY-42's own "the
discriminator does not exist": at two inverters, a wedged unit with materially
less energy headroom than its sibling can go unreported at ordinary house loads.
⚠️ **And OPT-09 supplies the mechanism SAFETY-42 said was missing, without
connecting it.** That module's rejection of re-allocation rests on *"the only
re-allocation this add-on can express is ALL OR NOTHING [...] a partial
re-allocation is what would make acting safe, and the allocator has no way to
express it today"*. Passing weights **is** that expression. Wiring delinquency
into them is deliberately **not** done here: it would be acting on the alarm,
which SAFETY-42 refused with rig evidence (a unit derated to 50 % is 400 W worse
dropped than kept, and no instrument this add-on has can tell it from a wedged
one), and - see §2 - a weight cannot stop a unit being commanded anyway. That is
a card, and this paragraph is its input.
## 6. Per-pack arbitrage sizing, and the half that is deliberately not moved
`arbitrage.decide()` was handed `site_max_w` on **both** sides, so a two-pack
site would buy at the sum of the ratings whatever either pack's state of charge
was. It is now sized to the packs whose own SoC permits that direction
(`Controller.trade_power_w()`), using **arbitrage's own** bounds rather than the
BMS cut-out - sizing a trade is the trade's business; permitting a command is the
pack's.
Two honest limits:
* **The sell side is a no-op on every roster today.** `decide()` is handed
`pack_floor_soc` - the emptiest pack - so a sell is refused outright whenever
*any* pack is at or under the floor, i.e. in exactly the states where the sum
could be smaller than the site rating. The arithmetic is graded directly
(`test_packshare.py` part 9); it becomes reachable the day that gate is
per-pack.
* **Moving that gate is not this card's.** Sizing a sell off the *fullest* pack
would let the trade proceed while the emptiest pack was below
`arbitrage_soc_floor`, and the only per-pack instrument that could keep it out
of that pack is the weights - which are a preference, and `_fill()` will hand
it watts anyway. Doing it safely needs an envelope that depends on WHICH CLAIM
WON the tick, which the arbiter's partition does not express. §2 is why that is
a real objection and not a hedge.
`arbitrage.decide()` also now refuses a **0 W sizing** by name (`IDLE_NO_SIZE`),
because a `Claim.set(..., 0.0)` at `P_ARBITRAGE` is a park order that outranks
the loop - the failure the note on `claim()` is already written against. The
sizing can never be zero on a tick that trades today; that is a coincidence of
the existing gate, and this makes it structural instead.
## 7. The fixture, which is the hard part
Two packs that differ in only ONE of SoC and capacity cannot separate a
SoC-aware weighting from a capacity-aware one; two identical packs cannot
separate ANY weighting from any other. `test_packshare.py` part 1 grades the
fixture itself before anything is asserted with it - four candidate schemes
(equal, power rating, SoC only, energy), four distinct splits of the same
1000 W:
equal A +500, B +500
power A +521, B +479
soc only A +125, B +875
energy A +97, B +903
and the counterfactuals: on two identical packs all four collapse to one answer;
at the same SoC, `soc only` collapses onto `equal`; at the same capacity,
`energy` collapses onto `soc only`.
## 8. Provenance of every number
* A: **5000 W, 15.36 kWh, 10 % floor** - measured (GW5000S-BP, register 0x0560).
* B: **4600 W, 20.48 kWh, 10 % floor** - **FROM DATASHEET AND SERIAL**
(`goodwe/NOTES.md` §1.1b, GW5048D-ES `55048ESU22AW0992`). ⚠️ **Inverter B does
not exist** (expected ~2026-09-12).
* Every two-unit run in `test_packshare.py` is arithmetic over intervals and
weights, or a plant modelled in that file. `sim/plantsim.py:216`'s `INVERTER_B`
is **inverter A copied** with `measured=False`, so any two-inverter simulation
result anywhere in this project is **two copies of A**.
* No rig was taken, no hardware was touched, and nothing here is evidence about
a second GoodWe inverter.
## 9. Config
`capacity_kwh` joins the `inverters` roster as an **optional** per-entry key
(`float(0.1,200)?`) - the opposite choice to SAFETY-32's required
`bms_soc_floor`, and for that card's own reason: a missing cut-out would have to
be *defaulted*, and one default for two batteries protects the shallower pack
with the deeper pack's number, whereas a missing capacity has an honest answer -
no opinion about the split, every unit weighs 1.0, which is exactly what shipped.
It is **all or nothing across the roster** and rule 9 refuses a partial one: a
pack measured in kWh beside a sibling weighing a dimensionless 1.0 is not a
comparison, `weights()` answers "no opinion" for the whole roster in that case,
and a key that silently changes nothing is worse than one that refuses to load.
There is deliberately **no flat `capacity_kwh`**: at one inverter there is no
split to weight, so it would be an option an operator can set and nothing reads.