diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index 419a84d..464a770 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -37,8 +37,25 @@ measurement. chases and never reaches. So the target is rounded **once**, at the top, and the fractional parts are settled by **largest remainder** with a deterministic tie-break; every sum below that line is integer arithmetic and cannot drift. - Verified over 250 000 randomised ticks: **0 sum failures, 0 envelope escapes, - 0 W accumulated bias.** + Verified over 250 000 randomised ticks: **0 sum failures, 0 envelope escapes**, + and the one rounding that is left - the single site-level `quantisation_w`, + never distributed into a per-unit bias - totals **-39.8 W over 137 964 + allocations, i.e. -0.0003 W per tick.** +* **Each share is clipped to that unit's interval, and THE TWO ENDS OF THE + INTERVAL ARE APPLIED INDEPENDENTLY.** `UnitBounds()` defaults to + `(-inf, +inf)`, so a one-sided envelope is the ordinary case at N>=2 rather + than an edge. The first cut of this work folded both ends into one + `min(max(v, lo), hi)` and substituted `v` itself for a missing bound, which + reads as "an absent bound cannot bind" but is not what it does: with + `hi == inf` the expression collapses to `min(max(v, lo), v) == v`, so **a + missing ceiling silently discarded the floor.** `_clamp_int(0, 2142, inf)` + returned 0. The worked failure it caused: a unit with a 2000 W floor and no + ceiling, beside a symmetric unit, against a +1000 W site target, was allocated + **+500 W - a quarter of its own floor** - and the sign-coherence refusal that + the same case with a finite ceiling correctly raises was **suppressed** with + it. Fixed by clamping each end on its own, and `UnitPlan.explain()` can no + longer print a share as being inside an interval that excludes it: that line + now reads `OUTSIDE ... - ENVELOPE VIOLATED, DO NOT WRITE`. * **The envelope is read per tick, never cached.** `plan()` is a pure function of that tick's intervals and holds no state, because a unit's range moves for reasons unrelated to the allocator - a stale link, a pack at its floor, a @@ -69,6 +86,19 @@ measurement. **re-aimed, not deleted** - `allocated=False` is now reachable on a mis-routed claim, an empty envelope set, or sign incoherence, and all three are graded on real cycles. +* **And the replay that missed it was widened, so the fix is graded rather than + merely applied.** The 250 000-tick replay drew every envelope as + `B(-uniform(0, rating), uniform(0, rating))` - **finite on both ends and + containing zero, every time** - while the check it fed claimed "no share ever + leaves the envelope it was clamped into". The data could only exercise a + symmetric closed interval around zero, so it reported 0 escapes against a live + defect. The generator now draws from **every shape `UnitBounds` can express**: + half-open above, half-open below, intervals that exclude zero, and bare + `UnitBounds()`. Measured A/B against the defective clamp on the same seed and + the same 250 000 ticks: **the old generator sees 0 escapes, the new one sees + 39 991.** Reverting the clamp fix under the widened generator takes + `test_alloc.py` red on four checks; re-applying it takes it green. The + collapse is now a row in part 12's mutation table. * **Known and not closed here:** at a roster of two the loop's `actual_w` is one inverter's power used as a site measurement, because the add-on binds a single `inverter_power` sensor. That is the *sensing* half of the same N=1 identity diff --git a/goodwe_controller/app/allocator.py b/goodwe_controller/app/allocator.py index aa122cc..78c8b9c 100644 --- a/goodwe_controller/app/allocator.py +++ b/goodwe_controller/app/allocator.py @@ -120,11 +120,46 @@ class UnitPlan: why: str pinned: bool = False clamped: bool = False + dropped: bool = False # not commanded at all - see `within_envelope` + + @property + def span(self) -> str: + return (f"[{'-inf' if self.lo == -inf else format(self.lo, '.0f')}, " + f"{'+inf' if self.hi == inf else format(self.hi, '.0f')}]") + + @property + def within_envelope(self) -> bool: + """Is the number this unit was told ACTUALLY inside the interval? + + ⚠️ THIS IS ASKED, NOT ASSUMED. The record is what SAFETY-28 gates on, + and a record that describes a share it did not check is worth less than + no record: the half-open clamp defect shipped a line reading + `inverter-a -> +500 W in [2000, +inf] (weighted share)` - the record + stating, in the same breath, both the floor and the share below it, and + agreeing with itself. The clamp is fixed, so this should now be true on + every plan the allocator builds; that is exactly why it is cheap to + assert rather than to trust. + + A DROPPED unit is always within: it was told nothing. Its interval may + be a contradiction (`lo > hi`), which no number is inside, and printing + that as a violation would cry wolf on the one case that is already + reported by name. + """ + if self.dropped: + return True + return self.lo <= self.share_w <= self.hi def explain(self) -> str: - span = (f"[{'-inf' if self.lo == -inf else format(self.lo, '.0f')}, " - f"{'+inf' if self.hi == inf else format(self.hi, '.0f')}]") - return f"{self.unit} -> {self.share_w:+d} W in {span} ({self.why})" + if self.dropped: + return (f"{self.unit} -> not commanded, envelope {self.span} " + f"({self.why})") + if not self.within_envelope: + # Unreachable as the allocator stands. If it ever is reachable + # again, the log says so in the line an operator is already reading + # rather than in a line nobody wrote. + return (f"{self.unit} -> {self.share_w:+d} W OUTSIDE {self.span} " + f"({self.why}) - ENVELOPE VIOLATED, DO NOT WRITE") + return f"{self.unit} -> {self.share_w:+d} W in {self.span} ({self.why})" @dataclass(frozen=True) @@ -206,8 +241,28 @@ def _int_envelope(b): def _clamp_int(v, lo, hi): - return min(max(int(v), lo if lo != -inf else int(v)), - hi if hi != inf else int(v)) + """`v` as an int, pulled inside `[lo, hi]`. EACH END IS APPLIED ON ITS OWN. + + ⚠️ THE TWO ENDS ARE INDEPENDENT AND MUST BE TREATED SO. An earlier form + substituted `v` itself for a missing bound and fed both into one + `min(max(...))`, which reads as "an absent bound cannot bind" but is not + what it does: with `hi == inf` the whole expression collapses to + `min(max(v, lo), v) == v`, so a MISSING UPPER BOUND SILENTLY DISCARDED THE + LOWER ONE. `_clamp_int(0, 2142, inf)` returned 0 - a share below a 2142 W + floor the unit itself filed - and every caller here inherited it. + + So: skip the end that is infinite, apply the end that is not, and never let + one end's absence reach the other. `UnitBounds()` defaults to + `(-inf, +inf)`, and this helper is the whole of the ticket's first + deliverable ("each share clipped to that unit's UnitBounds interval"), so a + one-sided envelope is the ordinary case, not an edge. + """ + v = int(v) + if lo != -inf: + v = max(v, lo) + if hi != inf: + v = min(v, hi) + return v def _fill(order, weights, cap, amount): @@ -502,7 +557,7 @@ def plan(target_w, bounds, *, weights=None, live=None, scope_errors=(), drop = dict(dropped).get(u) if drop is not None: plans.append(UnitPlan(u, 0, bounds[u].lo, bounds[u].hi, w[u], - f"DROPPED: {drop}")) + f"DROPPED: {drop}", dropped=True)) continue if u in pinned: by = getattr(bounds[u], "pinned_by", None) or "its own claim" diff --git a/goodwe_controller/test_alloc.py b/goodwe_controller/test_alloc.py index 25d7442..9a9f727 100644 --- a/goodwe_controller/test_alloc.py +++ b/goodwe_controller/test_alloc.py @@ -41,7 +41,7 @@ from math import floor, inf from app.allocator import ( DROP_CONTRADICTION, DROP_NO_INTEGER, DROP_STALE, REFUSE_EMPTY, - REFUSE_SCOPE, REFUSE_SIGN, Allocation, plan, + REFUSE_SCOPE, REFUSE_SIGN, Allocation, UnitPlan, _clamp_int, plan, ) from app.arbiter import ( ACTUATOR_SITE, P_ARBITRAGE, P_LOOP, P_MAINT_SHAPE, P_MAINTENANCE, P_PEAK, @@ -99,47 +99,138 @@ check("...and 995 is not 1990, which is the sentence ARCH-01 is about. A " # check", and it says so because the failure mode is a BIAS - a fraction of a # watt per tick that only shows up in aggregate. A spot check at a round number # cannot see it; 250 000 randomised ticks can. +# +# ⚠️ AND THE GENERATOR IS THE OTHER HALF OF THE CHECK, WHICH THIS FILE GOT +# WRONG ONCE AND IS THE REASON THIS PARAGRAPH EXISTS. The first version drew +# every envelope as `B(-uniform(0, RATING), uniform(0, RATING))` - finite on +# both ends and containing zero, EVERY TIME. The check it fed says "no share +# ever leaves the envelope it was clamped into", which reads as a statement +# about `UnitBounds`; the data could only ever exercise a symmetric closed +# interval around zero. So it reported 0 escapes over 250 000 ticks while a +# reviewer's own 120 000 - drawn over the whole shape space - reported 6 254, +# against a real clamp defect on half-open intervals. +# +# `UnitBounds` DEFAULTS TO `(-inf, +inf)`, so an envelope with one end missing +# is the ordinary case at N>=2, not an edge. `_shape()` therefore draws from +# every shape the type can express, and the counters below are printed so that +# "the replay covers half-open intervals" is a number a reader can see rather +# than a sentence this file asserts about itself. +def _shape(rnd): + """One random envelope, drawn across EVERY SHAPE `UnitBounds` CAN HOLD.""" + kind = rnd.randrange(9) + if kind <= 2: # finite, straddling zero + return B(-rnd.uniform(0.0, RATING), rnd.uniform(0.0, RATING)) + if kind == 3: # a FLOOR and no ceiling + return B(rnd.uniform(-RATING, RATING), inf) + if kind == 4: # a ceiling and no floor + return B(-inf, rnd.uniform(-RATING, RATING)) + if kind == 5: # finite, EXCLUDES zero, +ve + _lo = rnd.uniform(1.0, RATING) + return B(_lo, _lo + rnd.uniform(0.0, RATING)) + if kind == 6: # finite, EXCLUDES zero, -ve + _hi = -rnd.uniform(1.0, RATING) + return B(_hi - rnd.uniform(0.0, RATING), _hi) + if kind == 7: # half-open AND excludes zero + return (B(rnd.uniform(1.0, RATING), inf) if rnd.random() < 0.5 + else B(-inf, -rnd.uniform(1.0, RATING))) + return B() # bare: (-inf, +inf) + + +def _half_open(b): + return (b.lo == -inf) != (b.hi == inf) + + +def _excludes_zero(b): + return b.lo > 0.0 or b.hi < 0.0 + + random.seed(20260902) _N = 250_000 _bad_sum = _bad_env = _refused = 0 -_bias = 0 -_span = 0 +_refused_ordinary = _ordinary = 0 +_bad_record = 0 +_quant = 0.0 +_span = _n_open = _n_half = _n_nozero = _n_bare = 0 +_esc_half = _esc_nozero = 0 for _i in range(_N): _S = random.uniform(-6000.0, 6000.0) - _ba = B(-random.uniform(0.0, RATING), random.uniform(0.0, RATING)) - _bb = B(-random.uniform(0.0, RATING), random.uniform(0.0, RATING)) + _ba, _bb = _shape(random), _shape(random) + _env = {A: _ba, Bee: _bb} + for _b in (_ba, _bb): + _n_half += _half_open(_b) + _n_nozero += _excludes_zero(_b) + _n_bare += (_b.lo == -inf and _b.hi == inf) + _n_open += (_b.lo == -inf or _b.hi == inf) + # AN ORDINARY TICK: no pin, and every envelope contains zero. That is the + # population part 6 asks the sign gate never to fire on - and it is a + # POPULATION now rather than the whole replay, because the widened shapes + # include the forced floors the gate exists for. + _ord = all(_b.lo <= 0.0 <= _b.hi for _b in (_ba, _bb)) + _ordinary += _ord _wa = random.random() - _p = plan(_S, {A: _ba, Bee: _bb}, weights={A: _wa, Bee: 1.0 - _wa}) + _p = plan(_S, _env, weights={A: _wa, Bee: 1.0 - _wa}) if not _p.ok: _refused += 1 + _refused_ordinary += _ord continue if sum(_p.shares.values()) + _p.unmet_w != _p.sum_w: _bad_sum += 1 for _u, _s in _p.shares.items(): - _b = {A: _ba, Bee: _bb}[_u] - if not (_b.lo - 1.0 <= _s <= _b.hi + 1.0): + _b = _env[_u] + # ⚠️ NO TOLERANCE. `_int_envelope()` narrows INWARD, so every integer + # the allocator may command is inside the real interval - there is + # nothing for a slack of a watt to absorb except a defect. + if not (_b.lo <= _s <= _b.hi): _bad_env += 1 + _esc_half += _half_open(_b) + _esc_nozero += _excludes_zero(_b) + # AND THE RECORD IS GRADED AGAINST THE SAME REPLAY, because the defect that + # produced this generator also produced a `UnitPlan.explain()` line reading + # `inverter-a -> +500 W in [2000, +inf]` - the record agreeing with itself + # about a share the interval excludes. + if not all(_pl.within_envelope for _pl in _p.plans): + _bad_record += 1 + _quant += _p.quantisation_w if _p.unmet_w == 0: - _bias += _p.sum_w - sum(_p.shares.values()) _span += 1 print(f" replayed {_N} ticks at random targets, random envelopes and random " f"weights: {_bad_sum} sum failures, {_bad_env} envelope escapes, " f"{_refused} refusals") +print(f" envelope shapes drawn ({2 * _N} in total): {_n_open} with an " + f"infinite end, of which {_n_half} HALF-OPEN and {_n_bare} bare " + f"UnitBounds(); {_n_nozero} that EXCLUDE zero") check("⚠️ OVER A QUARTER OF A MILLION TICKS THE SUM IS EXACT EVERY TIME. Not " "within a tolerance - EXACT - because `sum_w` is an int, the shares are " "ints and the leftover is settled in integer arithmetic, so there is no " "float in the identity for an error to accumulate in", _bad_sum == 0) check("...and no share ever leaves the envelope it was clamped into, over the " - "same replay. An allocator that meets its sum by over-driving one unit " - "past its rating has met the wrong invariant", + "same replay - INCLUDING the half-open ones and the ones that exclude " + "zero, which is the coverage this check claimed before it had it. An " + "allocator that meets its sum by over-driving one unit past its rating " + "has met the wrong invariant", _bad_env == 0) -check("⚠️ AND THE ACCUMULATED BIAS IS ZERO, WHICH IS THE POINT OF LARGEST " - f"REMAINDER. Summed over {_span} unconstrained ticks the total shortfall " - "is 0 W. A 1 W systematic bias at 1-2 Hz is not a rounding artefact - it " - "is a permanent standing error with the site integrator wound around it, " - "chasing a target the allocator will never let it reach", - _bias == 0) +check("...and the replay is NOT VACUOUS about that: it really did draw " + f"{_n_half} half-open intervals, {_n_bare} bare `UnitBounds()` and " + f"{_n_nozero} intervals excluding zero. Revert `_clamp_int` to the form " + "that substitutes `v` for a missing bound and this check goes red on the " + "half-open draws; under the old generator it could not", + _n_half > 10_000 and _n_bare > 10_000 and _n_nozero > 10_000) +check("...and every plan in the record AGREES with the share it carries: no " + "`UnitPlan` was built asserting a number is inside an interval that " + "excludes it. The record is what SAFETY-28 gates on, so a record that " + "describes a share it did not check is worth less than no record", + _bad_record == 0) +check("⚠️ AND THE ONLY ROUNDING LEFT IS THE SITE-LEVEL ONE, WHICH DOES NOT " + f"DRIFT. Over {_N - _refused} allocations the total of " + f"`quantisation_w` - `sum_w - target_w`, at most half a watt each and " + f"NEVER distributed into a per-unit bias - is {_quant:+.1f} W, i.e. " + f"{_quant / max(1, _N - _refused):+.5f} W per tick. A 1 W systematic " + "bias at 1-2 Hz is not a rounding artefact - it is a permanent standing " + "error with the site integrator wound around it. (Largest remainder " + "against the per-unit alternative is measured in part 2; the sum " + f"identity above holds on all {_span} unconstrained ticks.)", + abs(_quant) / max(1, _N - _refused) < 0.01) # ============================================================================== @@ -371,10 +462,13 @@ check("the same pin with a site target BEYOND it is fine, because the shares " Bee: B(-RATING, RATING)}, site_priority=P_LOOP).shares == {A: 2500, Bee: 500}) check("⚠️ AND IT NEVER FIRES ON AN ORDINARY TICK, which is what makes it a gate " - "rather than a nuisance. Over the 250 000-tick replay in part 1 - random " - "targets, random envelopes, random weights, no pins - it refused " - f"{_refused} times", - _refused == 0) + "rather than a nuisance. Of the 250 000-tick replay in part 1 - random " + f"targets, random envelopes, random weights, no pins - {_ordinary} ticks " + "drew envelopes that all CONTAIN zero, half-open ones and bare " + f"`UnitBounds()` among them, and it refused {_refused_ordinary} of " + f"those. The other {_refused} refusals in that replay are this gate doing " + "its job on the forced floors the widened generator now draws", + _refused_ordinary == 0 and _ordinary > 50_000) check("...and it cannot: with no pin and an envelope that contains zero, the " "fill only ever moves shares in ONE direction away from zero, so two " "signs are not expressible. The gate exists for the envelope that " @@ -382,6 +476,58 @@ check("...and it cannot: with no pin and an envelope that contains zero, the " all(v >= 0 for v in plan(1990.0, TWO).shares.values()) and all(v <= 0 for v in plan(-1990.0, TWO).shares.values())) +# ⚠️ THE HALF-OPEN ENVELOPE, WHICH IS WHERE THIS GATE WAS ONCE SILENTLY +# DISARMED - and the ticket's first named deliverable, "each share clipped to +# that unit's UnitBounds interval", with it. `_clamp_int()` used to substitute +# `v` for a missing bound and feed both ends into one `min(max(...))`; with +# `hi == inf` that collapses to `min(max(v, lo), v) == v`, so THE FLOOR WAS +# DISCARDED BY THE ABSENCE OF A CEILING. The worked case: inverter-a with a +# 2000 W floor and no ceiling, inverter-b symmetric, a +1000 W site target. +_HALF = {A: B(2000.0, inf), Bee: B(-5000.0, 5000.0)} +_FIN = {A: B(2000.0, 9_000_000.0), Bee: B(-5000.0, 5000.0)} +_half, _fin = plan(1000.0, _HALF), plan(1000.0, _FIN) +print(f" floor 2000 W, NO ceiling: {_half.explain()}") +print(f" the same with a finite ceiling: {_fin.explain()}") +check("⚠️ A ONE-SIDED ENVELOPE REFUSES EXACTLY AS THE TWO-SIDED ONE DOES. Both " + "put inverter-a at its 2000 W floor against a +1000 W site target, so " + "inverter-b must charge for the shares to sum - circulating power, and " + "the gate fires. The defect made the half-open case return a=+500, " + "b=+500 instead: inverter-a commanded at a quarter of its own floor, and " + "a refusal that should have fired suppressed with it", + _half.ok is False and REFUSE_SIGN in _half.refused + and _fin.ok is False and REFUSE_SIGN in _fin.refused) +check("...and the two ends are applied INDEPENDENTLY, which is the shape of the " + "fix rather than a patch of the one case above: a missing ceiling does " + "not discard the floor, a missing floor does not discard the ceiling, and " + "a bare `UnitBounds()` binds nothing", + _clamp_int(0, 2142, inf) == 2142 + and _clamp_int(9999, 2142, inf) == 9999 + and _clamp_int(0, -inf, -500) == -500 + and _clamp_int(-9999, -inf, -500) == -9999 + and _clamp_int(-7, -inf, inf) == -7 + and _clamp_int(0, 2142, 5000) == 2142) +check("...and the RECORD can no longer assert a share is inside an interval " + "that excludes it. The defect's log line read `inverter-a -> +500 W in " + "[2000, +inf] (weighted share)` - the record naming the floor and the " + "share beneath it in one breath and agreeing with itself. If that state " + "is ever reachable again the line says OUTSIDE, and says it in the line " + "an operator is already reading", + UnitPlan(A, 500, 2000.0, inf, 0.5, "weighted share").within_envelope + is False + and "OUTSIDE" in UnitPlan(A, 500, 2000.0, inf, 0.5, "x").explain() + and "DO NOT WRITE" in UnitPlan(A, 500, 2000.0, inf, 0.5, "x").explain() + and "in [2000, +inf]" in UnitPlan(A, 2500, 2000.0, inf, 0.5, + "weighted share").explain()) +check("...and a DROPPED unit is not shouted about: it was commanded nothing, " + "and its interval may be a contradiction that no number is inside. That " + "case is already reported by name, and a violation banner on it would be " + "the guard crying wolf on the one row that is working", + "not commanded" in UnitPlan(A, 0, 3000.0, -3000.0, 0.0, + f"DROPPED: {DROP_CONTRADICTION}", + dropped=True).explain() + and UnitPlan(A, 0, 3000.0, -3000.0, 0.0, "DROPPED: x", + dropped=True).within_envelope is True) + # ============================================================================== print() @@ -784,6 +930,22 @@ MUT.append(("the site target TRUNCATED instead of rounded - a bias toward zero " max(map(abs, _q)) <= 0.5, "part 2: '|quantisation| is never more than half a watt'")) +# ⚠️ THE ONE THAT SHIPPED, AND THE ROW THIS TABLE IS MOST INDEBTED TO. This +# mutation IS the code that was on `186e13c`, so it is not hypothetical: the +# question it answers is not "would a check catch it" but "why did none". +_collapsed = min(max(int(0), 2142), 0) # the old form, evaluated at hi=inf +MUT.append(("the two ends of `_clamp_int` collapsed back into one min(max()), " + "so a MISSING CEILING discards the FLOOR", + _collapsed == 0 and _clamp_int(0, 2142, inf) == 2142 + and _half.ok is False, + "part 6: 'a one-sided envelope refuses exactly as the two-sided " + "one does', and part 1's replay - 39 991 envelope escapes over the " + "same 250 000 ticks. ⚠️ AND ZERO ESCAPES UNDER THE GENERATOR THIS " + "FILE ORIGINALLY SHIPPED WITH, which drew every envelope finite on " + "both ends and containing zero: the mutation was already live and " + "the table would have called it caught. The fix is graded by the " + "SHAPES the generator can express, not by the number of ticks")) + # ⚠️ AND NOW THE ONES THAT DO NOT BITE. Three of them, found by actually running # the mutations rather than by reasoning about them, and each is reported with # WHY - because a mutation table that only lists its successes is the same