Files
Entropyk/plans/audit-2026-07-remediation-plan.md
sepehr 5bd180b5b8
Some checks failed
CI / check (push) Has been cancelled
Snapshot WIP: solver HP epic progress, BPHX/HX physics, BMAD skill refresh.
Capture uncommitted solver robustness work (regularization, domain errors, linear solver lifecycle, tube DP/MSH), web workbench updates, and synced BMAD skills across IDE agent folders before starting BPHX pressure-drop.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-07-19 16:35:31 +02:00

173 lines
17 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.
# Entropyk Audit — Findings & Remediation Plan
**Date**: 2026-07-17
**Scope**: Full workspace audit (docs vs Rust code, tests, bindings, dead code)
**Audience**: Executing agent — this document is self-contained. Read it fully before acting.
---
## 0. Project Context You Need
Entropyk is a Rust thermodynamic cycle simulation library (HVAC/R). Workspace layout:
- `crates/core` — physical newtypes (Pressure Pa, Temperature K, Enthalpy J/kg, MassFlow kg/s)
- `crates/fluids``FluidBackend` trait + backends: CoolProp (feature `coolprop`, ON via facade), Tabular (bicubic), Incompressible
- `crates/components` — all thermodynamic components implementing the `Component` trait
- `crates/solver` — Newton-Raphson (Armijo line search, Jacobian freezing), Picard fallback, inverse control
- `crates/entropyk` — facade (SystemBuilder, rating, SimulationResult)
- `crates/cli` — CLI (8 subcommands: run, batch, validate, qualify, rate, scop, seer, schema)
- `crates/vendors` — vendor data parsers (Copeland, Danfoss, SWEP, Bitzer)
- `bindings/{python,c,wasm}` — at repo ROOT (NOT `crates/bindings/` as AGENTS.md claims)
- `demo/`, `apps/web` (Next.js), `ui/` (dead)
### Key mechanisms (post-CM1.x changes — the source of most test failures)
1. **State vector is `[ṁ, P, h]` per edge** (3 unknowns/edge), formerly `[P, h]`. Docs still say `[P, h]`.
2. **DoF gate in `System::finalize()`** (`crates/solver/src/system.rs:762-792`): over-constrained systems are REJECTED with `TopologyError::DofImbalance`. This is DELIBERATE. Escape hatch for tests: `set_enforce_dof_gate(false)` on `System`.
3. **Picard validates squareness**: `crates/solver/src/strategies/sequential_substitution.rs:305-313` returns `InvalidSystem` if `n_state != n_equations`.
4. **Canonical post-CM1.2 mock pattern** (see `crates/solver/tests/homotopy_continuation.rs`, which PASSES): each mock has 2 equations (P and h) AND implements `port_mass_flows()` providing mass-flow continuity, e.g. `Ok(vec![MassFlow::from_kg_per_s(0.05), MassFlow::from_kg_per_s(-0.05)])`.
### Hard rules for the executing agent
- **NEVER modify production code (`crates/*/src`, `bindings/*/src`) in Steps 1-2** — test-only changes. The DoF gate, the `"newton"` default, and the squareness check are deliberate.
- Project policy (AGENTS.md): zero-panic, exact Jacobians (no numerical differentiation unless explicitly requested), English for code/comments/docs.
- Verify with: `cargo test -p <package>` after each file; full `cargo test --workspace --no-fail-fast` at the end of Step 1.
- Do NOT run `git commit`/`git push` — the user handles git.
---
## 1. Audit Findings Summary
**Good**: workspace compiles cleanly (`cargo check --workspace --all-targets`, only cosmetic warnings). Crate unit tests all pass: components 867, solver lib 290, core 160, entropyk 75, fluids 115, cli lib 57. Solver features (Newton+Armijo, Jacobian freezing, Picard ω=0.5, fallback, Anderson acceleration, homotopy) exist and are individually tested.
**Bad**: ~28 integration tests FAIL in 16 suites (details in §2). Exact-Jacobian policy violated in production (§3). Python/C bindings ship mocked/stubbed physics without any user warning (§4). Various P1/P2 gaps (§5).
---
## 2. STEP 1 — Fix the failing tests (P0-0)
Test failures fall in 3 categories. Category A = obsolete tests after CM1.2/DoF gate (fix tests). Category B = genuine suspects (Step 2, do NOT fix yet). Category C = deliberate default change (update expectations).
### 2.1 Category A — re-balance obsolete mocks
**`crates/entropyk/tests/constraints_api.rs`** — `test_builder_dof_imbalance_two_constraints_one_control` (line ~125): the test does `.build().expect("build should succeed")` then asserts `validate_inverse_control_dof()` fails. With the DoF gate, `build()` now fails first with DofImbalance (5 eqs vs 4 unknowns). **Fix**: rewrite the test to assert `build()` returns the DofImbalance error (the new contract). Keep `test_builder_constraints_link_and_validate_dof` (passes) untouched.
**`crates/entropyk/tests/port_validated_edges.rs`** — `test_build_system_with_port_validated_edges` (line ~201): build fails, 6 eqs vs 5 unknowns (3 mock nodes × 2 eqs). The test targets port validation, not DoF. **Fix**: re-dimension mock `n_eqs` to balance (or, if SystemBuilder exposes it, disable the gate — check first; prefer re-balancing).
**`crates/entropyk/tests/simulation_result.rs`** — `test_realistic_cycle_json_output` (line ~102): finalize fails, 12 eqs vs 10 unknowns (compressor mock 6 eqs, condenser/valve/evap 2 each). **Fix**: adjust mock `n_eqs` to square the system (test targets JSON serialization).
**`crates/solver/tests/fallback_solver.rs`** — 7 failures:
- `test_fallback_stiff_nonlinear`, `test_timeout_across_switches`, `test_oscillation_prevention_newton_to_picard_stays`, `test_newton_redivergence_commits_to_picard`: `InvalidSystem "State dimension (3) does not match equation count (2)"`. Mock `StiffNonlinearSystem` (line 95) has n=2 cubic equations reading `state[1+i]` but the single self-loop edge yields 3 unknowns. **Fix**: add a 3rd equation for the ṁ slot (index 0, e.g. `r[n] = state[0] - target` with its Jacobian entry), preserving intent (fallback Newton↔Picard on stiff system).
- `test_fallback_disabled_pure_newton` (~line 227), `test_fallback_both_solvers_can_converge` (~362), `test_fallback_solver_integration` (~631): "Should converge with Newton" — same root cause on `LinearSystem` (line 48) / related mocks. Fix the same way.
**`crates/solver/tests/chiller_air_glycol_integration.rs`** — `test_two_circuit_chiller_topology` (line ~438): 46 eqs vs 36 unknowns (mock at line 104). **Fix**: re-dimension mocks (n_eqs and/or `port_mass_flows`) to balance both circuits.
**`crates/solver/tests/convergence_criteria.rs`** — `test_newton_with_criteria_single_circuit` (line ~283): 4 eqs vs 3 unknowns, `MockConvergingComponent` (line 246). **Fix**: add the ṁ equation or `port_mass_flows`.
**`crates/solver/tests/inverse_control.rs`** — `test_3x3_jacobian_block_is_fully_dense` (line ~1097): 9 eqs vs 7 unknowns, `MockPassThrough` (line 26). **Fix**: balance mocks; preserve the dense-3x3-Jacobian intent.
**`crates/solver/tests/macro_component_integration.rs`** — 5+ tests (`test_4_component_cycle_macro_creation`, `test_two_macro_chillers_in_parallel_topology`, `test_coupling_residuals_nonzero_at_inconsistent_state`, `test_jacobian_coupling_entries_correct`, `test_4_component_cycle_expose_two_ports`): all fail at line ~87, 12 eqs vs 9 unknowns, `PassThrough` mock (line 23). **Fix**: balance the mocks.
**`crates/cli/tests/single_run.rs`** — `test_failed_run_json_includes_failure_diagnostics` (line ~1365): the scenario now fails with an under-constrained DoF error (pump1/pump2, 4 eqs vs 5 unknowns) BEFORE the solver runs, so no `failure_diagnostics` are emitted. The test intent: a run that fails AFTER iterating must include `failure_diagnostics` in the JSON output. **Fix**: change the test config so the system is DoF-square but the SOLVER genuinely fails after some iterations (e.g. physically incompatible constraints or unreachable targets). Read how `failure_diagnostics` is produced in `crates/cli/src` (grep `failure_diagnostics`) and see `crates/solver/tests/failure_diagnostics.rs` (passes) for a working scenario.
### 2.2 Category C — update expectations
**`crates/cli/tests/config_parsing.rs`** — `test_parse_minimal_config` (line 14) and `test_solver_config_defaults` (line 173) expect `strategy == "fallback"`; the deliberate default is `"newton"` (`crates/cli/src/config.rs:444-446`, doc: `"newton"` or `"picard"`). **Fix**: update expectations to `"newton"`.
### 2.3 Step 1 acceptance
`cargo test --workspace --no-fail-fast`: everything green EXCEPT the 4 Category-B tests (§3), which must still fail at this stage.
---
## 3. STEP 2 — Investigate Category B (genuine suspects)
- `crates/solver/tests/inverse_calibration.rs::test_inverse_calibration_f_ua``NonConvergence { iterations: 100, final_residual: 9700.0 }`
- `crates/solver/tests/inverse_calibration_algorithm.rs::test_single_factor_calibration_f_ua``ConvergenceFailure { factor: "evaporator.z_ua", iterations: 100, residual_norm: 9700.0 }`
- `crates/solver/tests/jacobian_freezing.rs::test_frozen_jacobian_converges_cubic_system` and `test_auto_recompute_on_divergence_trend` — hard divergence (`Residual 999700029999 exceeds threshold 10000000000`)
First check whether these are also squareness victims (Category A in disguise). If not: the calibration failures are consistent with FD/incomplete Jacobians (§4) — the evaporator `z_ua` calibration drives a `HeatExchanger` whose Jacobian is 100% finite-difference. Document the root cause; if it is the FD Jacobian, the fix lands with Step 4.
---
## 4. STEP 3 & 4 — Exact Jacobians (P0-1)
### 4.1 Step 3 — Screw compressor (quick win, ~half day)
`crates/components/src/screw_economizer_compressor.rs:570-605``jacobian_entries` only emits trivial entries (∂/∂ṁ = ±1, ∂/∂W = ±1). Missing:
- **Row 3** (`r3 = p_eco_port √(p_suc·p_dis)`): zero entries. Add ∂r3/∂p_eco = 1, ∂r3/∂p_suc = 0.5·√(p_dis/p_suc), ∂r3/∂p_dis = 0.5·√(p_suc/p_dis). The P column indices must be resolved like the ṁ indices already are (`set_system_context` / port context — check how `suction_m_idx` etc. are wired and mirror it for pressures).
- **Rows 0, 1, 4**: curve derivatives ∂(m_suc_calc, m_eco_calc, w_calc)/∂(p_suc, p_dis). Curves are 2D polynomials — derive analytically; chain with ∂SST/∂p_suc, ∂SDT/∂p_dis (backend `dT_sat/dP`).
- Validate with finite-difference tests (pattern already used in the crate's test modules).
### 4.2 Step 4 — `HeatExchanger` generic (12 days)
`crates/components/src/heat_exchanger/exchanger.rs:816-865` — in 4-port mode (the only functional mode; otherwise `compute_residuals` errors with `live_state_required_error`), the Jacobian is **100% central finite differences**: 10 columns × 2 residual evaluations, each evaluation triggering up to 4 fluid-backend calls. This kills Newton's quadratic convergence and is the most expensive solver path.
**Fix**: add a `jacobian()` method to the `HeatTransferModel` trait (`crates/components/src/heat_exchanger/model.rs:125`):
- LMTD and ε-NTU residuals are analytically derivable w.r.t. the 4 ports' (T, ṁ).
- The only hard part: ∂T/∂(P,h) per fluid. Water/air: 1/Cp. Refrigerants: CoolProp analytic derivatives (`d(T)/d(Hmass)|P`); tabular backend: spline derivatives are free.
- Wire through `HeatExchanger::jacobian_entries` replacing the FD block. Keep a feature-flagged FD path for cross-checking.
- Validate: FD-comparison tests + rerun Category-B calibration tests (Step 2) + `cargo test -p entropyk-cli --test hx_standalone` (currently passes in ~29 s — must stay green, and should get faster).
### 4.3 Other FD violations in production (fix after Step 4, same pattern)
- `compressor.rs:1257-1307` (∂r/∂h via FD; also `estimate_density`/`estimate_temperature` at :1632-1705 are hardcoded placeholders bypassing FluidBackend)
- `pump.rs:593-611`, `pipe.rs:768-774` (polynomial curves — analytically derivable)
- `anchor.rs:294-313`, `isentropic_compressor.rs:967-1039`, `isenthalpic_expansion_valve.rs:337`
- `condenser.rs:1301-1311,1384-1392`, `evaporator.rs:1088-1096,1129-1140` (localized `dT_sat/dP` FD — acceptable if backend exposes no analytic derivative; document the justification)
---
## 5. STEP 5 — Binding honesty (P0-2)
### 5.1 The mocks (verified inventory)
**Python**`crates/components/src/python_components.rs` (14 `Py*Real` classes; ALL have EMPTY `jacobian_entries`):
| Python class | Reality |
|---|---|
| `Compressor` | ΔP hardcoded **+1 MPa** (:220); AHRI coeffs m7m10 and `efficiency` unused; docstring claims "real physics" |
| `ExpansionValve` | ΔP hardcoded 1 MPa (:327); `opening` parameter validated but NEVER used |
| `Evaporator` | fixed anchors 3.5 bar / 410 kJ/kg (:477-478); user UA/conditions ignored |
| `Pipe` | ΔP = 0 (:628, "no pressure drop for testing"); Darcy-Weisbach code exists but `dead_code` |
| `Pump` / `Fan` | **are `PyPipeReal`** (`bindings/python/src/components.rs:561,606`); `pressure_rise_pa` ignored |
| `FlowMerger` | unweighted averaging (physically wrong, :948-966) |
| `FlowSink` | stub, 0 equations |
| boundaries (Refrigerant/Brine/Air Source/Sink) | REAL — leave alone |
**C**`bindings/c/src/components.rs`: `compressor_create` (:114), `expansion_valve_create` (:194), `pipe_create` (:257) → `SimpleAdapter`: residuals = 0 (invisible to solver), parameters validated then discarded (`let _ = ahri_coeffs;`). FFI docs claim real behavior. `condenser/evaporator/economizer_create` are REAL.
**Demo**: `demo/src/bin/eurovent.rs:417-420` copies `eurovent_report.html` verbatim — committed hardcoded values presented as simulation output.
### 5.2 Remediation (two phases)
- **Phase A (half day)**: explicit `DeprecationWarning`/`RuntimeWarning` at construction of every mocked Python class stating which physics is mocked; honest docstrings; a printed warning or doc note for the 3 C stubs; mark the Eurovent report as a static sample. No physics changes.
- **Phase B (23 days, after Step 4)**: delegate `Py*Real` to the real Rust components (they all exist) and replace `SimpleAdapter` with real `Compressor`/`ExpansionValve`/`Pipe`. Move `python_components.rs` OUT of `crates/components` into `bindings/python` (dependency inversion).
---
## 6. P1/P2 Backlog (not in scope for immediate execution — record for planning)
1. **Factory divergence**: `crates/cli/src/run.rs::create_component` (33 arms) vs `crates/components/src/registry.rs` — registry lacks IsentropicCompressor, IsenthalpicExpansionValve, ReversingValve, Bphx*, Anchor, HeatSource, ThermalLoad, FreeCoolingExchanger, MchxCondenserCoil, AirCooledCondenser, FinCoilCondenser → `SystemBuilder::to_config_json` output is NOT re-loadable. Unify the two factories.
2. **Facade gaps**: `crates/entropyk/src/lib.rs` does not re-export `Bphx*`, `FloodedCondenser`, `Economizer`, `Node`, `MovingBoundaryHX` (the latter is implemented, tested, documented — but unreachable). CLI lacks arms for `Economizer`, `FloodedCondenser`, `Node`.
3. **Silent fallback**: `run.rs:3670``"Ahri540" | "AHRI540" | "ahri540" | _` arm swallows ANY typo in `model_type`. Remove the `_`. Also `run.rs:4286` "Supported:" error message is incomplete.
4. **Dead code**: `crates/components/src/{python_components_clean.rs, python_boundary_append.rs, mode_switch.rs, pump_controller.rs}` (not declared in lib.rs), `crates/components/patch_hx.py`, `bindings/python/src/{types_appended.rs, types_clean.rs}`, `ui/` (calls removed endpoint `/api/calculate`), `demo/{inverse_control_template.html, macro_chiller_schema.html}` (orphans), 5 committed Mach-O arm64 binaries in `bindings/c/tests/`, `bindings/python/{test!entropyk.py, print_eqs.py, test_eq_count.py}`, `crates/solver/tests/_tmp_analytic.rs`, `crates/cli/examples/_ui_repro_flooded_port.json`. `crates/vendors` has zero dependents.
5. **Docs to sync**: AGENTS.md (`crates/bindings/``bindings/`; `uv pip install -e ./bindings/python`), DOCUMENTATION.md (state vector [ṁ,P,h]; `Density` newtype doesn't exist; incompressible ρ is quadratic not linear; phases enum is Liquid/Vapor/TwoPhase/Supercritical), CLI_TUTORIAL.md (references 3 nonexistent example files; missing `qualify`/`schema` commands), `apps/web/README.md` (port 3031 → 3030), `crates/fluids/build.rs:12` (redundant `dylib=coolprop`).
6. **Zero-panic violations**: production `unwrap()`/`expect()` in condenser.rs (22), evaporator.rs (23), flooded_evaporator.rs (21), exchanger.rs (18), builder.rs (3), coolprop.rs:355,377 (RwLock), coolprop-sys (CString::new).
7. **Mojibake**: `python_components.rs` has 36 lines with U+FFFD and French comments — violates the English-docs convention.
---
## 7. Execution Order & Validation
| Step | Scope | Effort | Validation |
|---|---|---|---|
| 1 | Fix Category A + C tests (test files ONLY) | ½1 d | `cargo test --workspace --no-fail-fast` — all green except 4 Category-B |
| 2 | Root-cause Category B (no fixes yet) | ½ d | Written diagnosis; decide if fixed by Step 4 |
| 3 | Screw compressor analytic Jacobian | ½ d | FD-validation tests + `cargo test -p entropyk-components` |
| 4 | `HeatTransferModel::jacobian()` + wire-in | 12 d | FD-comparison tests; hx_standalone green; Category-B retest |
| 5A | Binding warnings + honest docs | ½ d | `cargo check --workspace`; manual warning check |
| 5B+ | Real delegation, factory unification, dead code, docs sync, zero-panic | later | per-item |
**Uncommitted work warning**: the repo currently has ~1470 modified-but-uncommitted files (211 `.rs`, +24 453/5 698 lines). Recommend the user commits BEFORE starting Step 1 so remediation diffs are reviewable.