Skip to content

High: [Containers/Phases] DynamicCollector divides mass-basis enthalpy by mole-basis Cp #84

Description

@sergey-gusev94

Maintainer triage (June 29, 2026)

  • 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.

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.

Metadata

Metadata

Assignees

Labels

area:containersMixer, collector, holdup, and container unit operationsarea:phasesPhase, stream, and mixed-phase property handlingarea:thermoThermodynamics, VLE, enthalpy, Cp, density, and viscositybugSomething isn't workingcorrectnessModel/numerical correctness defectseverity:highConfirmed bug with major correctness impact or broken documented workflowstatus:verifiedSource-reviewed against the current codebase

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions