Critical: [SimExec] Stream handoff routes nonlinear flowsheets to the next execution item

[bernalde]

Maintainer triage (June 29, 2026)

• Status: Verified bug against the current master source in this fork.
• Severity: Critical.
• Area labels: area:simexec, area:flowsheet.
• Tackle batch: P1 - critical blockers and severe unit errors.

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

Part of the model-correctness audit — see #17.

Severity: critical | Category: logic | Location: PharmaPy/SimExec.py lines 129-139, 149-153

What's wrong

After solving a unit name, the destination of the Connection is taken as self.execution_names[ind + 1] (the next unit in the topological execution order), while the actual successor(s) of name are in neighbors = self.graph[name]. neighbors is only used as a boolean gate (len(neighbors) > 0); its contents are ignored when choosing the destination. So connection = Connection(source_uo=getattr(self, name), destination_uo=getattr(self, uo_next)) with uo_next = execution_names[ind+1] routes the outlet to whatever unit happens to be next in the sorted order, not to the true downstream unit.

Why it's incorrect

Topological order is not unique and does not equal the physical edge list except for a single linear chain. Example: graph A->C, B->C, C->D (Mixer C fed by A and B). A valid topological order from topological_bfs is [A, B, C, D]. Processing A (ind=0): neighbors=[C] so the gate passes, but execution_names[1]=B, so transfer_data() copies A.Outlet into B's Inlet/Phases. A's material/energy is delivered to the wrong unit and B's true feed is overwritten. This violates mass and energy conservation across the flowsheet (a stream is delivered to a unit that is not connected to its source). It only coincides with correct behavior when the flowsheet is a single linear chain where topological order equals the edge sequence.

Suggested regression test

Build a Y-shaped flowsheet {'A':['C'], 'B':['C'], 'C':['D']} where A, B carry distinct compositions. After SolveFlowsheet, assert that C.Inlets (Mixer) received A.Outlet and B.Outlet, and that no Connection has destination == B. Then assert total mass into C equals mass out of A plus mass out of B. The current code will instead create a Connection A->B.

---

Verification: 1/1 adversarial reviewers confirmed (refuted: 0).

[bernalde]

Independent Codex verification: CONFIRMED

severity_assessment: critical
evidence: PharmaPy/SimExec.py:129-134 reads neighbors but connects to self.execution_names[ind + 1]; PharmaPy/SimExec.py:149-153 repeats the same next-order pattern for already solved units. This misroutes non-linear flowsheets whenever topological order is not identical to each graph edge.
fix_note: Create connections for the actual graph neighbor node or nodes, not the next execution-order entry.

[andres9403]

Triage: reproduced ✅ — stream handoff routes to next execution-order unit, not the graph successor

Defect reproduced: YES. The misrouting is confirmed by running the real SolveFlowsheet routing code on a Y-shaped flowsheet.

Environment

• Base commit: bff897f (origin/master).
• Env: conda environment.yml env (Python 3.9.25, assimulo 3.4.3, numpy 1.26.4, scipy 1.13.1), editable install.
• Install/test (per TESTING.md):

  conda env create -f environment.yml && conda activate pharmapy-assimulo
  python -m pip install -e . --no-deps
  python -m pytest tests/ -m "not assimulo"

Root cause

PharmaPy/SimExec.py:129-141 reads neighbors = self.graph[name] but only uses it as a boolean gate (len(neighbors) > 0); the connection destination is self.execution_names[ind + 1] — the next unit in topological order, not the actual successor. The already-solved branch at :149-153 repeats the same execution_names[ind + 1] pattern. Topological order is not unique and equals the edge list only for a single linear chain, so any fan-in/fan-out flowsheet is misrouted.

Reproduction

Runs the unmodified destination-selection logic in SolveFlowsheet on the graph A→C, B→C, C→D (Mixer C fed by A and B). Only machinery unrelated to routing is neutralized (modeling-object validation, per-unit solve, result aggregation, physical data transfer); a Connection recorder captures the (source → destination) pairs the real code produces.

import os, numpy as np
import PharmaPy.SimExec as se

PATH_PHYS = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(se.__file__))),
                         "tests", "Flowsheet", "data", "compound_database.json")

se.check_modeling_objects = lambda *a, **k: None
se.SimulationResult = lambda *a, **k: None

created = []
class ConnRecorder:
    def __init__(self, source_uo, destination_uo):
        created.append((source_uo._name, destination_uo._name))
    def transfer_data(self): pass
se.Connection = ConnRecorder

class _Result: time = np.array([0.0, 1.0])
class StubUO:
    __module__ = "PharmaPy.Containers"
    def __init__(self, name):
        self._name = name; self.outputs = {}; self.result = _Result()
    def solve_unit(self, **kw): pass
    def flatten_states(self): pass

