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

17 KiB
Raw Blame History

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/fluidsFluidBackend 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.rstest_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.rstest_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.rstest_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.rstest_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.rstest_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.rstest_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.rstest_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.rstest_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_uaNonConvergence { iterations: 100, final_residual: 9700.0 }
  • crates/solver/tests/inverse_calibration_algorithm.rs::test_single_factor_calibration_f_uaConvergenceFailure { 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-605jacobian_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)

Pythoncrates/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

Cbindings/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.