Docs: canonical ML/AI chapter (ml-models.md) - #9
Conversation
Consolidate the PV twin, load twin, price forecaster, battery dynamics twin, and MPC planner into one chapter with verified code references. Turn ml-twins.md into a single-line redirect so older links still land. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c35d6edf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Outer PI computes the desired actual grid power. When confidence is | ||
| high, we invert through the model so the command we send produces the | ||
| desired actual on the next cycle (`UseCascade` toggle in | ||
| `go/internal/control/dispatch.go:84`). When the model is unhealthy |
There was a problem hiding this comment.
This section says the controller inverts PI targets through battery.Model.Inverse when confidence is high, but that path is not implemented in the current runtime: the control loop sends DispatchTarget.TargetW directly to drivers, and UseCascade is defined but not consumed. Documenting this as active behavior can mislead operators during tuning/debugging because they will assume battery-model compensation is affecting commands when it is not.
Useful? React with 👍 / 👎.
|
🎉 This PR is included in version 2.0.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Three items in the 11-comment review are still outstanding after the earlier rebase — the rest were already addressed in b6a5c78 / 4f3ad01: - #3 EV card keyboard accessibility (web/index.html + web/app.js): add role="button", tabindex="0", aria-label, and Enter/Space key handler so the summary card can be activated without a mouse and is announced correctly by assistive tech. - #9 Hardcoded "easee" provider (web/settings.js): Connect button now derives the provider name from the driver's lua path (strip dirs, trailing "_cloud", ".lua"), falling back to "easee" when the driver config is missing or oddly named. Lets a second cloud driver slot in without touching the button. - #5 Missing tests for path normalization (config_test.go): cover stripLeadingDotDot, ResolveDriverPaths (relative / absolute-in-tree / absolute-out-of-tree / leading-"../"), and the full Resolve → Unresolve → Resolve round-trip. The round-trip test is the one that pins down Copilot #11 — absolute paths outside baseDir must stay absolute so the next Resolve doesn't strip a "../" from filepath.Rel and silently re-anchor the driver under baseDir. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
docs/ml-models.md: the single canonical ML/AI chapter covering PV twin, load twin, price forecaster, battery dynamics twin (ARX(1) + cascade + self-tune), and the MPC planner.docs/ml-twins.mdcollapsed to a single-line redirect so old links resolve.docs/mpc-planner.md,docs/battery-models.md,docs/api.md,docs/safety.md) noted; those chapters are landing in parallel PRs.Test plan
cd go && go build ./...clean.cd go && go test -timeout 120s -count=1 ./...all green (including mpc, pvmodel, loadmodel, priceforecast, battery, selftune, sunpos).Generated with Claude Code