graph = {"A": ["C"], "B": ["C"], "C": ["D"], "D": []}
flst = se.SimulationExec(PATH_PHYS, flowsheet=graph)
for n in graph: setattr(flst, n, StubUO(n))
print("execution_names :", flst.execution_names)
flst.SolveFlowsheet(verbose=False)
print("connections made:", created)
print("Mixer C receives from:", {s for (s, d) in created if d == "C"}, "(expected {'A','B'})")

Output (PYTHONHASHSEED=0):

execution_names  : ['B', 'A', 'C', 'D']
connections made : [('B', 'A'), ('A', 'C'), ('C', 'D')]
Mixer C receives from: {'A'}  (expected {'A', 'B'})

Observed vs expected

	Observed (base)	Expected
Connections created	B→A, A→C, C→D	A→C, B→C, C→D
B's outlet delivered to	A (overwrites A's feed)	C
Mixer C receives from	{A} only	{A, B}

The result is order-dependent but always wrong: under topological order ['A','B','C','D'] the misrouted edge is A→B and C receives only {B}; under ['B','A','C','D'] it is B→A and C receives only {A}. Either way one feed stream is delivered to the other feed unit (overwriting its inlet) and the Mixer never receives it — a mass/energy conservation violation.

Failing regression test (red on base)

Proposed for tests/ (e.g. tests/unit/test_simexec_routing.py); the unit marker is already registered in pytest.ini.

# -*- coding: utf-8 -*-
"""Regression test for issue #19: stream handoff must follow graph edges,
not topological execution order."""
import os
import numpy as np
import pytest

import PharmaPy
import PharmaPy.SimExec as se

REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(PharmaPy.__file__)))
PATH_PHYS = os.path.join(REPO_ROOT, "tests", "Flowsheet", "data",
                         "compound_database.json")


class _Result:
    time = np.array([0.0, 1.0])


class _StubUO:
    __module__ = "PharmaPy.Containers"  # skip flatten_states branch

    def __init__(self, name):
        self._name = name
        self.outputs = {}
        self.result = _Result()

    def solve_unit(self, **kw):
        pass

    def flatten_states(self):
        pass


@pytest.mark.unit
def test_stream_handoff_follows_graph_edges_not_execution_order(monkeypatch):
    """Y-shaped flowsheet A->C, B->C, C->D (Mixer C fed by A and B)."""
    graph = {"A": ["C"], "B": ["C"], "C": ["D"], "D": []}

    created = []

    class ConnRecorder:
        def __init__(self, source_uo, destination_uo):
            created.append((source_uo._name, destination_uo._name))

        def transfer_data(self):
            pass

    monkeypatch.setattr(se, "check_modeling_objects", lambda *a, **k: None)
    monkeypatch.setattr(se, "SimulationResult", lambda *a, **k: None)
    monkeypatch.setattr(se, "Connection", ConnRecorder)

    flst = se.SimulationExec(PATH_PHYS, flowsheet=graph)
    for name in graph:
        setattr(flst, name, _StubUO(name))

    flst.SolveFlowsheet(verbose=False)

    misrouted = [(s, d) for (s, d) in created if d not in graph[s]]
    assert not misrouted, "stream routed to non-neighbor unit(s): %s" % misrouted

    feeders_of_C = {s for (s, d) in created if d == "C"}
    assert feeders_of_C == {"A", "B"}, (
        "Mixer C should receive A and B, got %s" % feeders_of_C)

Red output on unchanged base source:

>       assert not misrouted, "stream routed to non-neighbor unit(s): %s" % misrouted
E       AssertionError: stream routed to non-neighbor unit(s): [('B', 'A')]
E       assert not [('B', 'A')]
...
1 failed

(Under PYTHONHASHSEED=2 the same assertion fails with [('A', 'B')], confirming the failure is independent of the non-unique topological order.)

Acceptance criteria (definition of done for the fix)

1. Each solved unit's outlet is delivered to its actual graph successor(s) in self.graph[name], never to self.execution_names[ind + 1].
2. Fan-out (a unit with multiple successors) creates a connection to each successor; fan-in (multiple sources feeding a Mixer) delivers each source to the Mixer.
3. For {'A':['C'],'B':['C'],'C':['D']}: connections A→C, B→C, C→D are created; no connection targets a non-neighbor (never A→B/B→A); Mixer C receives both A and B.
4. Mass/energy conservation holds: total mass into C equals mass out of A plus mass out of B; no feed inlet is overwritten by an unrelated source.
5. Routing is independent of the (non-unique) topological order returned by topological_bfs.
6. The mirrored already-solved branch (elif at :149-153) uses the same graph-neighbor routing.
7. The regression test above passes, and existing tests still pass — linear-chain flowsheets (e.g. R01 --> CR01 --> F01) remain correctly routed.

Fix location

PharmaPy/SimExec.py:129-141 and the mirrored branch :149-153.

Next step

gh-issue-to-pr 19 — the fix PR should reuse the failing test above and satisfy these acceptance criteria.