You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Status: Verified bug against the current master source in this fork.
Severity: High.
Area labels: area:containers, area:phases, area:thermo.
Tackle batch: P2 - shared thermo, phase, and basis correctness.
The source review confirmed that the cited code path and failure mode are still present. The original audit details are preserved below for reproduction notes and suggested fixes.
Original audit details
Severity: High · Part of #67 (round-2 audit batch)
Two confirmed findings describe ONE bug from two angles: the root-cause default basis='mole' in LiquidPhase.getCp (Phases.py:355) and the call site that relies on that default (Containers.py:790). They are fixed together (the cleanest fix is the call site, the most general is the default), so they are a single group with two co-dependent defect entries rather than independent bugs. The third 'DynamicCollector ... mole-basis Cp' finding is the same call-site defect verified separately and is merged into the Containers.py:790 entry.
This issue bundles 2 defects that share one fix/PR.
1. (High) LiquidPhase.getCp defaults to basis='mole', breaking mass-basis energy balances
Location:PharmaPy/Phases.py:355
LiquidPhase.getCp is declared with basis='mole', whereas every sibling accessor defaults to mass: SolidPhase.getCp (Phases.py:1187), VaporPhase.getCp (Phases.py:614), and LiquidPhase.getEnthalpy (Phases.py:367) / getDensity (Phases.py:340) all default basis='mass'. getCp forwards to ThermoModule where cpMass = cpMole/mw*1000 (ThermoModule.py:162), so a caller omitting the basis argument silently receives J/mol/K instead of the library-standard J/kg/K (differing by 1000/mw_av). Every getCp call inside an energy balance explicitly passes basis='mass' (Containers.py:111, Drying_Model.py:370) EXCEPT the DynamicCollector call site.
Trigger: Any caller of LiquidPhase.getCp (or LiquidStream.getCp, inherited) that relies on the default basis in a mass-basis context -- notably DynamicCollector.energy_balance.
Wrong result: Mole-basis Cp (~1000/mw_av times smaller, 10x for mw100) returned where mass-basis is expected; no error raised.
Suggested fix: Change the default to basis='mass' in LiquidPhase.getCp (Phases.py:355) to match getEnthalpy/getDensity and the Solid/Vapor getCp signatures. (Or fix the call site, below.)
2. (High) DynamicCollector energy_balance divides mass-basis enthalpy by mole-basis Cp
Location:PharmaPy/Containers.py:790 (in energy_balance, lines 783-794)
h_in (line 788) and h_tank (line 789) use getEnthalpy with default basis='mass' -> J/kg, but cp_tank = self.Liquid_1.getCp(temp=temp, mass_frac=fracs) (line 790) passes NO basis, so the buggy LiquidPhase.getCp default 'mole' returns J/(molK) (getCpMix auto-converts mass_frac to mole_frac, so it does not crash). Line 792 dtemp_dt = inlet_flow/mass/cp_tank(h_in-h_tank) thus divides J/kg by J/(molK), giving units molK/(s*kg) instead of K/s, numerically wrong by ~cp_mass/cp_mole = 1000/mw (~5x-55x). The sibling ContinuousHoldup.energy_balance (line 111) correctly passes basis='mass', confirming the omission.
Trigger: DynamicCollector with a non-MixedPhases liquid inlet (auto model_type='liquid_mixer', Containers.py:711-712), set Inlet, call solve_unit(runtime=...) non-isothermally; CVode invokes unit_model -> energy_balance on every RHS evaluation. Exercised in tests/Flowsheet/flowsheet_tests.py:212,310.
Wrong result: The tank temperature derivative is scaled by ~1/mw, so the collected-tank temperature trajectory (tempProf/result.temp/Outlet.temp) is physically wrong by ~1 order of magnitude and energy is not conserved for any non-isothermal feed; no exception raised. (Isothermal feed masks it since h_in-h_tank=0.)
Suggested fix: Pass basis='mass' at the call site: cp_tank = self.Liquid_1.getCp(temp=temp, mass_frac=fracs, basis='mass'), matching the mass-basis enthalpies and the ContinuousHoldup convention (line 111). (Fixing the Phases.py:355 default also resolves this.)
Related existing issues (referenced, not modified):#25 (Mixer slurry cp/mass mismatch), #38 (Mixer enthalpy reference temperatures), #75 (DynamicCollector state-vector ordering / dead crystallizer plot branch) -- reference only; none address the getCp default basis.
Found by a multi-agent source audit (round 2); confirmed by re-reading the source and cross-checked against all 80 existing open issues. Each defect was verified by an independent code-truth skeptic and a novelty skeptic.
Maintainer triage (June 29, 2026)
mastersource in this fork.The source review confirmed that the cited code path and failure mode are still present. The original audit details are preserved below for reproduction notes and suggested fixes.
Original audit details
Severity: High · Part of #67 (round-2 audit batch)
Two confirmed findings describe ONE bug from two angles: the root-cause default basis='mole' in LiquidPhase.getCp (Phases.py:355) and the call site that relies on that default (Containers.py:790). They are fixed together (the cleanest fix is the call site, the most general is the default), so they are a single group with two co-dependent defect entries rather than independent bugs. The third 'DynamicCollector ... mole-basis Cp' finding is the same call-site defect verified separately and is merged into the Containers.py:790 entry.
This issue bundles 2 defects that share one fix/PR.
1. (High) LiquidPhase.getCp defaults to basis='mole', breaking mass-basis energy balances
Location:
PharmaPy/Phases.py:355LiquidPhase.getCp is declared with basis='mole', whereas every sibling accessor defaults to mass: SolidPhase.getCp (Phases.py:1187), VaporPhase.getCp (Phases.py:614), and LiquidPhase.getEnthalpy (Phases.py:367) / getDensity (Phases.py:340) all default basis='mass'. getCp forwards to ThermoModule where cpMass = cpMole/mw*1000 (ThermoModule.py:162), so a caller omitting the basis argument silently receives J/mol/K instead of the library-standard J/kg/K (differing by 1000/mw_av). Every getCp call inside an energy balance explicitly passes basis='mass' (Containers.py:111, Drying_Model.py:370) EXCEPT the DynamicCollector call site.
10x for mw100) returned where mass-basis is expected; no error raised.2. (High) DynamicCollector energy_balance divides mass-basis enthalpy by mole-basis Cp
Location:
PharmaPy/Containers.py:790 (in energy_balance, lines 783-794)h_in (line 788) and h_tank (line 789) use getEnthalpy with default basis='mass' -> J/kg, but cp_tank = self.Liquid_1.getCp(temp=temp, mass_frac=fracs) (line 790) passes NO basis, so the buggy LiquidPhase.getCp default 'mole' returns J/(molK) (getCpMix auto-converts mass_frac to mole_frac, so it does not crash). Line 792 dtemp_dt = inlet_flow/mass/cp_tank(h_in-h_tank) thus divides J/kg by J/(molK), giving units molK/(s*kg) instead of K/s, numerically wrong by ~cp_mass/cp_mole = 1000/mw (~5x-55x). The sibling ContinuousHoldup.energy_balance (line 111) correctly passes basis='mass', confirming the omission.
Related existing issues (referenced, not modified): #25 (Mixer slurry cp/mass mismatch), #38 (Mixer enthalpy reference temperatures), #75 (DynamicCollector state-vector ordering / dead crystallizer plot branch) -- reference only; none address the getCp default basis.
Found by a multi-agent source audit (round 2); confirmed by re-reading the source and cross-checked against all 80 existing open issues. Each defect was verified by an independent code-truth skeptic and a novelty skeptic.