ALLSHIFT/docs/04-simulations/rule-based-controller-implementation.md
pepe 72dd781dbc Organize documentation into docs/ and superseded/
Audit every document in the repository, convert the non-markdown ones into
markdown reports, and split current documentation from outdated material.

docs/ — 31 markdown documents in seven numbered sections. Twenty are new
reports generated from .docx / .pdf / .xlsx / .mlx / .m sources that were
previously unreadable in the browser and undiffable in git. Each report
carries a provenance block (source path, format, MD5) and links back to its
original; all 13 recorded checksums verify against the files on disk.
Machine-extraction losses (PDF table column interleaving, Word OMML
equations, embedded figures) are called out explicitly rather than silently
smoothed over.

superseded/ — outdated material with a documented reason per entry:
two byte-identical ClickUp re-exports, an older revision of the BIDMC/UCSD
energy-flow doc (the retained copy adds the SoC Violation Rate KPI), a
duplicate of Shift input data.docx, the May 2026 simulation plan, the
root PV+Battery.md now covered by a fuller report, GitHub's stock
demo-repository template, and a zero-byte placeholder. Its README also
records what was deliberately NOT retired and why — the "Old Frameworks"
and "Old Simulations" folders hold unique Simulink revisions, and
"Big Ugly Folder" holds the only copy of framework revision 1.3.

Findings worth flagging, all documented in the reports:
- Simulink lineage recovered from each .slx's internal coreProperties.xml
  revision counter. The current model is
  Current Framework/Bobert0206_Initial_Simulation_Framework.slx (rev 2.7);
  the top-level copy is rev 1.3, five revisions behind.
- Simulations/Constants.m is a truncated byte-prefix of the Current
  Framework copy, silently missing H2_leak, H2_cap and E_H2_vol_h.
- The PEM electrolyser and fuel cell are unmodified MathWorks Simscape
  examples still at vendor defaults; the "10x bigger" sizing TODO recorded
  in Constants.m was never carried out.
- controller-claude.m does not compile — undefined P_Electro_max, outputs
  unassigned on several paths.
- The specification set uses two incompatible variable naming conventions
  and disagrees on action-space size (5 vs 16).
- MA_hourly_load.csv (13.7 MB) is the same 35,040 rows as 89993-0.parquet
  (2.4 MB).
- Clinical data is the MIMIC-IV *demo* (ODbL, 100 patients), not full
  MIMIC-IV — redistributable, but the licence and citation are unrecorded.

Housekeeping: untrack 21 Simulink build artefacts (slprj/, *.slxc) and add
ignore rules for them. Root README rewritten around the new layout.

Recruitment notes naming individual candidates are excluded from version
control via .gitignore rather than committed; the generic question template
is kept in docs/07-team-and-operations/.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-25 21:20:33 -07:00

