From a9d249ce6113e6a24ba1f41c48971959f8432f9d Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Tue, 4 Aug 2026 13:57:31 +0200 Subject: [PATCH] fix(models): version learned PV and load state against its feature vector Both learned models restored by unmarshalling a bare blob, gated only on a single sanity field, with nothing recording which feature vector the coefficients were fitted against. Change the harmonic count, the bucket a moment maps to, or what clear-sky irradiance refers to, and a stale Beta keeps predicting with a plausible-looking MAE from a fit that describes a different world. Each persisted model is now wrapped in {schema_version, feature_hash, model}. The fingerprint is derived from the feature functions themselves, evaluated over a fixed probe, so a change to the feature math moves it with nobody having to remember a constant; a short declared-semantics label next to the feature definition covers the one thing a probe cannot see, a caller passing in a differently defined quantity. On mismatch the model logs both hashes at Info and cold starts, which both models recover from and neither recovers from silent wrong coefficients. Pre-envelope state is adopted, but only while the running build still computes the space it was fitted against: each package freezes the fingerprint that was current when the envelope landed, and the first feature change retires that path on its own. Co-Authored-By: Claude Opus 5 --- .changeset/versioned-learned-model-state.md | 5 + go/internal/loadmodel/model.go | 77 ++++++- go/internal/loadmodel/persistence_test.go | 242 ++++++++++++++++++++ go/internal/loadmodel/service.go | 69 ++++-- go/internal/modelstate/modelstate.go | 191 +++++++++++++++ go/internal/modelstate/modelstate_test.go | 204 +++++++++++++++++ go/internal/pvmodel/model.go | 53 +++++ go/internal/pvmodel/persistence_test.go | 227 ++++++++++++++++++ go/internal/pvmodel/service.go | 39 +++- go/internal/pvmodel/service_test.go | 11 +- 10 files changed, 1075 insertions(+), 43 deletions(-) create mode 100644 .changeset/versioned-learned-model-state.md create mode 100644 go/internal/loadmodel/persistence_test.go create mode 100644 go/internal/modelstate/modelstate.go create mode 100644 go/internal/modelstate/modelstate_test.go create mode 100644 go/internal/pvmodel/persistence_test.go diff --git a/.changeset/versioned-learned-model-state.md b/.changeset/versioned-learned-model-state.md new file mode 100644 index 000000000..d41eb4db4 --- /dev/null +++ b/.changeset/versioned-learned-model-state.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Learned PV and load state is now stored with a fingerprint of the feature vector it was fitted against, and is discarded when that fingerprint no longer matches. Coefficients only mean something against the features that produced them: change the number of time-of-day harmonics, the bucket a moment maps to, or what clear-sky irradiance refers to, and the old model keeps predicting — with a plausible-looking error — from a fit that describes a different world. Nothing in the stored numbers gave that away before. The fingerprint is derived from the feature functions themselves, so a code change to the features moves it without anyone having to remember. On a mismatch the model logs both fingerprints and cold-starts, which both models recover from; wrong coefficients they do not recover from. Existing state is carried over unchanged on upgrade. diff --git a/go/internal/loadmodel/model.go b/go/internal/loadmodel/model.go index faf5ac991..d1c6ebf38 100644 --- a/go/internal/loadmodel/model.go +++ b/go/internal/loadmodel/model.go @@ -32,7 +32,10 @@ package loadmodel import ( "math" + "sync" "time" + + "github.com/srcfl/ftw/go/internal/modelstate" ) // Buckets is the number of hour-of-week buckets: 7 days × 24 hours. @@ -217,6 +220,69 @@ func HourOfWeek(t time.Time) int { return wd*24 + u.Hour() } +// heatingGain is the load a learned slope predicts at an outdoor +// temperature: linear in the shortfall below the reference, zero above it. +// One definition, used by both Predict and Update and probed by +// featureProbe — HeatingW_per_degC means nothing except against this shape. +func heatingGain(coefWPerDegC, tempC float64) float64 { + if tempC >= HeatingReferenceC { + return 0 + } + return coefWPerDegC * (HeatingReferenceC - tempC) +} + +// featureSemantics declares what the numbers this model learns from mean. It +// is the half of the fingerprint a probe cannot derive: change what the +// sampler subtracts before calling Update — stop netting out the EV, say — +// and every bucket mean is a measurement of something else, while nothing in +// the model's own code has moved. +// +// CHANGE THIS STRING in the commit that changes what a caller feeds in. +// Changes to the bucket indexing or the heating shape need no edit here — +// featureProbe moves the fingerprint on its own. +const featureSemantics = "loadmodel/1 load=site_w_less_pv_bat_ev_v2x temp=outdoor_c target=house_w" + +// featureProbe pins the two things whose change would invalidate stored +// coefficients: which bucket a moment maps to, and the shape the heating +// slope is measured against. +// +// The instants are given in a non-UTC zone and sit near midnight on purpose. +// Drop the UTC coercion in HourOfWeek and both the hour and the weekday move +// for those — which is precisely the defect commit 3255deba fixed, the one +// that silently misaligned every learned bucket across a DST change. +// +// Deliberately absent: typicalPrior. A bucket mean is measured watts and stays +// meaningful when the prior it started from is retuned; the prior only sets +// the fallback for buckets nobody has observed yet. Discarding months of +// learned buckets over a prior tweak would cost more than it protects. +func featureProbe() []float64 { + out := []float64{float64(Buckets), HeatingReferenceC} + zone := time.FixedZone("probe", 2*60*60) + for _, t := range []time.Time{ + time.Date(2024, 1, 1, 0, 30, 0, 0, time.UTC), + time.Date(2024, 3, 31, 1, 30, 0, 0, zone), + time.Date(2024, 6, 21, 23, 45, 0, 0, zone), + time.Date(2024, 10, 27, 0, 15, 0, 0, zone), + time.Date(2024, 12, 24, 18, 0, 0, 0, time.UTC), + } { + out = append(out, float64(HourOfWeek(t))) + } + for _, tempC := range []float64{-20, -3, 0, 10, 17.5, 18, 25} { + out = append(out, heatingGain(1, tempC)) + } + return out +} + +var featureHash = sync.OnceValue(func() string { + return modelstate.Fingerprint(featureSemantics, featureProbe()) +}) + +// FeatureHash fingerprints the feature space the bucket means and the heating +// slope are fitted against. Stored state is only restored when its recorded +// hash matches this one; see internal/modelstate for why, and service.go for +// what happens when it does not. +func FeatureHash() string { return featureHash() } + // Predict returns the expected load (W, non-negative) at time t with // outdoor temperature tempC (0 if unknown). Blends per-bucket EMA with // the typical prior by sample count, then adds the heating correction. @@ -229,11 +295,7 @@ func (m Model) Predict(t time.Time, tempC float64) float64 { } prior := m.prior(idx) base := trust*b.Mean + (1-trust)*prior - heating := 0.0 - if tempC < HeatingReferenceC { - heating = m.HeatingW_per_degC * (HeatingReferenceC - tempC) - } - y := base + heating + y := base + heatingGain(m.HeatingW_per_degC, tempC) if y < 0 { return 0 } @@ -321,10 +383,7 @@ func (m *Model) Update(t time.Time, actualLoadW, tempC float64) (updated bool) { // though a real baseline — fridge, server, standby — always exists). // Instead, skip the bucket update entirely for this sample and let // existing Samples + Mean stand. Global Samples and MAE still update. - heatEst := 0.0 - if tempC < HeatingReferenceC { - heatEst = m.HeatingW_per_degC * (HeatingReferenceC - tempC) - } + heatEst := heatingGain(m.HeatingW_per_degC, tempC) if heatEst < actualLoadW { baseSample := actualLoadW - heatEst if b.Samples < 10 { diff --git a/go/internal/loadmodel/persistence_test.go b/go/internal/loadmodel/persistence_test.go new file mode 100644 index 000000000..f0a7b3701 --- /dev/null +++ b/go/internal/loadmodel/persistence_test.go @@ -0,0 +1,242 @@ +package loadmodel + +import ( + "encoding/json" + "path/filepath" + "testing" + "time" + + "github.com/srcfl/ftw/go/internal/modelstate" + "github.com/srcfl/ftw/go/internal/state" + "github.com/srcfl/ftw/go/internal/telemetry" +) + +func openTestDB(t *testing.T) *state.Store { + t.Helper() + st, err := state.Open(filepath.Join(t.TempDir(), "state.db")) + if err != nil { + t.Fatalf("open state: %v", err) + } + t.Cleanup(func() { st.Close() }) + return st +} + +// trainedModel returns a model with a recognisable amount of learning in it. +func trainedModel(t *testing.T) *Model { + t.Helper() + m := newProfileModel(4000, ProfileHome) + m.HeatingW_per_degC = 275 + t0 := time.Date(2026, 1, 5, 12, 0, 0, 0, time.UTC) + for i := 0; i < 12; i++ { + m.Update(t0.AddDate(0, 0, 7*i), 2400, HeatingReferenceC) + } + if m.Samples != 12 { + t.Fatalf("fixture did not train: samples = %d", m.Samples) + } + return m +} + +// requireLegacyAdoption skips a case that depends on pre-envelope state still +// being adopted. Once the features move, that state is correctly discarded and +// the case stops existing — the migration and the tests that cover it retire +// together, so a feature change never leaves a test demanding the old +// behaviour back. +func requireLegacyAdoption(t *testing.T) { + t.Helper() + if FeatureHash() != legacyFeatureHash { + t.Skip("features have moved on; unversioned state is discarded, as designed") + } +} + +// TestFeatureHashPinned is a tripwire, not a rule. Changing the bucket +// indexing or the heating shape is allowed, but it makes every deployed site +// discard its learned week — and unlike the PV twin, which relearns in an +// afternoon, bucket coverage is rebuilt over weeks. That should be a +// decision, not a surprise found in production. +// +// If this test fails: confirm the change is intended, update the literal +// below, and say in the changeset that the load model cold-starts on upgrade. +// Do NOT touch legacyFeatureHash in service.go — that constant is frozen on +// purpose, and moving it would restore pre-envelope coefficients under the +// new features, which is the exact fault this guards against. +func TestFeatureHashPinned(t *testing.T) { + const want = "f79385ff0412d66b" + if got := FeatureHash(); got != want { + t.Errorf("load feature hash = %q, pinned at %q\n"+ + "the feature definition changed: every deployed site will cold-start", got, want) + } +} + +// The hash must move on its own when the model's definitions do, so nobody +// has to remember to bump a constant. +func TestFeatureHashTracksTheFeatureDefinition(t *testing.T) { + base := modelstate.Fingerprint(featureSemantics, featureProbe()) + if p := featureProbe(); p[0] != float64(Buckets) || p[1] != HeatingReferenceC { + t.Fatalf("featureProbe layout changed; update this test: %v", p[:2]) + } + + // Bucket indexing moving off UTC — the 3255deba regression, which + // silently misaligned every learned bucket across a DST change. + shifted := featureProbe() + shifted[2]++ // the first HourOfWeek probe value + if modelstate.Fingerprint(featureSemantics, shifted) == base { + t.Error("a change to bucket indexing must move the hash") + } + + // The heating shape the slope is measured against. + steeper := featureProbe() + steeper[1] = HeatingReferenceC + 2 + if modelstate.Fingerprint(featureSemantics, steeper) == base { + t.Error("a change to the heating reference must move the hash") + } + + // The semantics half: same math, different meaning for the sampled load. + if modelstate.Fingerprint("loadmodel/1 load=site_w_less_pv_bat", featureProbe()) == base { + t.Error("redeclaring what an input means must move the hash") + } +} + +// Retuning typicalPrior must NOT cold-start a site. A bucket mean is measured +// watts and stays meaningful when the prior it started from changes; the +// prior only supplies the fallback for buckets nobody has observed. Weighed +// against weeks of lost coverage, that trade is deliberate — see featureProbe. +func TestPriorIsNotPartOfTheFeatureIdentity(t *testing.T) { + probe := featureProbe() + for bucket := 0; bucket < Buckets; bucket++ { + p := typicalPrior(bucket) + for i, v := range probe { + if v == p { + t.Fatalf("probe[%d] carries typicalPrior(%d) = %v: retuning the "+ + "prior would cold-start every site for no safety gain", i, bucket, p) + } + } + } +} + +func TestPersistedStateRoundTrips(t *testing.T) { + st := openTestDB(t) + + s := NewService(st, telemetry.NewStore(), "site", 4000, 17250) + s.mu.Lock() + s.models[ProfileHome] = trainedModel(t) + s.mu.Unlock() + if err := s.persist(); err != nil { + t.Fatalf("persist: %v", err) + } + + restored := NewService(st, telemetry.NewStore(), "site", 4000, 17250).Model() + if restored.Samples != 12 { + t.Fatalf("samples = %d, want 12: matching feature hash must restore", restored.Samples) + } + if restored.HeatingW_per_degC != 275 { + t.Errorf("heating slope = %v, want 275", restored.HeatingW_per_degC) + } +} + +// The bug this PR exists for: bucket means and a heating slope fitted against +// one feature definition must not be restored into a build that computes +// another. They parse, they look sane, and they steer the plan. +func TestStateFittedAgainstOtherFeaturesColdStarts(t *testing.T) { + st := openTestDB(t) + + js, err := modelstate.Wrap("0000badfeature00", trainedModel(t)) + if err != nil { + t.Fatal(err) + } + if err := st.SaveConfig(stateKey(ProfileHome), js); err != nil { + t.Fatal(err) + } + + got := NewService(st, telemetry.NewStore(), "site", 4000, 17250).Model() + + if got.Samples != 0 { + t.Errorf("samples = %d, want 0: stale coefficients must not survive a feature change", got.Samples) + } + if got.HeatingW_per_degC != 0 { + t.Errorf("heating slope = %v, want 0 on a cold start", got.HeatingW_per_degC) + } +} + +// State written before the envelope existed is adopted, on both the +// per-profile key and the pre-profile one. Upgrading must not cost every site +// its learned week. +func TestUnversionedStateIsAdopted(t *testing.T) { + requireLegacyAdoption(t) + for name, key := range map[string]string{ + "per-profile key": stateKey(ProfileHome), + "pre-profile key": legacyStateKey, + } { + t.Run(name, func(t *testing.T) { + st := openTestDB(t) + bare, err := json.Marshal(trainedModel(t)) // pre-envelope on-disk shape + if err != nil { + t.Fatal(err) + } + if err := st.SaveConfig(key, string(bare)); err != nil { + t.Fatal(err) + } + + got := NewService(st, telemetry.NewStore(), "site", 4000, 17250).Model() + + if got.Samples != 12 { + t.Fatalf("samples = %d, want 12: unversioned state should be adopted", got.Samples) + } + if got.PeakW != 4000 || got.MaxPlausibleW != 17250 { + t.Errorf("config-owned fields not reapplied: peak %v, max %v", got.PeakW, got.MaxPlausibleW) + } + }) + } +} + +// The pre-profile key is the oldest state on any box. It now runs the same +// bucket repair the per-profile path has always run, so a model poisoned by +// the pre-guard heating-subtraction bug is repaired whichever key it sits in. +func TestLegacyKeyGetsTheBucketRepair(t *testing.T) { + requireLegacyAdoption(t) + st := openTestDB(t) + + poisoned := trainedModel(t) + poisoned.Bucket[0].Mean = 15 // prior for bucket 0 is ~300 W + poisoned.Bucket[0].Samples = 400 + bare, err := json.Marshal(poisoned) + if err != nil { + t.Fatal(err) + } + if err := st.SaveConfig(legacyStateKey, string(bare)); err != nil { + t.Fatal(err) + } + + got := NewService(st, telemetry.NewStore(), "site", 4000, 17250).Model() + + if got.Bucket[0].Mean != got.prior(0) { + t.Errorf("bucket 0 mean = %.1f, want the prior %.1f", got.Bucket[0].Mean, got.prior(0)) + } +} + +// A damaged blob on the boot path of a control system cold-starts. It does +// not panic, and it does not leave a half-decoded model behind. +func TestCorruptStateColdStarts(t *testing.T) { + for name, js := range map[string]string{ + "truncated envelope": `{"schema_version":1,"feature_hash":"f79385ff0412d66b","model":{"samples":`, + "not json": "\x00\x01\x02 not json", + "wrong shape": `[1,2,3]`, + "no model": `{"schema_version":1,"feature_hash":"f79385ff0412d66b"}`, + "future schema": `{"schema_version":99,"feature_hash":"f79385ff0412d66b","model":{"samples":900,"alpha":0.1}}`, + "half-decoded": `{"bucket":"not-an-array","samples":900,"alpha":0.1}`, + "no alpha": `{"samples":900}`, + } { + t.Run(name, func(t *testing.T) { + st := openTestDB(t) + if err := st.SaveConfig(stateKey(ProfileHome), js); err != nil { + t.Fatal(err) + } + got := NewService(st, telemetry.NewStore(), "site", 4000, 17250).Model() + if got.Samples != 0 { + t.Errorf("samples = %d, want 0 (cold start)", got.Samples) + } + if got.Alpha != newProfileModel(4000, ProfileHome).Alpha { + t.Errorf("alpha = %v, want the cold-start value", got.Alpha) + } + }) + } +} diff --git a/go/internal/loadmodel/service.go b/go/internal/loadmodel/service.go index 5fe9b3f77..27fef030b 100644 --- a/go/internal/loadmodel/service.go +++ b/go/internal/loadmodel/service.go @@ -2,13 +2,13 @@ package loadmodel import ( "context" - "encoding/json" "fmt" "log/slog" "strings" "sync" "time" + "github.com/srcfl/ftw/go/internal/modelstate" "github.com/srcfl/ftw/go/internal/state" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -28,6 +28,19 @@ const ( profileStateKey = "loadmodel/profile" ) +// legacyFeatureHash is the fingerprint of the feature space in force when the +// envelope was introduced. State written before then — both the per-profile +// keys and legacyStateKey — carries no fingerprint, but it was fitted against +// exactly this space, so it is still worth restoring, and only while the +// running build still computes that space. +// +// This constant is frozen. The first change to the bucket indexing or the +// heating shape moves FeatureHash() away from it, and unversioned state is +// discarded from then on, which is the whole point. Never update it to match +// a new FeatureHash(): that would re-arm the migration under features the old +// coefficients were never fitted against. +const legacyFeatureHash = "f79385ff0412d66b" + func stateKey(profile Profile) string { return stateKeyPrefix + string(profile) } // ParseProfile normalizes a user/API supplied load-model profile. @@ -86,7 +99,7 @@ func NewService(st *state.Store, tel *telemetry.Store, siteMeter string, peakW, } for _, profile := range Profiles() { if js, ok := st.LoadConfig(stateKey(profile)); ok && js != "" { - if m, ok := restoreModel(js, peakW, maxPlausibleW, profile); ok { + if m := restoreModel(js, peakW, maxPlausibleW, profile); m != nil { s.models[profile] = m loadedProfiles[profile] = true slog.Info("loadmodel restored", @@ -95,20 +108,16 @@ func NewService(st *state.Store, tel *telemetry.Store, siteMeter string, peakW, } } } - if js, ok := st.LoadConfig(legacyStateKey); ok && js != "" { - var m Model - if err := json.Unmarshal([]byte(js), &m); err == nil && m.Alpha > 0 { - if !loadedProfiles[ProfileHome] { - m.PeakW = peakW // config may have changed - m.MaxPlausibleW = maxPlausibleW - if m.PriorScale <= 0 { - m.PriorScale = 1 - } - s.models[ProfileHome] = &m - slog.Info("loadmodel migrated legacy state", - "profile", ProfileHome, "samples", m.Samples, - "mae_w", m.MAE, "quality", m.Quality()) - } + if js, ok := st.LoadConfig(legacyStateKey); ok && js != "" && !loadedProfiles[ProfileHome] { + // The pre-profile key. It goes through the same restore as every + // other blob, so the feature fingerprint gates it too: this is + // the oldest state on any box, and the likeliest to have been + // fitted against a feature space nobody remembers. + if m := restoreModel(js, peakW, maxPlausibleW, ProfileHome); m != nil { + s.models[ProfileHome] = m + slog.Info("loadmodel migrated legacy state", + "profile", ProfileHome, "samples", m.Samples, + "mae_w", m.MAE, "quality", m.Quality()) } } } @@ -127,10 +136,28 @@ func (s *Service) SetSiteMeter(name string) { s.mu.Unlock() } -func restoreModel(js string, peakW, maxPlausibleW float64, profile Profile) (*Model, bool) { +// restoreModel rebuilds one profile's model from stored state, or returns nil +// when that state cannot be trusted and the caller must cold start. +func restoreModel(js string, peakW, maxPlausibleW float64, profile Profile) *Model { var m Model - if err := json.Unmarshal([]byte(js), &m); err != nil || m.Alpha <= 0 { - return nil, false + res := modelstate.Unwrap(js, FeatureHash(), legacyFeatureHash, &m) + if !res.OK() || m.Alpha <= 0 { + reason := res.Reason + if reason == "" { + // Restored, but with no EMA coefficient it cannot predict. + // Kept as a second net below the hash. + reason = "no EMA coefficient" + } + // Info, not Warn: a cold start is the designed response to state we + // cannot vouch for. Both hashes go in the line so an operator can + // tell "the features changed under me" from "the file is damaged". + // Unlike the PV twin this costs weeks of bucket coverage, which is + // why the fingerprint is deliberately blind to prior retuning — see + // featureProbe. + slog.Info("loadmodel: discarding learned state, cold starting", + "profile", profile, "reason", reason, + "stored_hash", res.StoredHash, "current_hash", FeatureHash()) + return nil } m.PeakW = peakW // config may have changed m.MaxPlausibleW = maxPlausibleW // ditto — fuse size is editable @@ -140,7 +167,7 @@ func restoreModel(js string, peakW, maxPlausibleW float64, profile Profile) (*Mo // Repair any bucket means that were poisoned by the pre-guard bug where // heating-subtracted samples were clamped to 0 and stored in the EMA. m.repairPoisonedBuckets() - return &m, true + return &m } // Model returns a snapshot. @@ -400,7 +427,7 @@ func (s *Service) persist() error { if s.models[profile] == nil { continue } - js, err := json.Marshal(s.models[profile]) + js, err := modelstate.Wrap(FeatureHash(), s.models[profile]) if err != nil { s.mu.RUnlock() return err diff --git a/go/internal/modelstate/modelstate.go b/go/internal/modelstate/modelstate.go new file mode 100644 index 000000000..8cb3d48f2 --- /dev/null +++ b/go/internal/modelstate/modelstate.go @@ -0,0 +1,191 @@ +// Package modelstate versions the learned-model state FTW keeps on disk. +// +// A learned model is a set of coefficients that means nothing on its own. It +// means something only against the feature vector it was fitted to. Restore a +// PV Beta fitted against two time-of-day harmonics into a build that computes +// three, or fitted against horizontal irradiance into one that projects onto +// the array plane, and the model keeps predicting — with a plausible-looking +// MAE — from coefficients that describe a different world. Nothing in the +// numbers gives it away, and the plan is steered by the result. +// +// FTW has paid for this class of fault twice: commit 3255deba, where +// local-time bucket indexing misaligned the learned models across a DST +// change, and 41e59efb, where the models locked themselves out. Both were +// found from their effects, long after the change that caused them. +// +// So each persisted model is wrapped in {schema_version, feature_hash, +// model}, where feature_hash fingerprints the feature space its coefficients +// belong to. On any mismatch the stored state is discarded and the model cold +// starts. Both learned models recover from a cold start in bounded time. +// Neither recovers from silently wrong coefficients. +package modelstate + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "reflect" + "strconv" + "strings" +) + +// Version is the envelope's own schema version. Bump it when the envelope +// layout changes: every stored model then fails to restore and cold starts, +// which is the right outcome for a record this build can no longer read. +const Version = 1 + +type envelope struct { + SchemaVersion int `json:"schema_version"` + FeatureHash string `json:"feature_hash"` + Model json.RawMessage `json:"model"` +} + +// Wrap serialises a model together with the fingerprint of the feature space +// it was fitted against. +func Wrap(featureHash string, model any) (string, error) { + raw, err := json.Marshal(model) + if err != nil { + return "", err + } + js, err := json.Marshal(envelope{ + SchemaVersion: Version, + FeatureHash: featureHash, + Model: raw, + }) + if err != nil { + return "", err + } + return string(js), nil +} + +// Outcome is what Unwrap decided about a stored blob. +type Outcome int + +const ( + // Discard means the state could not be trusted; the caller cold starts + // and the destination is left as the caller supplied it. + Discard Outcome = iota + // Restored means the envelope matched and the destination is populated. + Restored + // RestoredLegacy means an unversioned blob was adopted — see Unwrap. + RestoredLegacy +) + +// Result carries what Unwrap did, with enough detail to log it. +type Result struct { + Outcome Outcome + // StoredHash is the fingerprint found on disk. Empty for an unversioned + // blob, which carries none. + StoredHash string + // Reason says why the state was discarded. Empty when it was restored. + Reason string +} + +// OK reports whether the destination was populated. +func (r Result) OK() bool { return r.Outcome != Discard } + +// Legacy reports whether the state restored was an unversioned blob. +func (r Result) Legacy() bool { return r.Outcome == RestoredLegacy } + +// Unwrap restores stored state into out. +// +// featureHash is the fingerprint of the feature space the running build +// computes. legacyFeatureHash is the fingerprint that was in force when the +// caller adopted the envelope: state written before then carries no +// fingerprint of its own, but we know which feature space produced it, so it +// is still worth restoring — and only while the running build still computes +// that same space. The first change to the features moves featureHash away +// from the caller's frozen legacyFeatureHash, and unversioned state is +// discarded from then on like any other stale fit. Nobody has to remember to +// retire the migration; it retires itself. +// +// Corrupt input is a discard, never a panic, and a discard never writes to +// out — the caller's cold-start value survives intact. This is +// operator-writable state read on the boot path of a control system, so a +// half-decoded model must not be able to reach the planner. +func Unwrap(js, featureHash, legacyFeatureHash string, out any) Result { + trimmed := strings.TrimSpace(js) + if trimmed == "" || trimmed == "null" { + return Result{Reason: "empty"} + } + var env envelope + if err := json.Unmarshal([]byte(js), &env); err != nil { + return Result{Reason: "unreadable: " + err.Error()} + } + if env.SchemaVersion == 0 && len(env.Model) == 0 { + // No envelope fields at all: an unversioned blob written before this + // package existed. json.Unmarshal ignores unknown fields, so a bare + // model lands here with every envelope field at its zero value. + if featureHash != legacyFeatureHash { + return Result{Reason: "unversioned state predates the current feature space"} + } + if err := decodeInto([]byte(js), out); err != nil { + return Result{Reason: "unreadable: " + err.Error()} + } + return Result{Outcome: RestoredLegacy} + } + if env.SchemaVersion != Version { + return Result{ + StoredHash: env.FeatureHash, + Reason: fmt.Sprintf("envelope schema %d, this build reads %d", env.SchemaVersion, Version), + } + } + if env.FeatureHash != featureHash { + return Result{StoredHash: env.FeatureHash, Reason: "fitted against a different feature space"} + } + if len(env.Model) == 0 || string(env.Model) == "null" { + return Result{StoredHash: env.FeatureHash, Reason: "envelope carries no model"} + } + if err := decodeInto(env.Model, out); err != nil { + return Result{StoredHash: env.FeatureHash, Reason: "unreadable: " + err.Error()} + } + return Result{Outcome: Restored, StoredHash: env.FeatureHash} +} + +// decodeInto is json.Unmarshal with all-or-nothing semantics. Plain +// Unmarshal populates the fields it understands before it reaches the one it +// cannot, so a blob truncated or damaged halfway through leaves a model that +// is part restored state and part cold start — a mixture nothing downstream +// can detect. Decode into a scratch value of the same type and copy over only +// on success. +func decodeInto(raw []byte, out any) error { + rv := reflect.ValueOf(out) + if rv.Kind() != reflect.Pointer || rv.IsNil() { + return fmt.Errorf("destination must be a non-nil pointer, got %T", out) + } + scratch := reflect.New(rv.Elem().Type()) + if err := json.Unmarshal(raw, scratch.Interface()); err != nil { + return err + } + rv.Elem().Set(scratch.Elem()) + return nil +} + +// Fingerprint derives a stable identifier for a feature space from a declared +// semantics label and the output of the feature functions over a fixed probe. +// +// The probe is what makes this hard to get wrong. Add a harmonic, reorder a +// slot, change an exponent, and the probe values move, so the fingerprint +// moves, with nobody having to remember to bump a constant — which is exactly +// the step that gets forgotten. The semantics label covers the one thing a +// probe cannot see: a caller feeding a differently defined quantity into an +// unchanged feature function. +func Fingerprint(semantics string, probe []float64) string { + h := sha256.New() + h.Write([]byte(semantics)) + for _, v := range probe { + // Separator, so that concatenated digits cannot make two different + // probes hash alike. + h.Write([]byte{0}) + // Twelve significant digits: far finer than any real change to a + // feature produces, far coarser than the sub-ULP spread the same + // expression can have between the amd64 host that built the release + // and the arm64 Pi that runs the plant. A state database stays + // portable between them. + h.Write([]byte(strconv.FormatFloat(v, 'g', 12, 64))) + } + // 64 bits is ample for detecting a change and short enough to read in a + // log line next to the hash it failed to match. + return hex.EncodeToString(h.Sum(nil))[:16] +} diff --git a/go/internal/modelstate/modelstate_test.go b/go/internal/modelstate/modelstate_test.go new file mode 100644 index 000000000..4fc2e6d23 --- /dev/null +++ b/go/internal/modelstate/modelstate_test.go @@ -0,0 +1,204 @@ +package modelstate + +import ( + "encoding/json" + "strings" + "testing" +) + +type toyModel struct { + Beta []float64 `json:"beta"` + Alpha float64 `json:"alpha"` +} + +const ( + hashA = "aaaaaaaaaaaaaaaa" + hashB = "bbbbbbbbbbbbbbbb" +) + +func TestWrapUnwrapRoundTrip(t *testing.T) { + want := toyModel{Beta: []float64{1, 2.5, -3}, Alpha: 0.1} + js, err := Wrap(hashA, want) + if err != nil { + t.Fatalf("wrap: %v", err) + } + + var got toyModel + res := Unwrap(js, hashA, hashB, &got) + if res.Outcome != Restored { + t.Fatalf("outcome = %v (%s), want Restored", res.Outcome, res.Reason) + } + if res.StoredHash != hashA { + t.Errorf("stored hash = %q, want %q", res.StoredHash, hashA) + } + if len(got.Beta) != 3 || got.Beta[1] != 2.5 || got.Alpha != 0.1 { + t.Errorf("round trip lost the model: %+v", got) + } +} + +// A model fitted against one feature space must never be restored into a +// build that computes another. The whole point of the envelope: the +// coefficients still parse, still look sane, and are still wrong. +func TestUnwrapDiscardsOnFeatureHashMismatch(t *testing.T) { + js, err := Wrap(hashA, toyModel{Beta: []float64{9, 9, 9}, Alpha: 0.1}) + if err != nil { + t.Fatalf("wrap: %v", err) + } + + got := toyModel{Beta: []float64{1}} // caller's cold-start value + res := Unwrap(js, hashB, hashB, &got) + + if res.OK() { + t.Fatalf("outcome = %v, want Discard", res.Outcome) + } + if res.StoredHash != hashA { + t.Errorf("stored hash = %q, want %q so the log line can name both", res.StoredHash, hashA) + } + if res.Reason == "" { + t.Error("a discard must carry a reason; it is the only thing the operator sees") + } + if len(got.Beta) != 1 || got.Beta[0] != 1 { + t.Errorf("destination was written on a discard: %+v", got) + } +} + +// The envelope's own layout can change. When it does, everything written by +// the previous layout must cold-start rather than be read as this one. +func TestUnwrapDiscardsUnknownSchemaVersion(t *testing.T) { + js := `{"schema_version":999,"feature_hash":"` + hashA + `","model":{"alpha":0.1}}` + + var got toyModel + res := Unwrap(js, hashA, hashA, &got) + + if res.OK() { + t.Fatalf("outcome = %v, want Discard", res.Outcome) + } + if !strings.Contains(res.Reason, "999") { + t.Errorf("reason %q should name the schema it found", res.Reason) + } +} + +// Boot path of a control system: damaged state cold-starts, it does not +// panic and it does not half-populate the model. +func TestUnwrapCorruptBlobColdStarts(t *testing.T) { + cases := map[string]string{ + "truncated": `{"schema_version":1,"feature_hash":"aaaa`, + "not json": "this is not json at all", + "empty": "", + "whitespace": " \n\t ", + "wrong shape": `[1,2,3]`, + "model garbage": `{"schema_version":1,"feature_hash":"` + hashA + `","model":"not-an-object"}`, + "model null": `{"schema_version":1,"feature_hash":"` + hashA + `","model":null}`, + "nul bytes": "\x00\x00\x00", + "legacy garbage": `{"beta":"should-be-numbers","alpha":0.1}`, + } + for name, js := range cases { + t.Run(name, func(t *testing.T) { + var got toyModel + res := Unwrap(js, hashA, hashA, &got) + if res.OK() { + t.Fatalf("outcome = %v, want Discard (reason %q)", res.Outcome, res.Reason) + } + if got.Alpha != 0 || got.Beta != nil { + t.Errorf("destination written on a discard: %+v", got) + } + }) + } +} + +// An unversioned blob is state written before this package existed. It is +// adopted only while the running build still computes the feature space it +// was fitted against. +func TestUnwrapAdoptsUnversionedBlobUnderMatchingLegacyHash(t *testing.T) { + bare, err := json.Marshal(toyModel{Beta: []float64{4, 5}, Alpha: 0.2}) + if err != nil { + t.Fatal(err) + } + + var got toyModel + res := Unwrap(string(bare), hashA, hashA, &got) + + if res.Outcome != RestoredLegacy { + t.Fatalf("outcome = %v (%s), want RestoredLegacy", res.Outcome, res.Reason) + } + if !res.Legacy() { + t.Error("Legacy() must report an adopted unversioned blob so the log can say so") + } + if res.StoredHash != "" { + t.Errorf("stored hash = %q, want empty: an unversioned blob carries none", res.StoredHash) + } + if got.Alpha != 0.2 { + t.Errorf("unversioned blob not restored: %+v", got) + } +} + +// The migration retires itself. Once the features move, the caller's frozen +// legacyFeatureHash no longer matches, and unversioned state is discarded +// like any other stale fit — with nobody having to remember to delete this +// path. +func TestUnwrapDiscardsUnversionedBlobOnceFeaturesMove(t *testing.T) { + bare, err := json.Marshal(toyModel{Beta: []float64{4, 5}, Alpha: 0.2}) + if err != nil { + t.Fatal(err) + } + + var got toyModel + res := Unwrap(string(bare), hashB, hashA, &got) // build moved on from hashA + + if res.OK() { + t.Fatalf("outcome = %v, want Discard", res.Outcome) + } + if got.Alpha != 0 { + t.Errorf("destination written on a discard: %+v", got) + } +} + +func TestFingerprintIsStableAndSensitive(t *testing.T) { + base := []float64{0, 1.5, -2.25, 1e6} + + if a, b := Fingerprint("s", base), Fingerprint("s", base); a != b { + t.Fatalf("fingerprint not stable: %q vs %q", a, b) + } + + // The declared semantics is part of the identity: same numbers, different + // meaning, different fingerprint. + if Fingerprint("s", base) == Fingerprint("s2", base) { + t.Error("semantics label must change the fingerprint") + } + + // A change to the feature math moves the probe values. + if Fingerprint("s", base) == Fingerprint("s", []float64{0, 1.5, -2.25, 1e6 + 1}) { + t.Error("changed probe value must change the fingerprint") + } + + // Adding a feature lengthens the probe. + if Fingerprint("s", base) == Fingerprint("s", append(append([]float64{}, base...), 0)) { + t.Error("added probe value must change the fingerprint") + } + + // Reordering a slot is exactly the change that stays invisible in a + // per-value check but invalidates every coefficient. + if Fingerprint("s", base) == Fingerprint("s", []float64{1.5, 0, -2.25, 1e6}) { + t.Error("reordered probe must change the fingerprint") + } + + // The separator is what makes reordering visible even when neighbouring + // values would otherwise concatenate to the same digits. + if Fingerprint("s", []float64{1, 23}) == Fingerprint("s", []float64{12, 3}) { + t.Error("probe values must not run together") + } +} + +// Rounding to twelve significant digits absorbs the sub-ULP difference the +// same expression can produce on a different CPU, so a state database stays +// portable between the build host and the Pi. +func TestFingerprintIgnoresSubULPDifference(t *testing.T) { + v := 0.8660254037844386 + nudged := v + 1e-16 // ~1 ULP at this magnitude + if v == nudged { + t.Skip("nudge below float64 resolution on this platform") + } + if Fingerprint("s", []float64{v}) != Fingerprint("s", []float64{nudged}) { + t.Error("a one-ULP difference must not invalidate learned state") + } +} diff --git a/go/internal/pvmodel/model.go b/go/internal/pvmodel/model.go index 238c82d49..92e310e5c 100644 --- a/go/internal/pvmodel/model.go +++ b/go/internal/pvmodel/model.go @@ -34,7 +34,10 @@ package pvmodel import ( "math" + "sync" "time" + + "github.com/srcfl/ftw/go/internal/modelstate" ) // NFeat is the number of features in the RLS regression. @@ -107,6 +110,56 @@ func Features(clearSkyW, cloudPct float64, t time.Time) [NFeat]float64 { } } +// featureSemantics declares what the arguments to Features mean. It is the +// half of the fingerprint a probe cannot derive: pass clear-sky irradiance +// projected onto the array plane instead of the horizontal, or low-cloud +// cover instead of total, and Features returns exactly the same numbers for +// the same arguments while Beta is now fitted against a different physical +// quantity. +// +// CHANGE THIS STRING in the commit that changes what a caller passes in. +// Changes to the feature math itself need no edit here — featureProbe moves +// the fingerprint on its own. +const featureSemantics = "pvmodel/1 clearsky=horizontal_ghi_wm2 cloud=total_cover_pct hour=utc target=ac_w" + +// featureProbe evaluates Features across a fixed grid of inputs. Its output +// is the automatic half of the fingerprint: add a harmonic, revive the dead +// slot, reorder a term or change the cloud exponent, and these numbers move. +// +// The grid spans night and full sun, clear and overcast, and four times of +// day chosen so both harmonics take distinct values — a probe that only +// looked at noon would miss a change to the phase of either one. +func featureProbe() []float64 { + times := []time.Time{ + time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2024, 6, 21, 5, 30, 0, 0, time.UTC), + time.Date(2024, 6, 21, 12, 0, 0, 0, time.UTC), + time.Date(2024, 12, 21, 17, 45, 0, 0, time.UTC), + } + clearSky := []float64{0, 137.5, 900} + cloud := []float64{0, 42.5, 100} + out := make([]float64, 0, len(times)*len(clearSky)*len(cloud)*NFeat) + for _, t := range times { + for _, cs := range clearSky { + for _, cc := range cloud { + x := Features(cs, cc, t) + out = append(out, x[:]...) + } + } + } + return out +} + +var featureHash = sync.OnceValue(func() string { + return modelstate.Fingerprint(featureSemantics, featureProbe()) +}) + +// FeatureHash fingerprints the feature space Beta is fitted against. Stored +// coefficients are only restored when their recorded hash matches this one; +// see internal/modelstate for why, and service.go for what happens when it +// does not. +func FeatureHash() string { return featureHash() } + // Predict returns the expected AC output in W (non-negative). Cold-start // behavior: during the first WarmupSamples we blend the learned β with // the naive physics prior so a wild β coefficient (which RLS can take a diff --git a/go/internal/pvmodel/persistence_test.go b/go/internal/pvmodel/persistence_test.go new file mode 100644 index 000000000..5887fc4e3 --- /dev/null +++ b/go/internal/pvmodel/persistence_test.go @@ -0,0 +1,227 @@ +package pvmodel + +import ( + "encoding/json" + "testing" + "time" + + "github.com/srcfl/ftw/go/internal/modelstate" +) + +// storedModel reads a persisted blob the way the service does. Tests assert +// against the model inside the envelope, not the envelope itself. +func storedModel(t *testing.T, js string) Model { + t.Helper() + var m Model + res := modelstate.Unwrap(js, FeatureHash(), legacyFeatureHash, &m) + if !res.OK() { + t.Fatalf("stored state did not restore: %s", res.Reason) + } + return m +} + +// requireLegacyAdoption skips a case that depends on pre-envelope state still +// being adopted. Once the features move, that state is correctly discarded and +// the case stops existing — the migration and the tests that cover it retire +// together, so a feature change never leaves a test demanding the old +// behaviour back. +func requireLegacyAdoption(t *testing.T) { + t.Helper() + if FeatureHash() != legacyFeatureHash { + t.Skip("features have moved on; unversioned state is discarded, as designed") + } +} + +// TestFeatureHashPinned is a tripwire, not a rule. Changing the feature +// vector is allowed and sometimes necessary — the roofmodel work changes what +// clearSkyW means — but it makes every deployed PV twin throw away its +// coefficients and relearn. That should be a decision, not a surprise found +// in production. +// +// If this test fails: confirm the feature change is intended, update the +// literal below, and say in the changeset that the PV model cold-starts on +// upgrade. Do NOT touch legacyFeatureHash in service.go — that constant is +// frozen on purpose, and moving it would restore pre-envelope coefficients +// under the new features, which is the exact fault this guards against. +func TestFeatureHashPinned(t *testing.T) { + const want = "fd42eb2a7c1e9f55" + if got := FeatureHash(); got != want { + t.Errorf("PV feature hash = %q, pinned at %q\n"+ + "the feature vector changed: every deployed twin will cold-start", got, want) + } +} + +// The hash must move on its own when the features do. This is the property +// that makes the guard hard to forget: nobody has to remember to bump a +// constant when they add a harmonic. +func TestFeatureHashTracksTheFeatureMath(t *testing.T) { + base := modelstate.Fingerprint(featureSemantics, featureProbe()) + + // Reviving the dead intercept slot — the #134 regression. + revived := featureProbe() + revived[0] = 1.0 + if modelstate.Fingerprint(featureSemantics, revived) == base { + t.Error("reviving the intercept slot must move the hash") + } + + // A third time-of-day harmonic. + extended := append(append([]float64{}, featureProbe()...), 0.5) + if modelstate.Fingerprint(featureSemantics, extended) == base { + t.Error("adding a feature must move the hash") + } + + // The semantics half: same math, different meaning for clearSkyW. This is + // what the roofmodel family changes, and no probe can see it. + if modelstate.Fingerprint("pvmodel/1 clearsky=plane_of_array_wm2", featureProbe()) == base { + t.Error("redeclaring what an input means must move the hash") + } +} + +func TestPersistedStateRoundTrips(t *testing.T) { + db := openTestDB(t) + cs := func(time.Time) float64 { return 500 } + cl := func(time.Time) (float64, bool) { return 20, true } + + svc := NewService(db, nil, cs, cl, 5000) + svc.mu.Lock() + svc.model.Samples = 137 + svc.model.MAE = 42 + svc.model.Beta[3] = 1.25 + svc.mu.Unlock() + svc.persist() + + restored := NewService(db, nil, cs, cl, 5000).Model() + if restored.Samples != 137 || restored.MAE != 42 || restored.Beta[3] != 1.25 { + t.Fatalf("matching feature hash must restore the model, got %+v", restored) + } +} + +// The bug this PR exists for: coefficients fitted against one feature space +// must not be restored into a build that computes another. They parse, they +// look sane, and they steer the plan from a different world. +func TestStateFittedAgainstOtherFeaturesColdStarts(t *testing.T) { + db := openTestDB(t) + cs := func(time.Time) float64 { return 500 } + cl := func(time.Time) (float64, bool) { return 20, true } + + trained := NewModel(5000) + trained.Samples = 900 + trained.MAE = 7 + trained.Beta[3] = 99 + js, err := modelstate.Wrap("0000badfeature00", trained) + if err != nil { + t.Fatal(err) + } + if err := db.SaveConfig(stateKey, js); err != nil { + t.Fatal(err) + } + + got := NewService(db, nil, cs, cl, 5000).Model() + + if got.Samples != 0 { + t.Errorf("samples = %d, want 0: stale coefficients must not survive a feature change", got.Samples) + } + if got.Beta[3] != 0 { + t.Errorf("Beta[3] = %v, want 0", got.Beta[3]) + } + if got.Beta[2] != 5000.0/1000 { + t.Errorf("Beta[2] = %v, want the cold-start physics prior", got.Beta[2]) + } +} + +// State written before the envelope existed is adopted, because the feature +// space it was fitted against is still the one this build computes. Upgrading +// must not cost every site its learned twin. +func TestUnversionedStateIsAdopted(t *testing.T) { + requireLegacyAdoption(t) + db := openTestDB(t) + cs := func(time.Time) float64 { return 500 } + cl := func(time.Time) (float64, bool) { return 20, true } + + trained := NewModel(5000) + trained.Samples = 480 + trained.MAE = 31 + trained.Beta[3] = 0.75 + bare, err := json.Marshal(trained) // pre-envelope on-disk shape + if err != nil { + t.Fatal(err) + } + if err := db.SaveConfig(stateKey, string(bare)); err != nil { + t.Fatal(err) + } + + got := NewService(db, nil, cs, cl, 5000).Model() + + if got.Samples != 480 || got.Beta[3] != 0.75 { + t.Fatalf("unversioned state should be adopted, got %+v", got) + } +} + +// The pre-#134 migration: a drifted intercept is zeroed on load. It survives +// the move to the envelope, on both the versioned and the unversioned path. +func TestBetaZeroMigrationSurvivesEnvelope(t *testing.T) { + cs := func(time.Time) float64 { return 500 } + cl := func(time.Time) (float64, bool) { return 20, true } + + drifted := NewModel(5000) + drifted.Samples = 300 + drifted.Beta[0] = 812 // intercept that drifted before #134 + + versioned, err := modelstate.Wrap(FeatureHash(), drifted) + if err != nil { + t.Fatal(err) + } + bare, err := json.Marshal(drifted) + if err != nil { + t.Fatal(err) + } + + for name, js := range map[string]string{"versioned": versioned, "unversioned": string(bare)} { + t.Run(name, func(t *testing.T) { + if name == "unversioned" { + requireLegacyAdoption(t) + } + db := openTestDB(t) + if err := db.SaveConfig(stateKey, js); err != nil { + t.Fatal(err) + } + got := NewService(db, nil, cs, cl, 5000).Model() + if got.Samples != 300 { + t.Fatalf("model should have restored, got %+v", got) + } + if got.Beta[0] != 0 { + t.Errorf("Beta[0] = %v, want 0: the #134 migration must still run", got.Beta[0]) + } + }) + } +} + +// A damaged blob on the boot path of a control system cold-starts. It does +// not panic, and it does not leave a half-decoded model behind. +func TestCorruptStateColdStarts(t *testing.T) { + cs := func(time.Time) float64 { return 500 } + cl := func(time.Time) (float64, bool) { return 20, true } + + for name, js := range map[string]string{ + "truncated envelope": `{"schema_version":1,"feature_hash":"fd42eb2a7c1e9f55","model":{"samples":`, + "not json": "\x00\x01\x02 not json", + "wrong shape": `[1,2,3]`, + "no model": `{"schema_version":1,"feature_hash":"fd42eb2a7c1e9f55"}`, + "future schema": `{"schema_version":99,"feature_hash":"fd42eb2a7c1e9f55","model":{"samples":900,"forgetting":0.995}}`, + "half-decoded": `{"beta":"not-an-array","samples":900,"forgetting":0.995}`, + } { + t.Run(name, func(t *testing.T) { + db := openTestDB(t) + if err := db.SaveConfig(stateKey, js); err != nil { + t.Fatal(err) + } + got := NewService(db, nil, cs, cl, 5000).Model() + if got.Samples != 0 { + t.Errorf("samples = %d, want 0 (cold start)", got.Samples) + } + if got.Forgetting != NewModel(5000).Forgetting { + t.Errorf("forgetting = %v, want the cold-start value", got.Forgetting) + } + }) + } +} diff --git a/go/internal/pvmodel/service.go b/go/internal/pvmodel/service.go index ad9bf4272..a3fe8b30b 100644 --- a/go/internal/pvmodel/service.go +++ b/go/internal/pvmodel/service.go @@ -2,11 +2,11 @@ package pvmodel import ( "context" - "encoding/json" "log/slog" "sync" "time" + "github.com/srcfl/ftw/go/internal/modelstate" "github.com/srcfl/ftw/go/internal/state" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -18,6 +18,18 @@ import ( // UTC-based Features(). Fresh init + ~50 samples retrains. const stateKey = "pvmodel/state_utc" +// legacyFeatureHash is the fingerprint of the feature space in force when the +// envelope was introduced. State written before then carries no fingerprint, +// but it was fitted against exactly this space, so it is still worth +// restoring — and only while the running build still computes that space. +// +// This constant is frozen. The first change to Features moves FeatureHash() +// away from it, and unversioned state is discarded from then on, which is the +// whole point. Never update it to match a new FeatureHash(): that would +// re-arm the migration under features the old coefficients were never fitted +// against, restoring exactly the silent-wrong-model fault this guards. +const legacyFeatureHash = "fd42eb2a7c1e9f55" + // ClearSkyFunc is injected by main.go to decouple pvmodel from the // forecast package. Returns clear-sky GHI (W/m²) for the site's lat/lon // baked into the closure. @@ -71,7 +83,24 @@ func NewService(st *state.Store, tel *telemetry.Store, cs ClearSkyFunc, cf Cloud if st != nil { if js, ok := st.LoadConfig(stateKey); ok && js != "" { var m Model - if err := json.Unmarshal([]byte(js), &m); err == nil && m.Forgetting > 0 { + res := modelstate.Unwrap(js, FeatureHash(), legacyFeatureHash, &m) + reason := res.Reason + if res.OK() && m.Forgetting <= 0 { + // No forgetting factor means no usable RLS state, whatever + // the envelope says. Kept as a second net below the hash. + reason = "no forgetting factor" + } + if reason != "" { + // Info, not Warn: a cold start is the designed response to + // state we cannot vouch for, and ~50 daylight samples + // rebuild it. Both hashes go in the line so an operator can + // tell "the features changed under me" from "the file is + // damaged" without a debugger. + slog.Info("pvmodel: discarding learned state, cold starting", + "reason", reason, + "stored_hash", res.StoredHash, + "current_hash", FeatureHash()) + } else { m.RatedW = ratedW // config may have changed rated value // Migrate pre-#134 persisted models: Beta[0] was a free // intercept that drifted during training and leaked into @@ -81,7 +110,9 @@ func NewService(st *state.Store, tel *telemetry.Store, cs ClearSkyFunc, cf Cloud // self-heal kicks in. m.Beta[0] = 0 s.model = &m - slog.Info("pvmodel restored", "samples", m.Samples, "mae_w", m.MAE, "quality", m.Quality()) + slog.Info("pvmodel restored", + "samples", m.Samples, "mae_w", m.MAE, "quality", m.Quality(), + "unversioned", res.Legacy()) } } } @@ -399,7 +430,7 @@ func (s *Service) persist() { s.persistMu.Lock() defer s.persistMu.Unlock() s.mu.RLock() - js, err := json.Marshal(s.model) + js, err := modelstate.Wrap(FeatureHash(), s.model) s.mu.RUnlock() if err != nil { return diff --git a/go/internal/pvmodel/service_test.go b/go/internal/pvmodel/service_test.go index a3c71fba1..2144c30c9 100644 --- a/go/internal/pvmodel/service_test.go +++ b/go/internal/pvmodel/service_test.go @@ -1,7 +1,6 @@ package pvmodel import ( - "encoding/json" "math" "path/filepath" "testing" @@ -36,10 +35,7 @@ func TestResetPersistsSurvivesRestart(t *testing.T) { if !ok || js == "" { t.Fatal("trained model not persisted") } - var trained Model - if err := json.Unmarshal([]byte(js), &trained); err != nil { - t.Fatal(err) - } + trained := storedModel(t, js) if trained.Samples != 200 { t.Fatalf("expected 200 samples in stored model, got %d", trained.Samples) } @@ -52,10 +48,7 @@ func TestResetPersistsSurvivesRestart(t *testing.T) { if !ok || js2 == "" { t.Fatal("reset model not persisted") } - var reset Model - if err := json.Unmarshal([]byte(js2), &reset); err != nil { - t.Fatal(err) - } + reset := storedModel(t, js2) if reset.Samples != 0 { t.Fatalf("expected 0 samples after reset, got %d", reset.Samples) }