Skip to content

CODE_REVIEW.md

Latest commit

 

History

History
49 lines (41 loc) · 3.43 KB

CODE_REVIEW.md

File metadata and controls

49 lines (41 loc) · 3.43 KB
Raw

Code Review: python_magnetcooling

Updated after fixes applied through this branch claude/update-code-review-PESmk.

What Was Fixed ✅

Issue Commit / Branch Detail
pextra dropped in hcorrelation 5bf1c67 Now Pextra=pextra correctly forwarded to Uw
Montgomery offset 273 → 273.15 025b541 Temperature conversion now matches correlations.py
Swamee constants 025b541 Now 1.325 / 3.7, matching standard formula
Optional[float] truthy check in _compute_channel_uniform 65e7727 Uses is not None
Optional[float] truthy check in _compute_channel_axial 65e7727 Uses is not None
matplotlib unconditional import d79b4fe Deferred to inside _create_plot with try/except
y_spec.pop("col") mutation d79b4fe Uses y_spec["col"] + filtered copy of kwargs
specific_heat_outlet wrong units prior branch Now Steam_outlet.cp [kJ/kg/K]; unit annotations updated
Truthy check in compute() (issue #2) Z2Ab6 branch thermohydraulics.py:209,211 now use is not None
_compute_mixed_outlet_temp unused pressure param (issue #7) Z2Ab6 branch Parameter removed; method now takes only channels
Duplicate class definitions in thermohydraulics.py (issue #8) Z2Ab6 branch All dataclasses imported from channel.py; no local copies
OOP modules not used by main solver (issue #9) Z2Ab6 branch ThermalHydraulicCalculator now routes through correlations.py / friction.py
fuzzy_factor incorrectly applied to Dittus/Colburn/Silverberg in correlations.py prior branch fuzzy is Montgomery-only; removed self.fuzzy_factor * from DittusBoelterCorrelation, ColburnCorrelation, SilverbergCorrelation
uguess=0 default in Uw (issue #4) this branch Changed to uguess: float = 1.0; added U = max(uguess, 1e-3) guard
compute_from_waterflow mutates caller's input (issue #5) this branch Uses dataclasses.replace(inputs, ...) to work on a copy
Global warning suppression in waterflow.py (issue #6) this branch Replaced simplefilter("ignore") with targeted pint-only filter
Dead importlib_metadata fallback in __init__.py (issue #10a) this branch Removed unreachable except ImportError branch; project requires Python 3.11+
Dead code in cooling.py:129-151 (issue #10b) prior branch Bare multi-line string of unimplemented friction variants was already removed
Commented-out heatBalance in heatexchanger_primary.py (issue #10c) this branch Removed commented-out function

Remaining Code Quality Issues

11. Incomplete test coverage

  • test_correlations.py only covers MontgomeryCorrelation and the base HeatCorrelation.compute_nusselt. DittusBoelterCorrelation, ColburnCorrelation, and SilverbergCorrelation have no tests.
  • test_friction.py only covers ConstantFriction and BlasiusFriction. FilonenkoFriction, ColebrookFriction, and SwameeFriction have no tests.
  • ThermalHydraulicCalculator has no direct unit tests (all six cooling levels: mean, meanH, grad, gradH, gradHZ, gradHZH).
  • No cross-checking tests between legacy cooling.py functions and their OOP counterparts in correlations.py / friction.py.
  • A multi-channel integration test for outlet_temp_mixed would catch future regressions in the specific_heat_outlet / compute_mixed_outlet_temperature weighting.