Skip to content

High: [SolidLiquidSep] DisplacementWashing static solve indexes 2D output as time series #86

Description

@sergey-gusev94

Maintainer triage (June 29, 2026)

  • Status: Verified bug against the current master source in this fork.
  • Severity: High.
  • Area labels: area:separations.
  • Tackle batch: P3 - documented-mode crashes and tooling breakages.

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)

conc_all sized for 2-D steady result but loop iterates over num_t time points (broadcast/index crash)

Location: PharmaPy/SolidLiquidSep.py:1207-1210

In the dynamic=False branch, conc_adim = self.material_bce(...) is 2-D (num_z, num_species) (material_bce, lines 1123-1140, has no time axis), and conc_all = np.zeros_like(conc_adim) is also (num_z, num_species). The loop 'for i in range(self.num_t): conc_all[:,i] = conc_adim[:,i]*(c_zero - c_inlet) + c_inlet' was copy-pasted from the dynamic branch where conc_adim is 3-D (num_z, num_t, num_species). In 2-D, conc_adim[:,i] is a column (num_z,) and (c_zero - c_inlet) is (num_z, num_species), so the product fails to broadcast at i=0 (ValueError 'operands could not be broadcast together with shapes (num_z,) (num_z,num_species)'); and for i >= num_species, conc_all[:,i] is out of bounds (IndexError). The correct vectorized 2-D form is the already-present commented line 1211.

  • Trigger: DisplacementWashing(solvent_idx, num_nodes, diam_unit=...) on a multi-component cake, then solve_unit(deltaP, wash_ratio=..., dynamic=False) -- the documented static Lapidus-Amundson wash mode. Default time_vals = np.linspace(eps, time_total) (50 points, line 1188); num_species (typically 2) << num_t, so the crash is guaranteed.
  • Wrong result: Immediate ValueError/IndexError at line 1210, so the entire static washing solve is unrunnable. (Even if it survived, retrieve_results assumes 3-D, e.g. np.transpose(conc,(1,0,2)) at line 1235, so the non-dynamic path is unwired.)
  • Suggested fix: In the dynamic=False branch, drop the time loop and use the commented vectorized form: conc_all = conc_adim*(c_zero - c_inlet) + c_inlet; or guard the loop with 'if dynamic:' and give the non-dynamic branch a consistent representation that retrieve_results can consume. Make conc_all's rank match conc_adim's actual rank in each mode.
  • Net-new (not a duplicate): Distinct from Medium: [SolidLiquidSep] DisplacementWashing diffusivity treats micrometer sizes as meters #65 (get_diffusivity units). This is a control-flow/shape crash from reusing the 3-D dynamic assembly loop for the 2-D steady output.

Related existing issues (referenced, not modified): #65 (get_diffusivity micrometer-as-meters units bug, different method/lines) -- reference only.


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:separationsSolid-liquid separation, deliquoring, and displacement washingbugSomething 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