213 lines
9.5 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Rule-Based Controller — MATLAB Implementation
> **Markdown report of a non-markdown source file.**
>
> | | |
> |---|---|
> | **Source** | `Shift Matlab Drive/Shift Matlab Drive/controller-claude.m` |
> | **Format** | MATLAB, 92 lines, 7,388 B |
> | **MD5** | `745f573a203238a32cbcd5b1a3d6ecc7` |
> | **Status** | **Draft — does not run.** See [Blocking defects](#blocking-defects) |
> | **Report generated** | 2026-07-25 |
## What it is
The MATLAB implementation of the priority-cascade dispatch policy specified in
[Rule-Based Controller](../02-specifications/rule-based-controller.md). Written as the body of
a Simulink **MATLAB Function block** — the signature is `function [...] = fcn(...)`, and `fcn`
is Simulink's default block function name.
```matlab
function [P_load_P, P_load_PB, P_battery_cont, P_load_PBH, P_FuelCell_cont, P_Electro_cont] = ...
fcn(Current_A, P_load, P_PV, E_battery_SOC, P_battery_max, E_battery_max, ...
E_H2_SOC, P_FuelCell_max, E_H2_max)
```
It resolves load in a fixed merit order, exactly as the specification describes:
| Stage | Lines | Behaviour |
|---|---|---|
| 1. PV direct | 8 | `P_load_P = P_load - P_PV` — residual after solar |
| 2. Battery | 1136 | Residual > 0 → discharge (capped by `P_battery_max` or remaining SOC); residual < 0 charge (capped by max power or remaining headroom) |
| 3. Hydrogen | 3964 | Same two-sided pattern fuel cell discharges the tank, electrolyser charges it |
| 4. Grid | 6772 | Whatever remains is bought or sold |
> [!WARNING]
> **This file does not run.** It has hard undefined-variable errors and cannot compile as a
> Simulink MATLAB Function block. It is an explicitly-marked work in progress — the author's
> own notes at lines 7479 say as much. The defect list below is documentation of a known
> draft, not a code review of something anyone claimed was finished.
## Blocking defects
**B1 — undefined `Electro_max` (line 54).**
```matlab
P_Electro_cont = -Electro_max % The ELECTROLYZER charges at it's maximum power allowed
```
`Electro_max` exists nowhere. Intended `P_Electro_max` missing the `P_` prefix.
**B2 — `P_Electro_max` is never defined either (lines 52, 53, 54).**
The signature declares nine inputs; `P_Electro_max` is not among them and is never assigned in
the body. Even after fixing B1, the reference is still undefined. **The entire electrolyser
branch (lines 5163) is dead.** The function signature needs a tenth input.
**B3 — outputs not assigned on all paths.**
A Simulink MATLAB Function block requires every declared output to be assigned on every path.
| Output | Unassigned when |
|---|---|
| `P_FuelCell_cont` / `P_Electro_cont` | Mutually exclusive the `if` at line 39 assigns only the first, the `else` at 51 only the second. Both are declared outputs |
| `P_load_PB` | Never assigned on the path through lines 2021 (battery too empty to discharge at max) |
| `P_load_PBH` | Never assigned on the path through lines 4849 (tank too empty) |
Consequently line 39 can read an unassigned `P_load_PB`, and line 67 an unassigned
`P_load_PBH`.
## Logic defects — silently wrong results
**B4 — wrong right-hand side (line 43).**
```matlab
P_FuelCell_cont = P_load_PBH;
```
The battery analogue it was copied from (line 15) is `P_battery_cont = P_load_P;` the
*incoming* residual. Here `P_load_PBH` is, under the guard at line 42 (`P_load_PBH < 0`), the
**negative overshoot**. It should read `P_FuelCell_cont = P_load_PB;`. As written the fuel cell
is commanded to a negative power to consume exactly when it is supposed to supply.
**B5 — sign convention inverted (lines 33 and 61).**
The stated convention (line 29) is *"negative `P_battery_cont` is battery charging, positive is
discharging"*. Line 26 respects it (`-P_battery_max`). But in the same charging branch:
```matlab
33: P_battery_cont = E_battery_max - E_battery_SOC; % headroom — POSITIVE
61: P_Electro_cont = E_H2_max - E_H2_SOC; % headroom — POSITIVE
```
Both assign **positive** values while on the charging path. Storage will be commanded to
discharge when it should charge.
**B6 — energy and power conflated throughout.**
Energy variables (`E_*`, kWh) are compared against and assigned to power variables (`P_*`, kW)
with no timestep factor:
| Line | Expression | Problem |
|---|---|---|
| 12 | `if E_battery_SOC > P_battery_max` | kWh compared to kW |
| 21 | `P_battery_cont = E_battery_SOC;` | kWh assigned to a kW command |
| 40 | `if E_H2_SOC > P_FuelCell_max` | kWh compared to kW |
| 49 | `P_FuelCell_cont = E_H2_SOC;` | kWh assigned to a kW command |
| 24, 33, 52, 61 | headroom expressions | same |
The author flags this at lines 7879:
> `%IMPLEMENT THE ARRAY-SCANNING FOR-LOOP AND MAKE SURE THE Power = Energy`
> `%Equations make sense????`
The arithmetic is only correct if the timestep is exactly 1 h *and* units are consistent
neither is enforced anywhere. This is the same `dt` question the
[Simulator I/O Interface](../02-specifications/simulator-io-interface.md#timestep) leaves open.
**B7 — load sign clashes with `Constants.m`.**
```matlab
if P_load < 0
P_load = 0;
end
```
`Constants.m` defines `E_HospitalLoad = -1000000; %kWh` hospital load is stored **negative**.
If that convention feeds `P_load`, this guard zeroes out the entire hospital demand and the
controller does nothing.
## Console output
**B8/B9 — two assignments missing a terminating semicolon**: line 26
(`P_battery_cont = -P_battery_max`) and line 54 (`P_Electro_cont = -Electro_max`). Both echo to
the console on every hit, every timestep. An automated scan found exactly these two.
**B10 — unconditional `disp` every timestep** (lines 6772):
```matlab
if P_load_PBH > 0
disp('buy')
else
disp('sell')
end
disp(P_load_PBH)
```
Beyond the spam, `disp` is an extrinsic function in a Simulink MATLAB Function block
unsupported for code generation. It will either force interpreted execution or fail the build.
## Incomplete sections
**I1 — grid exchange is never returned.** Lines 6772 determine buy vs. sell but only *print*
it. There is no output argument for grid power. The author's note at lines 7476:
> `%Doublecheck these and repeat the logic for the hydrogen, finally`
> `%selling/buying the rest from the grid. Make sure the discharging is at the`
> `%tank level and charging at hydrogen level`
**I2 — the time-series loop was never implemented** (lines 7879, quoted above).
**I3 — dead commented-out hydrogen integrator (lines 8389):**
```matlab
%for t = 1:24 %Check if this makes sense
% H2_stor(end) = H2_stor(end)*(1-H2_leak);
% H2_stor(end+1) = H2_stor(end)+H2_vol(t);
% H2_Flow_L_min(t) = (-P_H2(t)/E_H2_vol_h(t))*(1000/60);
%end
%E_H2_stor = H2_stor*E_H2_vol_h;
```
Uncommenting it would still fail. `H2_stor`, `H2_vol` and `P_H2` are undefined anywhere in the
repository. `H2_leak` and `E_H2_vol_h` do exist but **only in the complete `Constants.m`**,
not the stale top-level copy (see
[Simulink Model Inventory](simulink-model-inventory.md#constantsm--the-top-level-copy-is-stale)).
And line 86 indexes `E_H2_vol_h(t)` as an array when it is the scalar `3000` out of bounds
for any `t > 1`.
## Other observations
| # | Observation |
|---|---|
| Q1 | Input `Current_A` is declared but **never used** in the 92-line body a dead parameter |
| Q2 | **The filename is not a legal MATLAB function file.** `controller-claude.m` contains a hyphen; MATLAB identifiers cannot. It can never be called as `controller-claude(...)`. It only works pasted into a Simulink MATLAB Function block consistent with the function being named `fcn` |
| Q3 | It is the **only file at the `Shift Matlab Drive/Shift Matlab Drive/` root**, outside `Simulations/` entirely, and is not attached to any model in the repository |
| Q4 | The hydrogen block is a verbatim copy of the battery block with incomplete renaming. Lines 4546 sit in the *fuel cell* branch but say "max **battery** power"; line 57 references `P_battery_cont` inside the *electrolyser* branch; line 61's comment says "**Battery** charges only as much as there is space left" while assigning `P_Electro_cont`. **This is how B4 and B5 arose** |
| Q5 | Comment typos: `dirrectly` (7), `to empty` for "too empty" (20, 48), `ELETROLYZER` (52), `SOTRAGE` (60) |
## Fixing it
In rough dependency order:
1. Add `P_Electro_max` as a tenth input; fix the `Electro_max` typo (B1, B2).
2. Assign every output on every path initialise all six to `0` at the top (B3).
3. Fix line 43 to `P_FuelCell_cont = P_load_PB;` (B4).
4. Negate lines 33 and 61 (B5).
5. Decide the timestep and make every `E_*`/`P_*` conversion explicit (B6).
6. Settle the load sign convention against `Constants.m` (B7).
7. Add a grid-power output; delete the `disp` calls (I1, B10).
8. Rename the file to a legal MATLAB identifier, e.g. `rule_based_controller.m`, and move it
next to the model it drives.
Steps 5 and 6 are the ones that need a decision rather than an edit they are the same
open questions the
[Simulator I/O Interface](../02-specifications/simulator-io-interface.md#5-open-questions)
raises.
## Related
- [Rule-Based Controller](../02-specifications/rule-based-controller.md) the specification this implements
- [Simulator I/O Interface](../02-specifications/simulator-io-interface.md) the contract, including sign conventions and the `dt` question
- [Simulink Model Inventory](simulink-model-inventory.md) the `Constants.m` staleness this depends on
- [RL Action Variables](../02-specifications/rl-action-variables.md) what an RL policy would emit instead