diff --git a/docs/api.md b/docs/api.md index 3031a07..2ea53d5 100644 --- a/docs/api.md +++ b/docs/api.md @@ -156,8 +156,12 @@ the original stream handle to attach more than one device at distinct fractions. ### Automatic face selection `auto_faces=True` lets the engine choose among a symbol's movable port faces. -Use `nozzle()` to force a face when the piping convention matters. The choice is -made on the rendered sheet; a `Block`'s declared faces turn with the block. +It passes over a face blocked by another unit. Use `nozzle()` to force a face +when the piping convention matters. The choice is made on the rendered sheet; a +`Block`'s declared faces turn with the block. + +`Reactor` variants `default` and `plain` offer `outlet` on `"S"`, `"E"` or +`"W"`. `"S"` is used unless a side face gives a more direct run. Symbols whose function depends on gravity should keep their upright orientation. Validation reports `gravity-turned` when one is rotated away from it. diff --git a/docs/gallery/01_ammonia_loop.png b/docs/gallery/01_ammonia_loop.png index 28f8c10..ccbdecc 100644 Binary files a/docs/gallery/01_ammonia_loop.png and b/docs/gallery/01_ammonia_loop.png differ diff --git a/docs/gallery/01_ammonia_loop.svg b/docs/gallery/01_ammonia_loop.svg index 1288e6a..ef2f951 100644 --- a/docs/gallery/01_ammonia_loop.svg +++ b/docs/gallery/01_ammonia_loop.svg @@ -52,7 +52,7 @@ - + @@ -61,8 +61,8 @@ S1 S2 - - S3 + + S3 S4 diff --git a/docs/gallery/10_ethanol_pfd.png b/docs/gallery/10_ethanol_pfd.png index 497b6f8..97ab3ce 100644 Binary files a/docs/gallery/10_ethanol_pfd.png and b/docs/gallery/10_ethanol_pfd.png differ diff --git a/docs/gallery/10_ethanol_pfd.svg b/docs/gallery/10_ethanol_pfd.svg index c0d1f1b..e0b82d6 100644 --- a/docs/gallery/10_ethanol_pfd.svg +++ b/docs/gallery/10_ethanol_pfd.svg @@ -503,7 +503,7 @@ - + Fermentation Broth @@ -547,7 +547,7 @@ - + @@ -564,8 +564,8 @@ S-306 S-307 - - S-308 + + S-308 S-309 diff --git a/docs/gallery/18_fixed_bed_recycle.png b/docs/gallery/18_fixed_bed_recycle.png index d106195..8ed7237 100644 Binary files a/docs/gallery/18_fixed_bed_recycle.png and b/docs/gallery/18_fixed_bed_recycle.png differ diff --git a/docs/gallery/18_fixed_bed_recycle.svg b/docs/gallery/18_fixed_bed_recycle.svg index 8989f59..1da68c5 100644 --- a/docs/gallery/18_fixed_bed_recycle.svg +++ b/docs/gallery/18_fixed_bed_recycle.svg @@ -239,7 +239,7 @@ - + @@ -347,8 +347,8 @@ 350-LG-314-CS 350-LG-304-LT - - 350-LG-305-LT + + 350-LG-305-LT 350-LG-306-CS diff --git a/pandid/layout/__init__.py b/pandid/layout/__init__.py index 040667f..65022a3 100644 --- a/pandid/layout/__init__.py +++ b/pandid/layout/__init__.py @@ -1,44 +1,23 @@ -"""Layout Engine orchestrator. - -The layout engine computes geometry (each unit's Frame) from topology, -and it draws the sheet in the order a draughtsman does: the process -first, then the instrumentation onto it. - -**Stage 1, process.** Every unit that carries material and every stream -of kind ``"material"``. - -- Cycle breaking, so a return line is known to be one. -- Placement (:mod:`pandid.layout.place`): two weighted least-squares - fits, one per axis, over what the equipment says about where its - neighbours are drawn (:mod:`pandid.layout.claims`), solved in closed - form by :mod:`pandid.layout.solver`. A column says its condenser is - north east of it and its reboiler south east; every stream states two - such claims, one from each end, and the fit is the compromise between - all of them weighted by how hard each unit insists. -- Coordinates (:mod:`pandid.layout.coordinates`): grid to pixels, folded - into bands where the ribbon is wider than paper, and with the space - the instrumentation will need already reserved +"""Compute each unit's frame from the flowsheet topology. + +**Stage 1, process.** Units that carry material and streams of kind +``"material"``: + +- cycle breaking, which identifies return lines; +- placement (:mod:`pandid.layout.place`): one weighted least-squares fit per + axis over the neighbour positions each unit claims + (:mod:`pandid.layout.claims`), solved by :mod:`pandid.layout.solver`; +- coordinates (:mod:`pandid.layout.coordinates`): grid to pixels, folded + into bands, with instrumentation space reserved (:mod:`pandid.layout.halo`). -**Stage 2, control.** Every instrument, attached and free-standing, and -every signal run, placed against stage 1's frozen geometry -(:mod:`pandid.layout.control`). - -Two phases follow, both of which need every drawn box to be final: -port-face selection, then label placement. Their order is load-bearing: -a label goes to a face no connected nozzle occupies, so it has to be -told which faces those are. - -Face selection and stage 2 do not settle in a single pass, and they are -run to a fixed point rather than in an order. A balloon hung on a -*stream* lands on that stream's drawn path, and where the path leaves -each end is the face selection's answer -- while the selection reads the -boxes, of which the balloon is one. Neither can go first: run once, the -first ``layout()`` placed such a balloon from the faces a symbol defaults -to and the second placed it from the faces the first chose, so laying a -sheet out twice did not draw it twice the same. -``examples/04_control_loop.py``'s interlock, hung on the signal between -two balloons, moved 16px on the second run. +**Stage 2, control.** Instruments and signal runs, placed against the +stage 1 geometry (:mod:`pandid.layout.control`). + +Port-face selection and control placement then repeat until neither moves +the other, because an attached balloon sits on a drawn path that depends on +the selected faces. Labels are placed last, on faces no connected nozzle +uses. """ from typing import Literal, Protocol, TYPE_CHECKING @@ -48,16 +27,38 @@ class LayoutEngine(Protocol): + """Define the interface ``Flowsheet.layout`` requires of an engine.""" + def layout(self, fs: "Flowsheet") -> None: - """Layout the flowsheet by computing a Frame for each unit.""" + """Compute a frame for each unit. + + Parameters + ---------- + fs : Flowsheet + Sheet to lay out. + + Returns + ------- + None + Frames are stored on the units. + """ def _seed_slots(fs: "Flowsheet") -> None: - """Seed each unit's solver ``_Slot`` from its ``Pin`` intent. + """Seed each unit's solver slot from its pin. + + Every run starts from author intent, never from earlier coordinates, + so layout is idempotent. + + Parameters + ---------- + fs : Flowsheet + Sheet whose units receive fresh slots. - Reseeding from ``pin_`` on every run is what makes layout - idempotent: the solver never reads back a previous run's - coordinates, only the user's intent. + Returns + ------- + None + ``_slot`` is replaced on every unit. """ from pandid.geometry import _Slot from pandid.portgeom import resolve_size @@ -78,7 +79,7 @@ def _seed_slots(fs: "Flowsheet") -> None: class ConstraintLayoutEngine: - """The default auto-layout engine. + """Place units with the default automatic layout. Equipment and stream-relative stations are placed in separate passes when a station can be contracted. Other sheets use the full unit graph. @@ -87,7 +88,8 @@ class ConstraintLayoutEngine: def layout(self, fs: "Flowsheet", *, use_coarse: bool = True, reservation: Literal["conservative", "compact", "shared"] = "shared", row_compaction: float = 0.0, - directional_station: bool = False) -> None: + directional_station: bool = False, + unaligned: frozenset[int] = frozenset()) -> None: """Resolve process, inline, and control geometry for a sheet. Parameters @@ -102,6 +104,9 @@ def layout(self, fs: "Flowsheet", *, use_coarse: bool = True, Fraction of independent column-row compaction to apply. directional_station : bool, optional Follow explicitly mirrored station flow in a detached trial. + unaligned : frozenset[int], optional + Global indices of units a detached trial keeps on their row + axis. Equipment-first placement ignores it. Returns ------- @@ -132,17 +137,13 @@ def layout(self, fs: "Flowsheet", *, use_coarse: bool = True, if not fs._coarse_layout_candidate: _seed_slots(fs) assign_positions(fs) - assign_coordinates(fs, row_compaction=row_compaction) + assign_coordinates(fs, row_compaction=row_compaction, unaligned=unaligned) place_stations(fs) place_inline(fs) elif not inline_coarse: place_inline(fs) - # Choose the faces, and place again where that moved a balloon. - # The loop ends on a selection made against boxes nothing has - # moved since, so the sheet it hands on is a function of the - # model and not of what the last run left behind. Every sheet in - # the corpus settles in one pass and 04 in two; the cap is - # ``route()``'s own, for the same reason it has one. + # Repeat face selection and control placement until no balloon moves, + # up to the pass limit. for _ in range(MAX_PLACEMENT_PASSES): select_faces(fs) if not place_control(fs): diff --git a/pandid/layout/coordinates.py b/pandid/layout/coordinates.py index 9372dc3..3a05f5f 100644 --- a/pandid/layout/coordinates.py +++ b/pandid/layout/coordinates.py @@ -1,23 +1,12 @@ -"""Stage 1 geometry: grid positions to pixels, band by band. - -A unit's position is ``(band, column, row)``. The column and the row -come from :mod:`pandid.layout.place`; the band is decided here, because -it is the one part of the position that is about *paper* rather than -about the process, and paper is the first thing this module knows about. - -Why the band cannot be applied afterwards ------------------------------------------ -Cutting a solved ribbon at ``x > W`` and dropping the tail underneath -looks like the cheap way to wrap, and it is wrong by construction: a -stream crossing the cut now runs right to left, so the unit whose *west* -nozzle carries it sits east of its peer -- reintroducing exactly the -inconsistency between geometry and nozzle that the constraint solver -exists to remove. So the band is part of the position and the ribbon is -cut where the drawing has a seam, not where the ruler falls. - -:func:`assign_labels` closes the run but is a separate phase the engine -calls after :mod:`pandid.layout.faces` has chosen the movable ports' -faces, since a label dodges the faces the nozzles actually leave from. +"""Convert process grid positions to pixels, band by band. + +A unit's position is ``(band, column, row)``. Columns and rows come from +:mod:`pandid.layout.place`; bands are chosen here from the paper width. A +band boundary is part of the position: cutting a solved ribbon afterwards +would make streams across the cut run against their nozzle faces. + +:func:`assign_labels` runs as a separate phase after +:mod:`pandid.layout.faces`, because a label avoids the faces nozzles use. """ from __future__ import annotations @@ -36,46 +25,23 @@ from pandid.ports import Port from pandid.units import Unit -#: Clear paper between one column of boxes and the next, which is where -#: the run between them is drawn -- **and where its line number is -#: written**. ISO 15519-1 §7.2.5 puts that number along or beside its -#: own line and sends it away with a leader only where there is no room -#: beside it, so the gap has to be wide enough to be that room: the -#: longest number in the corpus is seventeen characters, a little under -#: 100 px of lettering, and 100 left it nothing either side. At 120 it -#: fits with a margin, and ``350-LG-314-CS`` on 18_fixed_bed_recycle -#: stops being written a lane away from its own run with a leader drawn -#: back across the loop gas line. -#: -#: It belongs with the placement change rather than in a tidying pass of -#: its own, and the reason is that the placement change is what needs -#: it. At 100 ``tests/test_label_invariants.py`` fails twice on this -#: branch and passes on ``main``: the fit puts two boxes a column apart -#: that the engine before it did not, and the run between them is now a -#: run with a thirteen-character number and 100 px to write it in. +#: Clear width between adjacent columns. It holds the connecting run and its +#: line number (ISO 15519-1 §7.2.5); the longest corpus number needs just +#: under 100 px. COL_GAP = 120.0 -ROW_GAP = 70 # gap between row bands, over the taller row +#: Clear height between adjacent rows, measured from the taller row. +ROW_GAP = 70 +#: Left and top sheet margins. MARGIN_X = 50 MARGIN_Y = 50 -#: Clear paper between one band of the ribbon and the one below it. Wide -#: enough that a run turning down at the end of a band and back along -#: the next has a lane of its own to turn in, and that a reader sees two -#: bands rather than one crowded sheet. +#: Clear height between adjacent bands, wide enough for a lane in which a +#: folded run turns. BAND_GAP = 160.0 -#: How wide a band may get before the ribbon is folded. Chosen as paper: -#: an A1 sheet is 841 mm across, which at the 96 dpi this library draws -#: in is a little over 3170 px, and a drawing wider than the largest -#: sheet anyone hangs on a wall is a drawing nobody reads. Below it a -#: sheet is left exactly as it was -- 16_demineralised_water is a -#: 2436 px water train with an aspect of 6.5 and folding it would be -#: rearranging a drawing that is already right. -#: -#: Deliberately a width and not a target aspect. Aspect cannot tell a -#: long *small* sheet from a long big one, and folding the small one is -#: how a fix for #429 would have broken the sheets it was told not to -#: touch. +#: Maximum band width before the ribbon folds: about the width of an A1 +#: sheet at 96 dpi. It is a width and not an aspect ratio, so a long, small +#: sheet is not folded (#429). BAND_WIDTH = 3200.0 @@ -83,7 +49,8 @@ def assign_coordinates(fs: "Flowsheet", *, units: list["Unit"] | None = None, extra_gap: dict[int, float] | None = None, links: list[tuple["Unit", "Unit", float]] | None = None, hosts: list["Host"] | None = None, - row_compaction: float = 0.0) -> None: + row_compaction: float = 0.0, + unaligned: frozenset[int] = frozenset()) -> None: """Map selected process-unit grid ranks to pixels. Parameters @@ -100,6 +67,8 @@ def assign_coordinates(fs: "Flowsheet", *, units: list["Unit"] | None = None, Contracted runs whose endpoint nozzles guide pixel alignment. row_compaction : float, optional Fraction of independent column-row compaction to apply. + unaligned : frozenset[int], optional + Global indices of units kept on their row axis. Returns ------- @@ -136,7 +105,7 @@ def assign_coordinates(fs: "Flowsheet", *, units: list["Unit"] | None = None, reference = {u: (s.x or 0.0, s.y or 0.0) for u, s in nominal.items()} moved = refine(fs, units, reference, links) - _straighten(fs, units, band_of, pads, hosts) + _straighten(fs, units, band_of, pads, hosts, unaligned) if moved: clear_pins(units, moved, STACK_CLEAR, pads) for u in units: @@ -153,34 +122,60 @@ def assign_coordinates(fs: "Flowsheet", *, units: list["Unit"] | None = None, class _Column: - """One column of the grid, and the paper it claims.""" + """Hold the units of one grid column and the width they need. + + Attributes + ---------- + units : list[Unit] + Units placed in the column. + lead : float + Clearance attached objects need on the west. + body : float + Width of the widest unit. + tail : float + Clearance attached objects need on the east. + """ def __init__(self) -> None: + """Create an empty column.""" self.units: list["Unit"] = [] - self.lead = 0.0 # clear paper the balloons on its west want - self.body = 0.0 # the widest box in it - self.tail = 0.0 # clear paper the balloons on its east want + self.lead = 0.0 + self.body = 0.0 + self.tail = 0.0 @property def span(self) -> float: + """Return the total width the column needs. + + Returns + ------- + float + Sum of the west clearance, body width, and east clearance. + """ return self.lead + self.body + self.tail -#: How far a boundary flag's nozzle stands inside its own frame origin. -#: The flag grows *west* from there as its label does, so a wide label -#: is drawn outside the box the column laid out for it. See -#: :func:`~pandid.portgeom.unit_box`, which is where the convention is. +#: Distance from a boundary flag's frame origin to its nozzle. The flag +#: extends west from the nozzle; see :func:`~pandid.portgeom.unit_box`. _FLAG_LEAD = 50.0 def _west(u: "Unit", pads: dict["Unit", Pad]) -> float: - """Clear paper this unit needs on its west, its own box included. + """Return the clearance a unit needs on its west side. + + A boundary flag extends west of its frame origin as its label grows. - A boundary flag is the one unit drawn outside the box the grid laid - out for it: its nozzle sits a fixed lead inside its own origin and - the pennant grows *west* from there as the label does (see - :func:`~pandid.portgeom.unit_box`), so a long tag reaches back into - whatever the column before it holds. + Parameters + ---------- + u : Unit + Unit to measure. + pads : dict[Unit, Pad] + Attached-object clearance around each unit. + + Returns + ------- + float + Clearance in pixels, including a flag's westward body. """ west = pads.get(u, Pad()).west if u.kind == "feed" and not slot(u).mirrored: @@ -189,7 +184,20 @@ def _west(u: "Unit", pads: dict["Unit", Pad]) -> float: def _columns(units: list["Unit"], pads: dict["Unit", Pad]) -> dict[int, _Column]: - """Every column the sheet uses, with the width it needs.""" + """Group units into grid columns and measure each column. + + Parameters + ---------- + units : list[Unit] + Process units with seeded grid columns. + pads : dict[Unit, Pad] + Attached-object clearance around each unit. + + Returns + ------- + dict[int, _Column] + Columns keyed by grid column number. + """ out: dict[int, _Column] = defaultdict(_Column) for u in units: column = out[slot(u).col or 0] @@ -201,13 +209,22 @@ def _columns(units: list["Unit"], pads: dict["Unit", Pad]) -> dict[int, _Column] def _wrappable(fs: "Flowsheet", units: list["Unit"]) -> bool: - """May this sheet be folded at all? + """Return whether the sheet may be folded into bands. + + A sheet with an absolute pin is not folded; a band could land on the + pinned unit. + + Parameters + ---------- + fs : Flowsheet + Sheet being laid out. + units : list[Unit] + Process units to inspect. - Not where anything is pinned to an absolute coordinate. A band is a - statement about where the grid goes, and a unit placed at ``x=420`` - is not on the grid: folding around it would drop a band on top of - equipment the author put somewhere on purpose. An author placing by - hand has already decided the shape of the sheet. + Returns + ------- + bool + Whether no unit is pinned to an absolute x or y. """ return not any(u.pin_ is not None and (u.pin_.x is not None or u.pin_.y is not None) for u in units) @@ -270,7 +287,18 @@ def _bands(units: list["Unit"], columns: dict[int, _Column], def _seam_cost(units: list["Unit"]) -> dict[int, int]: - """How many runs cross the gap after each column.""" + """Count the connections that cross the gap after each column. + + Parameters + ---------- + units : list[Unit] + Process units with seeded grid columns. + + Returns + ------- + dict[int, int] + Crossing count keyed by the column before the gap. + """ where = {u: (slot(u).col or 0) for u in units} cost: dict[int, int] = defaultdict(int) for u in units: @@ -291,9 +319,7 @@ def _slide(band: list[int], crossings: dict[int, int], protected: set[int] | None = None) -> list[int]: """Pull a band's last column back to the quietest seam near it. - At most a quarter of the band, so a seam is looked for where one - plausibly is and the fold never walks back to the start of a band it - has just filled. + The search covers at most the last quarter of the band. Parameters ---------- @@ -348,10 +374,8 @@ def _lay_columns(columns: dict[int, _Column], band: list[int], float Width of the band in pixels. """ - # A row with nothing to its west yet reserves nothing: the first - # column starts on the margin, and a boundary flag whose pennant - # reaches back past its own origin reaches into the margin rather - # than pushing the whole sheet right. + # Start the first column on the margin; a boundary flag there extends + # into the margin. position = slot if positions is None else positions.__getitem__ wall: dict[int, float] = {} cursor = float(MARGIN_X) @@ -413,21 +437,16 @@ def _lay_band(columns: dict[int, _Column], band: list[int], top: float, _lay_columns(columns, band, pads, place=True, positions=positions, extra_gap=extra_gap) - # Bands are built for every row the sheet names between the band's - # own first and last, and a pin can name one above row 0: - # ``pin(row=-1)`` is the band over it, which is where a header - # belongs. An empty row keeps a default height, so a run has a lane. + # Build every row between the band's first and last, including pinned + # rows above row 0. An empty row keeps a default height for a lane. banded = [position(u).row or 0 for u in members if position(u).y is None] if not banded: return top - # Row 0 anchors the top margin of the *first* band where nothing - # goes above it, so ``pin(row=2)`` keeps the two empty bands it - # asked for. A later band counts from its own first row instead: - # the rows are global, and building every band from zero would - # leave each one preceded by every band above it, empty. + # Anchor the first band at row 0 so ``pin(row=2)`` keeps two empty rows + # above it. Later bands start from their own first row. floor_row = min([*banded, 0]) if anchored else min(banded) rows = list(range(floor_row, max(banded) + 1)) - body = dict.fromkeys(rows, 50.0) # the tallest box in the row + body = dict.fromkeys(rows, 50.0) # Height of the tallest unit in each row. holds: dict[int, list["Unit"]] = {r: [] for r in rows} for u in members: if position(u).y is None: @@ -435,14 +454,8 @@ def _lay_band(columns: dict[int, _Column], band: list[int], top: float, holds[row].append(u) body[row] = max(body[row], position(u).h) - # The balloon demand is settled **per column**, not per row. A chain - # of bubbles standing 200 units over an orifice plate in column 3 - # wants 200 units of paper over *column 3*; charging it to the whole - # row charges it once per column the row has, and on a sheet with a - # dozen instrumented runs that is a sheet four times too tall. - # ``floor`` is how far down each column is already spoken for, which - # is what lets a chain reach up through a band its own column has - # nothing in. + # Reserve attached-object clearance per column, not per row. ``floor`` + # records how far down each column is already occupied. floor: dict[int, float] = {} axis: dict[int, float] = {} cursor_y = top @@ -518,10 +531,7 @@ def _compact_column_rows(columns: dict[int, _Column], band: list[int], top: floa # Straightening # --------------------------------------------------------------------------- -#: How far short of the nozzle it drops onto a sideways-facing one is -#: aimed. The router stands a run off a nozzle before it may turn, so -#: aiming dead on costs a turn out and a turn back; this is that -#: stand-off, spent going the way the run was already going. +#: Router stand-off reserved when a unit is aligned with a vertical nozzle. STACK_LEAD = 25.0 @@ -555,34 +565,15 @@ def _target_y(other_u: Unit, other_port: Port, contracted: bool) -> float: def _straighten(fs: "Flowsheet", units: list["Unit"], band_of: dict["Unit", int], - pads: dict["Unit", Pad], hosts: list["Host"] | None = None) -> None: - """Turn staircase jogs into straight runs, within one band. - - Walk units left to right and, where a unit has a single horizontal - process connection to pull it, shift it vertically so the two ports - share an absolute height. Only a peer strictly to the left counts as - upstream; everything else buckets downstream, a peer in this unit's - own column included, so a same-column peer can be the lone anchor. - Nothing rules that out ahead of time: the overlap check is what - settles it, and two boxes asked to share a height in one column will - fail it unless the target is an N/S escape lane clear of both. - - A peer in another band is no anchor at all. Aiming at it would drag - a unit out of its own band and into the gap between two, which is - the lane the folded runs are drawn in. - - A unit a nozzle stacks something over does not move on its own: the - whole stack goes with it. The rows already say a relief valve is - above the vessel it protects; straightening the vessel onto its own - spine and leaving the valve on the row axis would say it in the row - numbers and deny it in the ink, which is exactly the disagreement - between geometry and nozzle this engine exists to end. - - The slot carries the same box the Frame will (size and transform), - so the port resolver answers here exactly as it will once the frames - are emitted. That is the point: a target read off the symbol instead - ignores the resize, the mirror and any ``nozzle()`` choice, and aims - at the wrong height. + pads: dict["Unit", Pad], hosts: list["Host"] | None = None, + unaligned: frozenset[int] = frozenset()) -> None: + """Shift units vertically so horizontal runs become straight. + + Units are visited left to right. A unit with exactly one upstream peer, + or no upstream peer and exactly one downstream peer, is shifted so its + sideways port shares the peer's height. A peer in another band is + ignored. A stack moves as one group. A shift is rejected when it breaks + grid-row order or overlaps a neighbour's padded box. Parameters ---------- @@ -596,6 +587,8 @@ def _straighten(fs: "Flowsheet", units: list["Unit"], band_of: dict["Unit", int] Reserved clearance around equipment. hosts : list[Host] or None Contracted material runs between retained equipment. + unaligned : frozenset[int], optional + Global indices of units kept on their row axis. Returns ------- @@ -607,10 +600,7 @@ def _straighten(fs: "Flowsheet", units: list["Unit"], band_of: dict["Unit", int] from pandid.layout.stages import process_streams from pandid.portgeom import resolve_port - # Both questions below are asked of a *neighbourhood* -- the runs on - # one unit, and the boxes in one column -- and both used to be - # answered by reading the whole sheet: a pass over every stream and - # a pass over every unit, per unit. Indexed once here instead. + # Index units by column and connections by unit once. by_col: dict[int | None, list["Unit"]] = defaultdict(list) for u in units: by_col[slot(u).col].append(u) @@ -635,13 +625,23 @@ def _straighten(fs: "Flowsheet", units: list["Unit"], band_of: dict["Unit", int] pixel_pins = not _wrappable(fs, units) def overlaps(u: "Unit", new_y: float, moving: set["Unit"]) -> bool: - """Would ``u`` at ``new_y`` land on a neighbour, or on its halo? - - Measured on the *padded* box, so the paper a unit reserved for - the balloons hanging off it is paper the straightener cannot - spend either. Reserving it in the band layout and then handing - it out here is how a bubble comes to be drawn over the boundary - flag beside its own orifice plate. + """Return whether a vertical shift is illegal. + + Boxes include the clearance reserved for attached objects. + + Parameters + ---------- + u : Unit + Unit to move. + new_y : float + Proposed top coordinate. + moving : set[Unit] + Units shifting together, which do not block each other. + + Returns + ------- + bool + Whether the shift breaks grid-row order or overlaps a neighbour. """ lower, upper = grid_limits(u, "y", boxes, moving=moving) if not lower <= new_y <= upper: @@ -664,6 +664,7 @@ def overlaps(u: "Unit", new_y: float, moving: set["Unit"]) -> bool: for u in units: stacked[stack_of[u]].append(u) + held = {fs.units[index] for index in unaligned} settled: set["Unit"] = set() for u in sorted(units, key=lambda v: (slot(v).col or 0, slot(v).y or 0.0)): s = slot(u) @@ -672,22 +673,22 @@ def overlaps(u: "Unit", new_y: float, moving: set["Unit"]) -> bool: group = stacked[stack_of[u]] if any(v.pin_ is not None and v.pin_.y is not None for v in group): continue + if held.intersection(group): + continue ups: list[tuple] = [] downs: list[tuple] = [] for pair in touching[u]: if band_of.get(pair[1]) != band_of.get(u) or pair[1] in group: continue (ups if (pair[1]._slot.col or 0) < (s.col or 0) else downs).append(pair) - # A single upstream anchor chains the spine; fall back to a - # single downstream one so terminals (Feed) still align. + # Use a single upstream anchor, else a single downstream one. anchor = ups[0] if len(ups) == 1 else (downs[0] if not ups and len(downs) == 1 else None) if anchor is None: continue my_port, other_u, other_port, contracted = anchor if slot(other_u).y is None: continue - # Only straighten horizontal runs: the port must face the - # neighbour sideways (E/W); a vertical port keeps the row axis. + # Straighten only a port that faces east or west. (_, my_y), _, my_d = resolve_port(u, s, my_port.name) if my_d not in ("E", "W"): continue @@ -762,20 +763,28 @@ def _stack_offsets(fs: "Flowsheet", units: list["Unit"], return out -#: Clear paper a sideways nudge has to leave between the box it moves -#: and the one beside it. Not a collision margin: a run between two -#: boxes has to be *drawn*, and its number written along it, and a -#: number is a couple of dozen pixels of lettering before it is -#: anything else. Slid until it merely fails to overlap, an ejector -#: lining up with the vent above it left 17 px between itself and the -#: splitter feeding it -- a run too short to write ``S7`` beside, so the -#: number went off looking for paper and had to be drawn back to its own -#: line across the splitter (15_condensing_turbine). +#: Clearance a sideways nudge leaves beside the moved unit, enough to draw +#: a run and write its number. STACK_CLEAR = 40.0 def _overlaps_x(u: "Unit", new_x: float, units: list["Unit"]) -> bool: - """Would moving ``u`` to ``new_x`` crowd a unit beside it?""" + """Return whether a horizontal move crowds a neighbouring unit. + + Parameters + ---------- + u : Unit + Unit to move. + new_x : float + Proposed left coordinate. + units : list[Unit] + Units that may be beside it. + + Returns + ------- + bool + Whether a vertically overlapping unit lies within ``STACK_CLEAR``. + """ s = slot(u) if s.y is None: return True @@ -795,20 +804,22 @@ def _overlaps_x(u: "Unit", new_x: float, units: list["Unit"]) -> bool: # --------------------------------------------------------------------------- _DIR_OF_SIDE = {"top": "N", "bottom": "S", "left": "W", "right": "E"} -#: Label sides in the order a sheet prefers them, best first. +#: Label sides in order of preference. LABEL_SIDES = ("top", "bottom", "right", "left") def free_label_sides(u) -> list[str]: - """The sides of a box no connected nozzle leaves, best first. - - Read off :mod:`pandid.portgeom`, so the faces this reports free are - the faces the router and the renderer will actually see empty. The - renderer asks the same question again once the sheet is routed, - because a face free of nozzles can still have a passing line or an - impulse line across it, and neither of those exists yet while layout - runs. This is the half of the answer that does: a nozzle is - geometry, and geometry is settled here. + """Return the sides of a unit that no connected nozzle uses. + + Parameters + ---------- + u : Unit + Placed unit. + + Returns + ------- + list[str] + Free sides in order of preference; empty for an unplaced unit. """ from pandid.portgeom import port_anchor @@ -824,18 +835,26 @@ def free_label_sides(u) -> list[str]: def assign_labels(fs: "Flowsheet") -> None: - """Resolve each label side, avoiding faces a live port holds. + """Choose a label side for every placed unit. - Explicit user ``label_pos`` or a symbol default wins; otherwise the - label goes to the first free face in top → bottom → right → left - order, so a stream leaving (say) a pump's top nozzle does not run - through its label. + An explicit ``label_pos`` wins, then the symbol's default, then the + first side free of connected nozzles. + + Parameters + ---------- + fs : Flowsheet + Sheet with placed frames and selected port faces. + + Returns + ------- + None + ``label_pos`` is stored on each frame. """ from pandid.render.symbols import default_registry for u in fs.units: if u.kind in ("feed", "product") or u.frame is None: - continue # labels are drawn inline on the arrow + continue # A boundary flag draws its own label. explicit = getattr(u, "label_pos", None) if explicit: u.frame.label_pos = explicit diff --git a/pandid/layout/faces.py b/pandid/layout/faces.py index 6465941..03810db 100644 --- a/pandid/layout/faces.py +++ b/pandid/layout/faces.py @@ -1,25 +1,13 @@ -"""Port-face selection: which declared placement gets the stream. - -Policy only. Every coordinate here is asked for from -:mod:`pandid.portgeom`, which stays the single authority on where a port -is; this module names a candidate face and reads back where that puts -the ink. *portgeom answers where; this answers which.* - -Where this runs ---------------- -Selection is a layout phase, after coordinate assignment has settled -every drawn box and before anything reads a face. That is the only cut -with no cycle in it: a face can only be judged against where its peer's -box ended up, and the passes that place those boxes (ranking, the spine -straightener) resolve ports themselves, so a pick they could see would -feed back into the geometry it was derived from. Downstream, labels, -routing and rendering all read -:func:`~pandid.portgeom.resolve_port` and so see the same answer. - -The pick is written to the resolved :class:`~pandid.geometry.Frame`. The -solver's ``_Slot`` has no such field, which makes "the box passes cannot -read a pick" structural rather than a convention, and every layout run -emits fresh frames, so laying a sheet out twice draws it the same way. +"""Select which declared placement each movable port is drawn on. + +This module states a policy only. Coordinates come from +:mod:`pandid.portgeom`. + +Selection runs after coordinate assignment has placed every box, because a +face is judged against where the peer landed, and before labels, routing, +and rendering read a face. The choice is stored on the resolved +:class:`~pandid.geometry.Frame`, which each layout run replaces, so placement +passes never read a previous choice. """ from __future__ import annotations @@ -31,11 +19,8 @@ from pandid.ports import Port from pandid.units import Unit -# How far the router stands a run off a nozzle before it may turn. A -# face pointing away from its peer pays that twice: out to the -# stand-off, then back past the nozzle before the run has made any -# ground. That is what separates a detour from a merely longer straight -# run. +#: Router stand-off from a nozzle. A face pointing away from its peer costs +#: twice this distance. _ESCAPE = 25.0 @@ -89,55 +74,25 @@ def select_faces( None Automatic selections are stored on each resolved frame. """ + from pandid.layout.stages import is_control + for index, unit in enumerate(fs.units): frame = unit.frame if frame is None: continue - # Start from nothing. An attached balloon's frame carries its - # picks across a re-place, so without this the phase would read - # its own last answer back in and stop being a function of the - # sheet alone. + # Clear earlier choices so selection depends on the sheet alone. frame.port_faces.clear() if not fs.auto_faces: continue - # A nozzle the author put on a coordinate. Its face is settled - # by that, not chosen here: ``pin(port="in_1", y=440)`` is - # honoured by deriving a corner from where the nozzle sits on - # the box (:attr:`pandid.units.Unit.pin_`), and the box is - # already placed by the time this phase runs -- so a pick made - # here would move the nozzle off the coordinate the solver was - # seeded from, with nothing downstream to re-derive the corner. - # A tank pinned by its inlet was drawn with the run entering its - # roof instead of the pinned point on its wall (#294). - # - # The constraint goes this way round and not the other because - # the cut above it is load-bearing: a face is only judgeable - # against where its peer's box landed, so selection cannot run - # before placement, so a pin cannot be re-derived from a pick. - # A pin is the author's boundary condition and a face is the - # engine's preference, and it is the preference that yields. - # :meth:`~pandid.units.Unit.nozzle` is how a face is *chosen* - # for a pinned nozzle, and it wins here by the same exemption. + # ``eligible_faces`` excludes pinned and author-chosen nozzles: a pin + # fixes the box from the nozzle position, so a later choice would move + # the nozzle off its pin (#294). live = [name for name, port in unit.ports.items() if port.stream is not None] menus = {name: menu for name in live if (menu := eligible_faces(fs, unit, name))} - # Points already spoken for: a nozzle the author named, one - # fixed by physics. Two live connections resolving to one point - # is a hard validation error, and the selector must not be the - # thing that makes one. Ports are served in declaration order, - # so a sheet does not depend on which of them the author - # happened to connect first. - # - # A boundary flag carrying several runs is the one place two - # live connections *do* resolve to one point, deliberately - # (:attr:`pandid.units.Unit.ONE_NOZZLE_MANY_RUNS`), and nothing - # here has to change for it: a flag's connection has a single - # placement, so no member of it is ever ``movable``, and they - # all land in ``taken`` -- which is a set, and already holds one - # entry for one point. The rule this comment states is still the - # rule and the selector still cannot break it; what the flag - # changes is only that ``validate`` no longer *reports* the - # coincidence, and it reports it for everything else. + # Record the points of fixed ports. Ports are served in declaration + # order and no choice may land on a taken point; two connections on + # one point is a validation error. taken = {_point(unit, frame, name) for name in live if name not in menus} for name, menu in menus.items(): target = _reference(unit.ports[name]) @@ -146,30 +101,131 @@ def select_faces( preferred_face = None if preferred is None else preferred.get((index, name)) face = (_best(unit, frame, name, [preferred_face], target, taken) if preferred_face in menu else None) + if face is None and not is_control(unit): + usable = _usable_faces(fs, unit, frame, name, list(menu)) + face = _best(unit, frame, name, usable, target, taken) if face is None: face = _best(unit, frame, name, list(menu), target, taken) if face is None: - continue # all faces taken; leave the symbol's + continue # Every face is taken; keep the symbol's placement. frame.port_faces[name] = face taken.add(_point(unit, frame, name)) +def _usable_faces(fs: "Flowsheet", unit: "Unit", frame, port_name: str, + menu: list[str]) -> list[str]: + """Drop faces that are blocked or need more bends than the home face. + + A face is blocked when its outward stub crosses another unit. When the + peer's nozzle is fixed, an alternate face is also dropped if it needs + more bends than the first face of the menu. The menu is returned + unchanged when every face is blocked. + + Parameters + ---------- + fs : Flowsheet + Placed drawing. + unit : Unit + Owner of the port. + frame : Frame + Resolved frame of the unit. + port_name : str + Connected port with more than one declared face. + menu : list[str] + Declared faces in symbol preference order. + + Returns + ------- + list[str] + Usable faces in the same order. + """ + from pandid.layout.attach import is_attached + from pandid.layout.conflicts import segment_crosses_box + from pandid.portgeom import face_point, port_faces, resolve_port, unit_box + from pandid.routing.metrics import min_bends + + port = unit.ports[port_name] + stream = port.stream + assert stream is not None + peer = stream.dest if stream.source is port else stream.source + owner = peer.owner + boxes = [ + unit_box(other, other.frame) + for other in fs.units + if other is not unit and other is not owner and other.frame is not None + and not is_attached(other) + ] + fixed_peer = ( + resolve_port(owner, owner.frame, peer.name) + if owner is not None and owner.frame is not None + and len(port_faces(owner, peer.name, owner.frame)) == 1 + else None + ) + clear: list[str] = [] + bends: dict[str, int | None] = {} + for face in menu: + frame.port_faces[port_name] = face + _, anchor, landed = resolve_port(unit, frame, port_name) + normal = face_point(unit, frame, landed)[1] + tip = (anchor[0] + _ESCAPE * normal[0], anchor[1] + _ESCAPE * normal[1]) + if not any(segment_crosses_box(anchor, tip, box) for box in boxes): + clear.append(face) + bends[face] = ( + None if fixed_peer is None + else min_bends(anchor, landed, fixed_peer.anchor, fixed_peer.face) + ) + frame.port_faces.pop(port_name, None) + if not clear: + return menu + home = bends[clear[0]] if clear[0] == menu[0] else None + if home is None: + return clear + usable: list[str] = [] + for face in clear: + needed = bends[face] + if face == menu[0] or (needed is not None and needed <= home): + usable.append(face) + return usable + + def _point(unit: "Unit", frame, port_name: str) -> tuple[float, ...]: - """Where a nozzle lands, rounded as the coincidence check is.""" + """Return a port's drawn point, rounded for coincidence checks. + + Parameters + ---------- + unit : Unit + Owner of the port. + frame : Frame + Resolved frame of the unit. + port_name : str + Port to resolve. + + Returns + ------- + tuple[float, ...] + Point coordinates rounded to three decimals. + """ from pandid.portgeom import resolve_port return tuple(round(v, 3) for v in resolve_port(unit, frame, port_name).point) def _reference(port: "Port") -> tuple[float, float] | None: - """The point a candidate face for ``port`` is scored against. - - None when there is nothing placed to aim at. The peer's nozzle is - the target where it has one placement and so cannot move; where it - can move it has no settled position yet (reading a provisional one - would make each port's answer depend on the order the ports happened - to be visited), so the centre of its unit's drawn box stands in for - it, which is the same point whichever face it ends up on. + """Return the point a candidate face is scored against. + + A peer with one declared face gives its nozzle anchor. A peer with + several gives the centre of its unit, which does not depend on the face + it later takes. + + Parameters + ---------- + port : Port + Connected port being placed. + + Returns + ------- + tuple[float, float] or None + Target point, or ``None`` when the peer is unplaced. """ from pandid.portgeom import port_faces, resolve_port, unit_box @@ -188,12 +244,30 @@ def _reference(port: "Port") -> tuple[float, float] | None: def _best(unit: "Unit", frame, port_name: str, menu: list[str], target: tuple[float, float], taken: set[tuple[float, ...]]) -> str | None: - """The cheapest free face in ``menu`` for a run to ``target``. + """Return the cheapest untaken face for a run to the target. + + Ties go to the face aimed most directly at the peer, then to the + symbol's preference order. + + Parameters + ---------- + unit : Unit + Owner of the port. + frame : Frame + Resolved frame of the unit. + port_name : str + Port being placed. + menu : list[str] + Candidate faces in symbol preference order. + target : tuple[float, float] + Point the run must reach. + taken : set[tuple[float, ...]] + Points already used by other connected ports. - Scoring a candidate by resolving it is what keeps the geometry in - one place: the selector states a face and reads back the anchor and - normal the router and renderer will use, rather than deriving a - second set of its own. + Returns + ------- + str or None + Chosen face, or ``None`` when every candidate point is taken. """ from pandid.portgeom import face_point, resolve_port, unit_box @@ -206,12 +280,7 @@ def _best(unit: "Unit", frame, port_name: str, menu: list[str], if tuple(round(v, 3) for v in point) in taken: continue normal = face_point(unit, frame, landed)[1] - # Ties are common and expected: a balloon is square, so stepping - # a signal round to the next face trades exactly as much - # horizontal run for vertical. Break them towards the face - # pointing most directly at the peer, then on the symbol's own - # order of preference, so the answer never turns on how the menu - # was iterated. + # Order by cost, then aim, then symbol preference. scored.append((_cost(anchor, normal, target), -_aim(centre, normal, target), rank, face)) frame.port_faces.pop(port_name, None) @@ -220,7 +289,23 @@ def _best(unit: "Unit", frame, port_name: str, menu: list[str], def _cost(anchor: tuple[float, float], normal: tuple[float, float], target: tuple[float, float]) -> float: - """Orthogonal run to the peer, and the price of turning back.""" + """Return the orthogonal run length plus a penalty for facing away. + + Parameters + ---------- + anchor : tuple[float, float] + Port anchor on the candidate face. + normal : tuple[float, float] + Outward normal of that face. + target : tuple[float, float] + Point the run must reach. + + Returns + ------- + float + Manhattan distance, plus twice the stand-off when the target lies + behind the face. + """ (ax, ay), (nx, ny), (tx, ty) = anchor, normal, target reach = (tx - ax) * nx + (ty - ay) * ny return abs(tx - ax) + abs(ty - ay) + (0.0 if reach >= 0.0 else 2.0 * _ESCAPE) @@ -228,5 +313,20 @@ def _cost(anchor: tuple[float, float], normal: tuple[float, float], def _aim(centre: tuple[float, float], normal: tuple[float, float], target: tuple[float, float]) -> float: - """How squarely a face's outward normal points at the peer.""" + """Return how directly a face's outward normal points at the target. + + Parameters + ---------- + centre : tuple[float, float] + Centre of the unit's drawn box. + normal : tuple[float, float] + Outward normal of the candidate face. + target : tuple[float, float] + Point the run must reach. + + Returns + ------- + float + Projection of the centre-to-target vector on the normal. + """ return (target[0] - centre[0]) * normal[0] + (target[1] - centre[1]) * normal[1] diff --git a/pandid/layout/trials.py b/pandid/layout/trials.py index 579fdd6..ca28ef2 100644 --- a/pandid/layout/trials.py +++ b/pandid/layout/trials.py @@ -17,8 +17,13 @@ from pandid.routing import Router +#: Smallest estimated gain, as a fraction of total route length, that +#: justifies an exact trial. MIN_ESTIMATED_GAIN_FRACTION = 0.004 +#: Maximum re-layouts one default route spends on sealed nozzle exits. +MAX_EXIT_ROUNDS = 3 + @dataclass(frozen=True) class TrialResult: @@ -265,8 +270,82 @@ def _publish_candidate(fs: Flowsheet, candidate: Flowsheet) -> None: fs._route_stale = False +def _exits_repaired(before: Quality, after: Quality) -> bool: + """Check that a re-layout removed hard conflicts and added none. + + Parameters + ---------- + before, after : Quality + Measurements of the current and re-laid completed drawing. + + Returns + ------- + bool + Whether author intent and pins are preserved, no hard count rises, + and fewer hard conflicts remain. + """ + return ( + before.author_intent == after.author_intent + and before.frame_transform == after.frame_transform + and before.pin_geometry == after.pin_geometry + and before.manual_endpoint_geometry == after.manual_endpoint_geometry + and all(new <= old for old, new in zip(before.hard, after.hard)) + and len(after.hard_conflicts) < len(before.hard_conflicts) + ) + + +def refine_exits(fs: Flowsheet) -> bool: + """Undo the row alignment that seals a connected nozzle exit. + + Each round re-lays a detached copy with the owner and blocker of every + ``blocked-exit`` conflict kept on their row axis. Pinned units are + unaffected. Equipment-first layouts are left unchanged. + + Parameters + ---------- + fs : Flowsheet + Completed default drawing to repair without changing its model. + + Returns + ------- + bool + Whether a re-laid drawing with fewer hard conflicts was published. + """ + from pandid.layout import default_layout_engine + + if fs._coarse_layout_candidate: + return False + best = measure_final(fs) + winner: Flowsheet | None = None + held: set[int] = set() + for _ in range(MAX_EXIT_ROUNDS): + blocked = { + index + for conflict in best.hard_conflicts + if conflict.kind == "blocked-exit" + for index in conflict.units + } - held + if not blocked: + break + held |= blocked + trial = copy.deepcopy(fs) + default_layout_engine.layout(trial, unaligned=frozenset(held)) + trial._layout_stale = False + trial._route_stale = True + settle(trial) + after = measure_final(trial) + if not _exits_repaired(best, after): + break + best, winner = after, trial + if winner is None: + return False + _publish_candidate(fs, winner) + fs._search_seed_frames = None + return True + + def refine_default(fs: Flowsheet, *, max_trials: int = 1) -> bool: - """Publish the first qualifying local proposal within a fixed budget. + """Repair sealed exits, then publish the first qualifying local proposal. Parameters ---------- @@ -278,13 +357,14 @@ def refine_default(fs: Flowsheet, *, max_trials: int = 1) -> bool: Returns ------- bool - Whether an accepted proposal changed the drawing. + Whether a repair or an accepted proposal changed the drawing. """ from pandid.layout.candidates import generate from pandid.routing.metrics import path_length if max_trials <= 0: return False + repaired = refine_exits(fs) total_length = sum( path_length(stream.route.waypoints) for stream in fs.streams if stream.route is not None ) @@ -304,7 +384,7 @@ def refine_default(fs: Flowsheet, *, max_trials: int = 1) -> bool: ) _publish_candidate(fs, candidate) return True - return False + return repaired def refine_rows(fs: Flowsheet) -> bool: diff --git a/pandid/render/_vendored_symbols.py b/pandid/render/_vendored_symbols.py index 86d22f8..6f12672 100644 --- a/pandid/render/_vendored_symbols.py +++ b/pandid/render/_vendored_symbols.py @@ -347,6 +347,7 @@ def register_vendored(registry): drawio_shape='mxgraph.pid.vessels.pressurized_vessel', # must not be turned: top-entering agitator over a dished bottom gravity_fixed=True, + port_faces={'outlet': {'S': (31.0, 100.0), 'E': (62.0, 82.5), 'W': (0.0, 82.5)}}, port_series=(PortSeries('feed_', 'W', pitch=14.0, extent=0.32, at=50.0, singular='feed'),), ), 'default') @@ -1097,6 +1098,7 @@ def register_vendored(registry): drawio_shape='mxgraph.pid.vessels.reactor', # must not be turned: vent on the top head, outlet in the floor gravity_fixed=True, + port_faces={'outlet': {'S': (20.0, 95.4), 'E': (40.0, 77.7), 'W': (0.0, 77.7)}}, port_series=(PortSeries('feed_', 'W', pitch=14.0, extent=0.4, at=30.0, singular='feed'),), ), 'plain') diff --git a/pandid/render/preview.py b/pandid/render/preview.py index e96efbe..e36093d 100644 --- a/pandid/render/preview.py +++ b/pandid/render/preview.py @@ -109,6 +109,22 @@ def _macos_gui_session() -> bool: return True +def _close(root) -> None: + """Destroy a Tk root after running its pending idle tasks. + + Tk 9 on macOS aborts the process when a later root services an idle + task queued by a root that no longer exists. + + Parameters + ---------- + root : tkinter.Tk + Window to close. + """ + root.withdraw() + root.update_idletasks() + root.destroy() + + def _no_display() -> str: """Return a display-unavailability reason, if any. @@ -129,7 +145,7 @@ def _no_display() -> str: root = tkinter.Tk() except tkinter.TclError as e: return f"no display ({e})" - root.destroy() + _close(root) return "" @@ -250,7 +266,7 @@ def _browser(svg: str, title: str, why: str) -> None: """ global _grace_until path = _preview_dir() / f"{_slug(title)}.svg" - path.parent.mkdir(parents=True, exist_ok=True) # a sweep may have taken it + path.parent.mkdir(parents=True, exist_ok=True) # Recreate it after a sweep. path.write_text(svg, encoding="utf-8") print(f"pandid: no window available ({why}); opened {path} in your browser instead") _grace_until = time.monotonic() + _GRACE_S @@ -306,7 +322,7 @@ def preview(svg: str, *, title: str = "") -> str: _window(root, png, title) except tkinter.TclError as e: # Fall back when Tk cannot read PNG data. - root.destroy() + _close(root) why = f"this Tk cannot display a PNG ({e})" else: root.mainloop() diff --git a/pandid/routing/__init__.py b/pandid/routing/__init__.py index f3de97d..14a57bb 100644 --- a/pandid/routing/__init__.py +++ b/pandid/routing/__init__.py @@ -1,9 +1,11 @@ +"""Route automatic streams orthogonally between placed nozzles.""" + import math import warnings from typing import Protocol, TYPE_CHECKING from pandid.geometry import Route -from pandid.portgeom import outward_dir as get_outward_dir # re-exported for callers +from pandid.portgeom import outward_dir as get_outward_dir # Re-exported for callers. if TYPE_CHECKING: from pandid.flowsheet import Flowsheet @@ -14,8 +16,21 @@ class Router(Protocol): + """Define the interface ``Flowsheet.route`` requires of a router.""" + def route(self, fs: "Flowsheet") -> None: - """Route all streams in the flowsheet.""" + """Route all streams in the flowsheet. + + Parameters + ---------- + fs : Flowsheet + Sheet with resolved unit frames. + + Returns + ------- + None + Routes are stored on the streams. + """ def _clamp_projection( @@ -25,17 +40,29 @@ def _clamp_projection( target: tuple[float, float], nodes: set[tuple[float, float]], ) -> tuple[float, float]: - """Pull an escape projection back onto a lane the stub would otherwise overshoot. + """Shorten an escape projection onto the lane its peer occupies. - The escape distance is a *maximum* stand-off, not a fixed one. When the far - end of the run already sits on the outward ray (a nozzle 15px above the lane - its stream has to join, say, projected 25px out), insisting on the full - projection makes the path overshoot and come back, which is two bends and - part of the stub drawn twice. Turning onto that lane on the way out is one. + The stand-off is a maximum. Turning onto the peer's lane on the way out + avoids an overshoot and return. - Going further out stays available: the continuation is the same direction, - so it costs no bend, which makes this purely a relaxation of where the path - is *allowed* to turn. + Parameters + ---------- + anchor : tuple[float, float] + Port anchor on the unit body. + proj : tuple[float, float] + Nominal escape node. + d : str or None + Outward port direction. + target : tuple[float, float] + Escape node at the other end of the stream. + nodes : set[tuple[float, float]] + Visibility-graph nodes. + + Returns + ------- + tuple[float, float] + The shortened escape node, or ``proj`` when the peer's lane is not + strictly inside the stub or is not a graph node. """ ax, ay = anchor px, py = proj @@ -48,7 +75,7 @@ def _clamp_projection( cand = (tx, ay) if lo < tx < hi else proj else: return proj - # A lane the visibility grid does not carry is no lane at all. + # Keep the nominal projection when the graph has no node on that lane. return cand if cand in nodes else proj @@ -60,48 +87,43 @@ def _fallback_path( obstacles: list, crossing_index: "CrossingIndex | None" = None, ) -> list[tuple[float, float]]: - """The L to draw when the search comes back with nothing. - - A* returns nothing when the two escape nodes are not connected on the - lane grid -- most often because one of them has been sealed inside a - neighbouring unit, which leaves it out of the graph entirely and so - with no edges to search along. Something still has to be drawn, and an - L through the two projections is it. - - Which L, though, is a choice, and it was not being made: the corner - always went across first and then down, wherever that landed. Both - orders are candidates here and the one crossing fewer obstacles wins; - a tie on obstacles is broken by which crosses fewer already-drawn - lines (``crossing_index``, when the caller has one -- this function is - also called before any stream has been routed, from tests and from - ``route_quality.py``'s own probes, where there is nothing yet to - cross), and the across-first order keeps whatever tie is left, so a - sheet nothing is in the way of draws exactly what it drew before. - - Considering obstacles first and lines second, not the other way round - or as one combined score, is deliberate: a line drawn through a vessel - is a worse defect than one drawn over another line, which is a stated - convention with its own hop (#384), not damage. This function is not - part of the search, so it cannot weigh the two against each other the - way ``CROSSING_PENALTY`` weighs a crossing against a bend inside - ``find_path`` -- it only ever has two fixed candidates to rank, and - ranks them the same way an author choosing by eye would: clear of - equipment first, clear of other lines second. - - Where every candidate is blocked there is no better line to draw -- - the search has already established that the grid has none -- so the - least bad one goes down and ``validate()`` names it, under - ``route-crosses-unit``. Silently preferring a clear L when one exists - is the half that belongs here; saying so when none does is the half - that belongs there. - - Any corner repeating the point before it is dropped. Two projections - already in one column put the corner on top of ``start_proj``, and the - caller's simplifier keeps both verbatim (it never drops a projection - point), leaving a zero-length segment that the separation pass then - reads as a horizontal run on a track the stream does not occupy. + """Return the L-shaped route drawn when no path is found. + + Both corner orders are candidates. The one crossing fewer obstacles + wins, then the one crossing fewer recorded lines, then the across-first + order. A route through equipment is a defect; a line crossing is a drawn + convention. ``validate()`` reports a blocked result as + ``route-crosses-unit``. + + Parameters + ---------- + start, goal : tuple[float, float] + Port anchors. + start_proj, goal_proj : tuple[float, float] + Escape nodes of the two ports. + obstacles : list + Rectangles the route should avoid. + crossing_index : CrossingIndex or None, optional + Routes already drawn in this pass. + + Returns + ------- + list[tuple[float, float]] + Waypoints from ``start`` to ``goal`` without repeated points. """ def through(corner: tuple[float, float]) -> list[tuple[float, float]]: + """Build the route that turns at one corner. + + Parameters + ---------- + corner : tuple[float, float] + Point where the route changes direction. + + Returns + ------- + list[tuple[float, float]] + Waypoints with consecutive duplicates removed. + """ pts = [start] for point in (start_proj, corner, goal_proj, goal): if point != pts[-1]: @@ -109,6 +131,18 @@ def through(corner: tuple[float, float]) -> list[tuple[float, float]]: return pts def score(pts: list[tuple[float, float]]) -> tuple[int, int]: + """Count the obstacles and recorded lines a route crosses. + + Parameters + ---------- + pts : list[tuple[float, float]] + Candidate route waypoints. + + Returns + ------- + tuple[int, int] + Segments that hit an obstacle, then crossings of recorded lines. + """ obstacle_hits = sum( any(o.intersects_segment(a[0], a[1], b[0], b[1]) for o in obstacles) for a, b in zip(pts, pts[1:]) @@ -126,23 +160,24 @@ def score(pts: list[tuple[float, float]]) -> tuple[int, int]: def _record_route(crossing_index: "CrossingIndex", waypoints: list[tuple[float, float]], manual: bool) -> None: - """Record one committed route's geometry into *crossing_index*. - - A router-drawn path is orthogonal by construction and - ``CrossingIndex.record`` raises if one somehow is not, because that would - be a router bug. A manual (``.via()``) route carries no such guarantee -- - ``validate()``'s own ``route-diagonal`` finding exists because an - author's waypoints can legitimately be drawn on the slant -- so it is - recorded one orthogonal run at a time here, and a diagonal leg is left - out of the index rather than taking the whole sheet's routing down with - it; the author still gets ``route-diagonal`` for the leg itself. - - Leaving the leg out means it is never priced against either: a diagonal - ``.via()`` leg is drawn, and a later search can cross it for free. - Tracked as #510, not fixed here -- pricing a genuinely diagonal segment - is a geometric extension to ``CrossingIndex``/``crossings_along``, which - only ever test an orthogonal one, not a bookkeeping change like this - function. + """Record one committed route in the crossing index. + + A manual route is recorded one orthogonal run at a time. Its diagonal + legs are left out, so later searches do not price crossing them (#510). + + Parameters + ---------- + crossing_index : CrossingIndex + Index of routes drawn so far in this pass. + waypoints : list[tuple[float, float]] + Route waypoints. + manual : bool + Whether the author supplied the waypoints with ``via()``. + + Returns + ------- + None + The index is updated in place. """ if not manual: crossing_index.record(waypoints) @@ -159,14 +194,24 @@ def _record_route(crossing_index: "CrossingIndex", waypoints: list[tuple[float, def _refuse_non_finite_geometry(fs: "Flowsheet") -> None: - """Refuse a sheet carrying a NaN or an infinity in its geometry. - - Such a coordinate does not stay where it was put. It reaches the - visibility graph as a lane and as an obstacle edge, and from there the - search, where it is fatal: every comparison against NaN is false, so A*'s - ``visited`` prune settles nothing, every state re-expands and the render - never returns. Caught here, before the graph is built, because this is the - last place that can name the unit carrying it. + """Reject a sheet whose frames hold a NaN or an infinity. + + A non-finite coordinate stops the path search from terminating. + + Parameters + ---------- + fs : Flowsheet + Sheet about to be routed. + + Returns + ------- + None + Every placed frame is finite. + + Raises + ------ + ValueError + If a frame has a non-finite position or size. """ for unit in fs.units: frame = unit.frame @@ -190,10 +235,11 @@ class DefaultRouter: ---------- recover_exits : bool, optional Try connected lanes before a blocked nominal nozzle projection. + ``False`` searches from the nominal projection only. """ - def __init__(self, *, recover_exits: bool = False) -> None: - """Set the opt-in outward-stub recovery policy. + def __init__(self, *, recover_exits: bool = True) -> None: + """Set the outward-stub recovery policy. Parameters ---------- @@ -214,6 +260,11 @@ def route(self, fs: "Flowsheet") -> None: ------- None Final route waypoints and fallback status are stored on streams. + + Raises + ------ + ValueError + If a frame is non-finite or a stream's port has no owner. """ from pandid.routing.visibility import VisibilityGraph, share_escape_room from pandid.routing.astar import CrossingIndex, find_path @@ -222,70 +273,46 @@ def route(self, fs: "Flowsheet") -> None: _refuse_non_finite_geometry(fs) graph = VisibilityGraph(fs, margin=15.0) edge_penalties: dict[tuple[tuple[float, float], tuple[float, float]], float] = {} - # Every earlier stream's drawn segments in this loop -- see - # ``find_path``'s own docstring for ``crossing_index`` and - # ``CrossingIndex`` for the index itself. Earlier-only and rebuilt - # fresh on every call is deliberate: a stream can only be drawn - # crossing a line the sheet already carries, never one that has yet - # to be routed, and each of ``Flowsheet.route()``'s placement passes - # re-routes every stream against a graph that may have new - # obstacles, so a crossing charge left over from a previous pass - # would be pricing a sheet that no longer exists. + # Index the streams drawn earlier in this pass. Rebuild it on every + # call because each pass routes against new geometry. crossing_index = CrossingIndex() routed: list["Stream"] = [] def settle(stream: "Stream") -> None: - """Add *stream*, which already carries its final waypoints for - this pass, to ``crossing_index``. - - Rebuilds the whole index from a *preview* of where - ``separate_streams`` will actually put every routed stream's - waypoints (``preview_separated_waypoints``), not the raw, - pre-separation ones this loop searches and draws with, so a - later stream's search prices crossings against geometry close - to the sheet that gets drawn rather than one that never is -- - ``separate_streams`` only nudges a track a few pixels to keep - two parallel lines visually apart, but a few pixels is exactly - enough to make or unmake a crossing near a span's edge, and a - manual (``.via()``) route participates in that pass too. Redone - from scratch each call, not adjusted incrementally, because a - preview is cheap and correct while an incremental one would - have to reason about which of an earlier stream's crossings its - own last settlement already accounted for. - - Still an approximation: a stream not yet routed can pull an - already-recorded track when it is added, the same as any later - stream in this preview's own set can -- closing that gap - requires knowing the full final set before the first stream is - searched, which is not what "earlier only" means here. Measured - at 14 divergences out of 1032 streams on this corpus, not the - zero once claimed from a comparison that could not have found - them (see ``preview_separated_waypoints``, and #509 for the - undercharging that follows from it). + """Add a routed stream to the crossing index. + + The index is rebuilt from a preview of the separated waypoints, + so later searches price crossings against the drawn geometry. + A stream routed later can still move an earlier track (#509). + + Parameters + ---------- + stream : Stream + Stream carrying its waypoints for this pass. + + Returns + ------- + None + ``crossing_index`` is replaced. """ nonlocal crossing_index routed.append(stream) preview = preview_separated_waypoints(routed) crossing_index = CrossingIndex() for s in routed: - assert s.route is not None # every entry in ``routed`` was just given one + assert s.route is not None # Every routed stream has a route. wp = preview.get(id(s), s.route.waypoints) _record_route(crossing_index, wp, s.route.manual) for stream in fs.streams: if stream.route and stream.route.manual: - # A hand-drawn (``.via()``) route is still a line on the - # sheet, and one an auto-routed stream later in this order - # can genuinely cross -- it just is not one the search chose, - # so there is nothing here to search for or penalise reuse - # of, only geometry to record. + # Record a manual route so later streams price crossing it. settle(stream) continue src_u = stream.source.owner dst_u = stream.dest.owner - # Not an ``assert``: ``python -O`` strips those, and the line below - # would then fail with an AttributeError naming none of this. + # Raise explicitly because ``python -O`` removes assertions. if src_u is None or dst_u is None: orphan = stream.source if src_u is None else stream.dest raise ValueError( @@ -293,9 +320,7 @@ def settle(stream: "Stream") -> None: f"{orphan.name!r} belongs to no unit." ) - # The three ways a stream drops out below all leave ``stream.route`` - # None, which draws no line and sends every later render back - # through routing again. Say so rather than skip in silence. + # Warn when a stream is left unrouted; its route stays ``None``. if src_u.frame is None or dst_u.frame is None: unplaced = src_u if src_u.frame is None else dst_u warnings.warn( @@ -321,14 +346,12 @@ def settle(stream: "Stream") -> None: start_dir = graph.port_dirs.get((src_u.name, stream.source.name)) goal_dir = graph.port_dirs.get((dst_u.name, stream.dest.name)) - # The graph stood every port off to its escape node already, at the - # label-aware distance, and carries the lanes those land on. + # Read the escape nodes the graph placed at the stand-off distance. start_proj = graph.port_projs[(src_u.name, stream.source.name)] goal_proj = graph.port_projs[(dst_u.name, stream.dest.name)] - # The stand-off is a ceiling, and it applies at both ends. Prefer to - # give way onto a lane the grid already carries; only when neither - # end can is the room between them split down the middle. + # Shorten each stand-off onto the peer's lane, then share the room + # between two facing ports. start_proj = _clamp_projection(start, start_proj, start_dir, goal_proj, graph.nodes) goal_proj = _clamp_projection(goal, goal_proj, goal_dir, start_proj, graph.nodes) start_proj, goal_proj = share_escape_room( @@ -355,7 +378,7 @@ def settle(stream: "Stream") -> None: if path: path = [start] + path + [goal] - # Penalize used segments so later streams avoid overlapping them. + # Penalise used segments so later streams avoid overlapping them. for i in range(len(path) - 1): u_node, v_node = path[i], path[i+1] edge_penalties[(u_node, v_node)] = edge_penalties.get((u_node, v_node), 0.0) + 2000.0 @@ -365,8 +388,7 @@ def settle(stream: "Stream") -> None: start, start_proj, goal_proj, goal, graph.obstacles, crossing_index ) - # Simplify (remove collinear intermediate points), but never drop the - # projection points that guarantee a clean port exit/entry. + # Remove collinear points but keep both escape nodes. simplified = [path[0]] for i in range(1, len(path)-1): prev = simplified[-1] @@ -383,16 +405,9 @@ def settle(stream: "Stream") -> None: stream.route = Route(waypoints=simplified, used_fallback=not path_found) - # Record this stream's own drawn segments -- the fallback L - # included -- so a later one prices crossing them. Not folded - # into the ``if path:`` branch above: a fallback L is still a - # line drawn on the sheet, and one #425's own corpus already had - # a later stream cross unnoticed (``AE-303-80-80-SS`` on - # ``11_ethanol_pid+auto``, a stream this router itself drew by - # the fallback path) while only edge-reuse pricing, not - # crossing pricing, has ever had a reason to stay blind to it. + # Record every drawn route, including a fallback, for later streams. settle(stream) - # Apply parallel segment separation pass + # Separate parallel segments that share a lane. from pandid.routing.separation import separate_streams separate_streams(fs) diff --git a/scripts/route_quality.py b/scripts/route_quality.py index 9f66355..827b48c 100644 --- a/scripts/route_quality.py +++ b/scripts/route_quality.py @@ -1,48 +1,26 @@ #!/usr/bin/env python3 -"""Measure whether the routes ``route()`` draws are *good* drawings, not just fast ones. - -``route_bench.py`` times ``layout()`` and ``route()``; this asks the question -speed says nothing about: given the sheet it was handed, did the router draw a -line a draughtsman would draw? +"""Measure the quality of the routes ``route()`` draws. python scripts/route_quality.py # every example - python scripts/route_quality.py 11_ethanol_pid # one of them + python scripts/route_quality.py 11_ethanol_pid # one example python scripts/route_quality.py --synthetic # the probe sheets only - python scripts/route_quality.py --worst 15 # widen the offender lists - -Five things are measured, each against a bound computed from the geometry -itself rather than a fixed threshold, so a sheet with tighter equipment reads -no worse than a roomy one for the same quality of routing: - -- **bends**, against the fewest an orthogonal path leaving the source nozzle - along its outward face and arriving at the destination against its can - possibly have (:func:`min_bends`) -- zero for two nozzles facing each other - down one lane, and never fewer than the geometry allows regardless of what - is in the way; -- **length**, against the Manhattan distance between the two nozzles, which - bounds *every* orthogonal path between them, direction and obstacles alike; -- **crossings** between two different streams' drawn segments, and, for each, - whether the *later*-routed stream (the one whose search saw the earlier - one's ``edge_penalties``) had a same-or-near-length alternative that misses - it -- found by re-running its own search with that one lane barred, so an - "avoidable" verdict is never asserted, only demonstrated; -- **the fallback path**: how many auto-routed streams got the L - ``_fallback_path`` draws because ``find_path`` returned nothing, which is - issue #355's territory -- see :class:`Recorder`; -- **the expansion budget** (``pandid/routing/astar.py``'s - ``max(50_000, 16 * nodes)``), as a fraction actually spent per search, so a - search that nearly ran out reads as different from one that walked away - from most of its ceiling. - -Every metric reads the complete drawn stream path after ``route()`` returns -- -the nozzle points and separated route waypoints -- except the -avoidability re-route, which necessarily replays the pre-separation search -(see :class:`Recorder`'s docstring for why that is the more honest ground to -test on, and why it does not change the headline crossing count). - -Nothing here changes a route. This is read-only over what ``route()`` already -decided; the only extra work it does is a handful of *additional* searches, -each with one lane barred, purely to answer "was there another way". + python scripts/route_quality.py --worst 15 # longer offender lists + +Each measure is compared with a bound computed from the sheet's own geometry: + +- **bends**, against the fewest an orthogonal path between the two nozzle + faces can have (:func:`min_bends`); +- **length**, against the Manhattan distance between the two nozzles; +- **crossings** between different streams, and whether the later-routed + stream had an alternative of similar length, found by repeating its search + with the crossed lane barred; +- **fallback routes**, drawn when no route was found: a search that returned + nothing, or a nozzle exit sealed before any search (#355); +- **expansion budget**, as the fraction each search spent. + +Metrics read the complete drawn paths after ``route()`` returns. The +alternative-route test replays the search before separation. The script +changes no route; its only extra work is those repeated searches. """ from __future__ import annotations @@ -76,7 +54,7 @@ MIN_EXPANSION_BUDGET, find_path, ) -from pandid.routing.visibility import VisibilityGraph # noqa: E402 +from pandid.routing.visibility import TRAVEL, VisibilityGraph # noqa: E402 from pandid.routing.metrics import ( # noqa: E402 Point, crossing_point, @@ -90,6 +68,18 @@ @dataclass class Crossing: + """One crossing between two different streams. + + Attributes + ---------- + h_stream, v_stream : Stream + Streams owning the horizontal and vertical segments. + h_seg, v_seg : tuple[Point, Point] + Endpoints of the two segments. + point : Point + Where the segments cross. + """ + h_stream: object h_seg: tuple[Point, Point] v_stream: object @@ -99,6 +89,18 @@ class Crossing: @dataclass class Overlap: + """One stretch of track shared by two streams. + + Attributes + ---------- + a, b : Stream + Streams drawn on the same track. + axis : str + ``"h"`` or ``"v"``. + length : float + Shared length in drawing pixels. + """ + a: object b: object axis: str @@ -153,8 +155,7 @@ def find_crossings_and_overlaps(fs: Flowsheet) -> tuple[list[Crossing], list[Ove # --------------------------------------------------------------------------- -# Instrumented find_path: fallback, budget headroom, and the raw search state -# an avoidability re-route needs. +# Record path searches: budget use and the state a repeated search needs. # --------------------------------------------------------------------------- @@ -180,6 +181,8 @@ class Call: Search result before route separation. expansions, budget : int Search effort and its limit. + route_pass : int + Router pass that made the search, counted from one. """ sheet: Flowsheet @@ -193,6 +196,7 @@ class Call: result: list[Point] expansions: int budget: int + route_pass: int class Recorder: @@ -214,6 +218,9 @@ def __init__(self) -> None: """ self.calls: list[Call] = [] self._active_sheet: Flowsheet | None = None + self._active_pass = 0 + self._passes = 0 + self._last_pass: dict[int, int] = {} self._count = 0 def __enter__(self) -> "Recorder": @@ -249,12 +256,30 @@ def _record_route(self, router: DefaultRouter, fs: Flowsheet) -> None: None Routes are stored on ``fs`` by the original router. """ - previous = self._active_sheet - self._active_sheet = fs + previous, previous_pass = self._active_sheet, self._active_pass + self._passes += 1 + self._active_sheet, self._active_pass = fs, self._passes + self._last_pass[id(fs)] = self._passes try: self._original_route(router, fs) finally: - self._active_sheet = previous + self._active_sheet, self._active_pass = previous, previous_pass + + def final_pass(self, sheet: Flowsheet) -> list[Call]: + """Return the searches of the last router pass over one drawing. + + Parameters + ---------- + sheet : Flowsheet + Live or isolated drawing that was routed. + + Returns + ------- + list[Call] + Searches in router order; empty when the pass made none. + """ + last = self._last_pass.get(id(sheet)) + return [call for call in self.calls if call.sheet is sheet and call.route_pass == last] def _record_path(self, graph, start, goal, start_dir=None, goal_dir=None, edge_penalties=None, is_recycle=False, crossing_index=None): @@ -294,7 +319,8 @@ def _record_path(self, graph, start, goal, start_dir=None, goal_dir=None, budget = max(MIN_EXPANSION_BUDGET, MAX_EXPANSIONS_PER_NODE * len(graph.nodes)) self.calls.append( Call(self._active_sheet, graph, start, goal, start_dir, goal_dir, - dict(edge_penalties or {}), is_recycle, list(result), self._count, budget) + dict(edge_penalties or {}), is_recycle, list(result), self._count, budget, + self._active_pass) ) return result @@ -381,12 +407,23 @@ def _same_drawing(first: Flowsheet, second: Flowsheet) -> bool: def eligible_streams(fs: Flowsheet, graph: VisibilityGraph) -> list: - """Streams ``route()`` will call ``find_path`` for, in the same order. + """Return the streams ``route()`` draws automatically, in router order. + + The filter copies ``DefaultRouter.route()``: manual routes, unplaced + units, and ports without an anchor are excluded. Each stream makes at + most one ``find_path`` call per pass. - Replicates ``DefaultRouter.route()``'s filter (manual route, missing - frame, missing anchor) so the result lines up 1:1 with ``Recorder``'s - call log -- checked by the caller, not assumed, since a drift here would - silently misattribute every call downstream of it. + Parameters + ---------- + fs : Flowsheet + Routed drawing. + graph : VisibilityGraph + Graph of the pass being measured. + + Returns + ------- + list[Stream] + Streams in flowsheet order. """ out = [] for stream in fs.streams: @@ -405,12 +442,100 @@ def eligible_streams(fs: Flowsheet, graph: VisibilityGraph) -> list: return out +#: Longest stand-off ``escape_distance`` gives a nozzle, in drawing pixels. +MAX_ESCAPE = 50.0 + + +def _on_exit_ray(anchor: Point, direction: Optional[str], node: Point) -> bool: + """Check that a search endpoint lies on a nozzle's outward stub. + + Parameters + ---------- + anchor : Point + Nozzle anchor on the unit body. + direction : str or None + Outward face of the nozzle. + node : Point + Search start or goal. + + Returns + ------- + bool + Whether the node is the anchor's escape node at any permitted reach. + """ + travel = TRAVEL.get(direction) if direction is not None else None + if travel is None: + return node == anchor + axis, sign = travel + reach = sign * (node[axis] - anchor[axis]) + return node[1 - axis] == anchor[1 - axis] and 0.0 <= reach <= MAX_ESCAPE + + +def pair_calls(eligible: list, calls: list[Call], name: str) -> dict[int, Call]: + """Match one pass's searches to the streams that made them. + + The router draws a stream whose nozzle exit is sealed without searching, + so a pass can log fewer calls than it has streams. + + Parameters + ---------- + eligible : list[Stream] + Automatically routed streams in router order. + calls : list[Call] + Searches logged by one router pass, in order. + name : str + Sheet name used in the failure message. + + Returns + ------- + dict[int, Call] + Search keyed by ``id()`` of its stream; streams drawn without a + search have no entry. + """ + paired: dict[int, Call] = {} + used = 0 + for stream in eligible: + if used == len(calls): + break + call = calls[used] + source = (stream.source.owner.name, stream.source.name) + dest = (stream.dest.owner.name, stream.dest.name) + anchors, faces = call.graph.port_anchors, call.graph.port_dirs + if (_on_exit_ray(anchors[source], faces.get(source), call.start) + and _on_exit_ray(anchors[dest], faces.get(dest), call.goal)): + paired[id(stream)] = call + used += 1 + assert used == len(calls), ( + f"{name}: {len(calls) - used} of {len(calls)} searches match no " + f"stream of the {len(eligible)} eligible -- eligibility filter " + f"drifted from DefaultRouter.route(); re-check eligible_streams() " + f"against pandid/routing/__init__.py." + ) + return paired + + # --------------------------------------------------------------------------- -# Avoidability: could the later stream have missed this lane for free? +# Test whether the later stream could have avoided a crossed lane. # --------------------------------------------------------------------------- def lane_edges(graph: VisibilityGraph, axis: str, track: float) -> list[tuple[Point, Point]]: + """Return the graph edges that lie on one lane. + + Parameters + ---------- + graph : VisibilityGraph + Search graph. + axis : str + ``"h"`` for a horizontal lane, ``"v"`` for a vertical one. + track : float + Fixed coordinate of the lane. + + Returns + ------- + list[tuple[Point, Point]] + Directed edges whose two nodes are on the lane. + """ idx = 1 if axis == "h" else 0 edges = [] for u, neighbors in graph.edges.items(): @@ -423,9 +548,22 @@ def lane_edges(graph: VisibilityGraph, axis: str, track: float) -> list[tuple[Po def track_covering(raw_points: list[Point], axis: str, coord: float) -> Optional[float]: - """The pre-separation graph lane whose span covers *coord*, read off the - raw search path -- an exact ``graph.xs``/``graph.ys`` value, not the - (possibly separation-offset) drawn one.""" + """Return the lane of the raw search path that covers a coordinate. + + Parameters + ---------- + raw_points : list[Point] + Search path before separation, including both endpoints. + axis : str + ``"h"`` or ``"v"``. + coord : float + Position along the lane. + + Returns + ------- + float or None + Fixed coordinate of the covering segment, or ``None`` if none covers it. + """ for p1, p2 in zip(raw_points, raw_points[1:]): if axis == "h" and p1[1] == p2[1]: lo, hi = sorted((p1[0], p2[0])) @@ -439,22 +577,28 @@ def track_covering(raw_points: list[Point], axis: str, coord: float) -> Optional def avoidable(call: Call, axis: str, coord: float) -> tuple[Optional[str], Optional[float]]: - """Could *call*'s search have missed the lane its crossing segment is - on, for the same length or close to it? - - Bars every graph edge on that lane (on top of the exact - ``edge_penalties`` the real search saw) and re-runs the search. That is - a strictly harder problem than the real one, so a path found this way is - never better than the real one for a reason unrelated to the ban -- - only worse or equal, which is what "cost of avoiding it" means. - - Returns ``(None, None)`` when there is nothing to compare (the call - itself found no path, or the crossing point cannot be matched back to - one of its own segments -- both defensive, neither expected to fire on - the corpus); otherwise one of ``"avoidable"`` (an alternative within 2% - of the original length exists), ``"costly"`` (one exists but is - longer), or ``"forced"`` (no alternative at all), with the length - difference in px. + """Classify the cost of avoiding one crossed lane. + + The search is repeated with every edge on that lane barred, on top of + the penalties the original search saw. + + Parameters + ---------- + call : Call + Search made by the later-routed stream. + axis : str + Axis of that stream's crossing segment. + coord : float + Position of the crossing along the segment. + + Returns + ------- + tuple[str or None, float or None] + ``"avoidable"`` when an alternative within 2 % of the original + length exists, ``"costly"`` when a longer one exists, or + ``"forced"`` when none exists, with the added length in pixels. + ``(None, None)`` when the call found no path or the crossing + matches none of its segments. """ if not call.result: return None, None @@ -477,12 +621,32 @@ def avoidable(call: Call, axis: str, coord: float) -> tuple[Optional[str], Optio # --------------------------------------------------------------------------- -# One sheet, fully measured +# Measure one sheet. # --------------------------------------------------------------------------- @dataclass class StreamRow: + """Route measurements for one drawn stream. + + Attributes + ---------- + sheet, name : str + Sheet and stream names. + actual_bends : int + Direction changes on the drawn path. + min_bends : int or None + Fewest bends the two nozzle faces allow. + length : float + Drawn path length in pixels. + manhattan : float or None + Manhattan distance between the two nozzles. + is_fallback : bool + Whether the router drew the fallback route. + is_manual : bool + Whether the author supplied the waypoints. + """ + sheet: str name: str actual_bends: int @@ -495,6 +659,36 @@ class StreamRow: @dataclass class SheetReport: + """Route measurements for one sheet. + + Attributes + ---------- + sheet : str + Sheet name. + units, streams, nodes : int + Counts of units, streams, and visibility-graph nodes. + routed : int + Streams with a drawn path. + undrawn : list[str] + One message per stream left without a route. + fallback : int + Automatically routed streams drawn by the fallback. + auto_routed : int + Streams the router draws automatically. + budget_ratios : list[float] + Fraction of the expansion budget each final-pass search spent. + rows : list[StreamRow] + Per-stream measurements. + crossings : list[Crossing] + Crossings between different streams. + overlaps : list[Overlap] + Shared tracks between different streams. + self_crossings : int + Crossings of a stream with itself. + verdicts : list[tuple[str, float or None]] + Verdict and added length for each crossing tested for an alternative. + """ + sheet: str units: int streams: int @@ -508,7 +702,7 @@ class SheetReport: crossings: list[Crossing] overlaps: list[Overlap] self_crossings: int - verdicts: list[tuple[str, Optional[float]]] # (verdict, extra px) per testable crossing + verdicts: list[tuple[str, Optional[float]]] def measure_sheet(fs: Flowsheet, name: str) -> SheetReport: @@ -536,22 +730,10 @@ def measure_sheet(fs: Flowsheet, name: str) -> SheetReport: # Rejected isolated trials leave no trace in the measured route graph. owner = next((call.sheet for call in reversed(rec.calls) if _same_drawing(call.sheet, fs)), fs) - calls = [call for call in rec.calls if call.sheet is owner] - graph = calls[-1].graph if calls else VisibilityGraph(fs, margin=15.0) + final_pass = rec.final_pass(owner) + graph = final_pass[-1].graph if final_pass else VisibilityGraph(fs, margin=15.0) eligible = eligible_streams(fs, graph) - n = len(eligible) - if n == 0: - assert not calls, f"{name}: no eligible streams but {len(calls)} calls logged" - final_pass: list[Call] = [] - else: - assert len(calls) % n == 0, ( - f"{name}: {len(calls)} calls is not a whole number of " - f"{n}-stream passes -- eligibility filter drifted from " - f"DefaultRouter.route(); re-check eligible_streams() against " - f"pandid/routing/__init__.py." - ) - final_pass = calls[-n:] - stream_call = dict(zip((id(s) for s in eligible), final_pass)) + stream_call = pair_calls(eligible, final_pass, name) undrawn = [f"stream {stream.name!r} is left unrouted" for stream in fs.streams @@ -570,7 +752,6 @@ def measure_sheet(fs: Flowsheet, name: str) -> SheetReport: b = port_point(dst_u, dst_u.frame, s.dest.name) dir_a = graph.port_dirs.get((src_u.name, s.source.name)) dir_b = graph.port_dirs.get((dst_u.name, s.dest.name)) - call = stream_call.get(id(s)) rows.append(StreamRow( sheet=name, name=s.name, @@ -578,7 +759,7 @@ def measure_sheet(fs: Flowsheet, name: str) -> SheetReport: min_bends=min_bends(a, dir_a, b, dir_b) if a and b else None, length=path_length(wp), manhattan=(abs(b[0] - a[0]) + abs(b[1] - a[1])) if a and b else None, - is_fallback=bool(call and not call.result), + is_fallback=bool(not s.route.manual and s.route.used_fallback), is_manual=bool(s.route.manual), )) @@ -599,10 +780,10 @@ def measure_sheet(fs: Flowsheet, name: str) -> SheetReport: if verdict is not None: verdicts.append((verdict, extra)) - # Fallback and budget both read the final pass only, for the same reason - # the call-to-stream matching above does: an earlier pass's search is not - # what is on the sheet. ``len(rec.calls)`` includes trial searches too. - fallback = sum(1 for c in final_pass if not c.result) + # Read budget use from the final pass only; earlier passes and trials are + # not drawn. Count fallbacks from the published routes, which include a + # stream drawn without a search. + fallback = sum(1 for s in eligible if s.route is not None and s.route.used_fallback) return SheetReport( sheet=name, units=len(fs.units), @@ -611,7 +792,7 @@ def measure_sheet(fs: Flowsheet, name: str) -> SheetReport: routed=len(rows), undrawn=undrawn, fallback=fallback, - auto_routed=len(final_pass), + auto_routed=len(eligible), budget_ratios=[c.expansions / c.budget for c in final_pass], rows=rows, crossings=crossings, @@ -622,30 +803,48 @@ def measure_sheet(fs: Flowsheet, name: str) -> SheetReport: def measure_stem(stem: str) -> SheetReport: + """Measure one shipped example. + + Parameters + ---------- + stem : str + Example filename stem. + + Returns + ------- + SheetReport + Measurements of the routed example. + """ with contextlib.redirect_stdout(io.StringIO()): fs, _kwargs = gallery.flowsheet(stem) return measure_sheet(fs, stem) # --------------------------------------------------------------------------- -# Synthetic probes: controlled two-unit geometries, both to sanity-check -# min_bends against a hand-verifiable answer and to demonstrate #355's -# sealed-projection failure mode in isolation. +# Build probe sheets with known bend counts, a sealed projection (#355), and +# forced crossings. # --------------------------------------------------------------------------- def _facing_pair(bx: float, perp_offset: float, mirror_b: bool = False) -> Flowsheet: - """A's discharge to B's suction, B's suction placed *perp_offset* px off - A's discharge on the perpendicular axis (0 keeps the two nozzles level). - - B's suction is placed *by the nozzle*, via ``pin(port="suction", ...)``, - rather than by guessing the box corner that would put it there: the - two ports sit at different offsets within a pump's own artwork (10px - and 30px down from the top, at this writing, and this helper does not - hardcode either), so pinning the boxes level does not put the *ports* - level. Reading A's actual discharge anchor and asking B for the same y - on its suction is what a caller who wants that would do, and it stays - correct however the artwork is redrawn. + """Build two pumps joined discharge to suction. + + B is pinned by its suction nozzle, because the two ports sit at + different heights within the pump symbol. + + Parameters + ---------- + bx : float + X coordinate of B's suction. + perp_offset : float + Vertical offset of B's suction from A's discharge; 0 keeps them level. + mirror_b : bool, optional + Mirror B so its suction faces east. + + Returns + ------- + Flowsheet + Unrouted two-pump sheet. """ from pandid.portgeom import port_anchor @@ -660,41 +859,36 @@ def _facing_pair(bx: float, perp_offset: float, mirror_b: bool = False) -> Flows def synthetic_sheets() -> list[tuple[str, Flowsheet]]: + """Build the probe sheets. + + Returns + ------- + list[tuple[str, Flowsheet]] + Name and unrouted sheet for each probe. + """ sheets = [] - # Facing, level: A's discharge (E) meets B's suction (W) on the same row. - # min_bends must read 0, and a good router should draw a straight line. + # Face the nozzles on one row: minimum 0 bends. sheets.append(("facing_level", _facing_pair(300, 0))) - # Facing, offset: same directions, B's suction dropped 150px -- min_bends reads 2. + # Face the nozzles with B 150 px lower: minimum 2 bends. sheets.append(("facing_offset", _facing_pair(300, 150))) - # Facing, but B is *behind* A along the direction A leaves in (B's box - # sits west of A, though the two still face each other) -- min_bends - # reads 4, the one case that needs a full turn-around. + # Put B behind A's outward direction: minimum 4 bends. sheets.append(("facing_behind", _facing_pair(-300, 150))) - # Same-side: mirroring B puts its suction on the east face too, so both - # nozzles face the same compass direction. min_bends reads 2. + # Mirror B so both nozzles face east: minimum 2 bends. sheets.append(("same_side", _facing_pair(300, 150, mirror_b=True))) - # Perpendicular: rotate B a quarter turn so its suction faces north - # instead of west. min_bends reads 1 when B sits ahead on both axes. + # Rotate B so its suction faces north: minimum 1 bend. fs = Flowsheet("probe") a = fs.add(Pump("A")).pin(x=0, y=0) b = fs.add(Pump("B")).pin(x=300, y=200, orientation=90) fs.connect(a.discharge, b.suction) sheets.append(("perpendicular", fs)) - # https://github.com/Alpha9463/pandid/issues/355: an instrument balloon - # placed close enough to a feed's own outlet that the feed's escape node - # lands strictly inside the balloon's own obstacle box. find_path is - # handed a start node the graph does not carry and returns nothing; the - # stream is drawn by _fallback_path instead, which validate() then - # reports as route-crosses-unit. Still open, measured here rather than - # fixed: this reproduction crosses zero units from the shipped corpus, - # and the one real consequence (S1 crosses FCI-1) is already a warning a - # caller sees, not a silent miss. + # Place a balloon over the feed's nominal escape node (#355). The router + # shortens the stub and routes around the balloon. fs = Flowsheet("probe-355") f = fs.add(Feed("F")).pin(x=60, y=100) p = fs.add(Product("P")).pin(x=400, y=100) @@ -702,9 +896,7 @@ def synthetic_sheets() -> list[tuple[str, Flowsheet]]: fs.add_instrument("FCI", 1, near=f) sheets.append(("sealed_projection_355", fs)) - # Crossing stress: three parallel forward streams and one that must jog - # across all of them to reach a valve tapped off the far side, to give - # the crossing/avoidability analysis something to measure. + # Run one stream across three parallel streams to force crossings. fs = Flowsheet("probe-cross") for i in range(3): src = fs.add(Feed(f"F{i}")).pin(x=0, y=i * 60) @@ -721,19 +913,52 @@ def synthetic_sheets() -> list[tuple[str, Flowsheet]]: # --------------------------------------------------------------------------- -# Aggregation and reporting +# Aggregate and report. # --------------------------------------------------------------------------- def _pct(n: int, d: int) -> str: + """Format a ratio as a whole percentage. + + Parameters + ---------- + n, d : int + Numerator and denominator. + + Returns + ------- + str + Percentage, or ``"n/a"`` for a zero denominator. + """ return f"{100 * n / d:.0f}%" if d else "n/a" def _fmt(x: Optional[float], nd: int = 2) -> str: + """Format an optional number to fixed decimals. + + Parameters + ---------- + x : float or None + Value to format. + nd : int, optional + Decimal places. + + Returns + ------- + str + Formatted value, or ``"n/a"`` for ``None``. + """ return f"{x:.{nd}f}" if x is not None else "n/a" def print_sheet_table(reports: list[SheetReport]) -> None: + """Print one row of route measurements per sheet. + + Parameters + ---------- + reports : list[SheetReport] + Measured sheets. + """ header = ( f"{'sheet':<24}{'units':>6}{'strm':>6}{'routed':>7}{'undrwn':>7}{'fbck':>6}" f"{'bnd~0':>7}{'bnd+avg':>8}{'bnd+max':>8}{'len avg':>9}{'len max':>9}" @@ -742,11 +967,8 @@ def print_sheet_table(reports: list[SheetReport]) -> None: print(header) print("-" * len(header)) for r in reports: - # Manual (.via()) routes are excluded: they are the author's own - # choice, not the router's, and the stub-direction contract - # min_bends and the Manhattan bound both rest on -- first segment - # along the source's outward face, last along the destination's -- - # is the router's, not one a hand-written waypoint list has to keep. + # Exclude manual routes: the bend and length bounds assume the + # router's outward nozzle stubs. with_min = [(row, row.min_bends) for row in r.rows if row.min_bends is not None and not row.is_manual] excess = [row.actual_bends - mb for row, mb in with_min] optimal = sum(1 for e in excess if e == 0) @@ -766,6 +988,13 @@ def print_sheet_table(reports: list[SheetReport]) -> None: def print_crossing_summary(reports: list[SheetReport]) -> None: + """Print crossing totals and alternative-route verdicts. + + Parameters + ---------- + reports : list[SheetReport] + Measured sheets. + """ total = sum(len(r.crossings) for r in reports) verdicts: dict[str, int] = {} extra_costly = [] @@ -786,6 +1015,15 @@ def print_crossing_summary(reports: list[SheetReport]) -> None: def print_worst(reports: list[SheetReport], n: int) -> None: + """Print the worst streams by excess bends and length, then fallbacks. + + Parameters + ---------- + reports : list[SheetReport] + Measured sheets. + n : int + Streams listed per category. + """ all_rows = [row for r in reports for row in r.rows] with_min = [(row, row.min_bends) for row in all_rows if row.min_bends is not None and not row.is_manual] print(f"\nworst {n} by excess bends (actual - geometric minimum):") @@ -807,6 +1045,13 @@ def print_worst(reports: list[SheetReport], n: int) -> None: def print_undrawn(reports: list[SheetReport]) -> None: + """Print the streams left without a route. + + Parameters + ---------- + reports : list[SheetReport] + Measured sheets. + """ undrawn = [(r.sheet, msg) for r in reports for msg in r.undrawn] if not undrawn: return @@ -816,6 +1061,13 @@ def print_undrawn(reports: list[SheetReport]) -> None: def main() -> None: + """Parse the command line, measure the sheets, and print the report. + + Raises + ------ + SystemExit + If a named example does not exist. + """ ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) ap.add_argument("sheet", nargs="*", help="example stems; default is all of them") ap.add_argument("--synthetic", action="store_true", help="run only the synthetic probe sheets") diff --git a/scripts/vendor_symbols.py b/scripts/vendor_symbols.py index 7c33c21..7b64fbc 100644 --- a/scripts/vendor_symbols.py +++ b/scripts/vendor_symbols.py @@ -394,9 +394,13 @@ # stream out through the shell. So it sits at (62, 10), on the straight east # wall just under the tangent line at y 7.5, which is where the off-gas # leaves a vessel whose head is occupied. + # + # The outlet also has east and west placements at y 165, on the straight + # wall (y 15..185) below the duty and the feed family. ("reactor", "default"): ("vessels", "Pressurized Vessel", {"feed": ("SERIES", "W", 100.0, 28.0, 0.32), - "outlet": ("S", 50.0), "duty": ("E", 100.0), + "outlet": [("S", 50.0), ("E", 165.0), ("W", 165.0)], + "duty": ("E", 100.0), "vent": ("AT", 100.0, 20.0)}), # The drawing ``default`` used to be, kept under a name that says what it is # rather than deleted: a box with a stirrer perched on top of it is still @@ -1340,10 +1344,13 @@ ("thickener", "default"): ("vessels", "Settling Tank", {"feed": ("W", 8.0), "overflow": ("E", 8.0), "underflow": ("S", 50.0)}), - # Reactor / separator styles. The straight wall spans y 7.69..87.69. + # Reactor / separator styles. The straight wall spans y 7.69..87.69. The + # outlet also has east and west placements at y 77.7, below the duty and + # the feed family. ("reactor", "plain"): ("vessels", "Reactor", {"feed": ("SERIES", "W", 30, 14, 0.4), - "outlet": ("S", 20), "duty": ("E", 47), + "outlet": [("S", 20), ("E", 77.7), ("W", 77.7)], + "duty": ("E", 47), "vent": ("AT", 30.0, 7.69)}), # Horizontal vessel: reflux drum, accumulator, knock-out pot. A lying # cylinder with dished ends: the shape a vertical vessel does NOT become @@ -1806,11 +1813,24 @@ def patch_shape(stencil, name, el): - """Apply :data:`STENCIL_PATCHES` to one parsed , if it has one. + """Apply the :data:`STENCIL_PATCHES` entry for one parsed shape. + + The fragment is appended to the shape's ````, so it paints + over what the stencil drew. + + Parameters + ---------- + stencil : str + Stencil file stem. + name : str + Shape name within the stencil. + el : xml.etree.ElementTree.Element + Parsed ```` element. - The fragment is appended to the shape's , so it paints over - what the stencil already drew, exactly as a stencil author would have - written it in the first place. + Returns + ------- + xml.etree.ElementTree.Element + The same element, patched in place when an entry exists. """ entry = STENCIL_PATCHES.get((stencil, name)) if entry is None: @@ -2347,44 +2367,53 @@ def patch_shape(stencil, name, el): def scale_for(kind, variant): - """The (sx, sy) a symbol's artwork is drawn at, from :data:`SCALE`. + """Return the scale a symbol's artwork is drawn at. - A (kind, variant) entry beats the kind's own, so one variant can be resized - without dragging its siblings with it. + A ``(kind, variant)`` entry in :data:`SCALE` takes precedence over the + entry for the kind. + + Parameters + ---------- + kind : str + Unit kind. + variant : str + Symbol variant. + + Returns + ------- + tuple[float, float] + Horizontal and vertical scale factors. """ s = SCALE.get((kind, variant), SCALE.get(kind, 1.0)) return (float(s), float(s)) if isinstance(s, (int, float)) else (float(s[0]), float(s[1])) def drawio_shape_key(namespace, shape_name): - """The key draw.io's own stencil registry files one shape under. - - mxGraph builds it out of the two names the stencil file already carries: - ``mxStencilRegistry.parseStencilSet`` takes the package off the root - element, the shape's own name off the ````, replaces the spaces in - the shape's name with underscores and lowercases the pair. So - ``mxGraph.pid.valves`` and ``Gate Valve`` make - ``mxgraph.pid.valves.gate_valve``, which is what a ``shape=`` in a draw.io - style has to say to reach the very drawing this generator converted. - - Reproducing the rule is the whole point: a guessed key is not merely wrong, - it fails *quietly*. draw.io answers an unresolvable ``shape=`` with its - default rectangle, so the file still opens, the equipment is still all - there, and every symbol on the sheet has become a box. Deriving the key from - the same two strings the artwork was converted from is what makes the export - wrong loudly or not at all. - - Only spaces become underscores; the punctuation upstream puts in a shape - name survives ("Tank (Dished Roof)", "Y-Type Strainer", "Rotary Drum Drier, - Tumbling Drier"), and it has to, since draw.io does an exact dictionary - lookup on the key it built by the same rule. A **dot** is the one character - that cannot survive, and is refused rather than passed through: draw.io - finds the *file* to load by splitting the key on dots and treating all but - the last part as a path (``mxStencilRegistry.getBasenameForStencil``), so a - dot inside the shape's own name moves that boundary and sends draw.io - looking for a stencil file that was never there. No shape in the vendored - set has one today; this is here so that the day one arrives the generator - stops instead of emitting a reference nothing can resolve. + """Return the key draw.io's stencil registry gives a shape. + + draw.io joins the stencil package and the shape name, replaces spaces + with underscores, and lowercases the result. It draws a plain rectangle + for an unknown key, so the key is derived by the same rule. draw.io + splits the key on dots to find the stencil file, so a dot in the shape + name is rejected. + + Parameters + ---------- + namespace : str + Package declared by the stencil file, for example + ``mxGraph.pid.valves``. + shape_name : str + Shape name as written in the stencil. + + Returns + ------- + str + Registry key, for example ``mxgraph.pid.valves.gate_valve``. + + Raises + ------ + SystemExit + If the shape name contains a dot. """ if "." in shape_name: raise SystemExit( @@ -2397,25 +2426,47 @@ def drawio_shape_key(namespace, shape_name): def is_series(spec): - """True for a port spec declaring a *family* rather than one nozzle.""" + """Return whether a port spec declares a nozzle family. + + Parameters + ---------- + spec : object + Port specification from :data:`KIND_MAP`. + + Returns + ------- + bool + Whether the spec is a ``("SERIES", ...)`` tuple. + """ return isinstance(spec, tuple) and spec[0] == "SERIES" def resolve_port(spec, constraints, w, h): - """Resolve a port spec to (x, y) in the shape's own units. - - ``"W"`` - a named draw.io anchor (compass point). - ``("E", 10.0)`` - a point on a bounding-box edge, at the given offset. - ``("AT", x, y)`` - an absolute point, for nozzles that sit inboard of the - bounding box (e.g. a dome crown, or a shell wall drawn - inside the box because brackets widen the extent). - - A fourth form, ``("SERIES", edge, along, pitch, extent)``, is not a nozzle at - all: it hands the port to a :class:`~pandid.render.symbols.PortSeries`, which - places as many as the unit turns out to have, ``pitch`` apart and centred on - ``along``. A sixth element names the alignment - (:data:`~pandid.render.symbols.FROM_START`) for a body with no room to - straddle its own nozzle. See :func:`is_series`. + """Resolve a single-nozzle port spec to a point in the shape's units. + + A spec is a named draw.io constraint such as ``"W"``, an edge offset + such as ``("E", 10.0)``, or an absolute point such as ``("AT", x, y)`` + for a nozzle inboard of the bounding box. A ``("SERIES", ...)`` spec is + handled by :func:`drawing`, not here. + + Parameters + ---------- + spec : str or tuple + Port specification. + constraints : dict[str, tuple[float, float]] + Named anchors read from the stencil. + w, h : float + Shape width and height. + + Returns + ------- + tuple[float, float] + Port coordinates. + + Raises + ------ + SystemExit + If a named constraint is missing from the stencil. """ if isinstance(spec, str): if spec not in constraints: @@ -2430,13 +2481,35 @@ def resolve_port(spec, constraints, w, h): def drawing(el, kind, variant, port_map, sx, sy): - """One shape converted, its ports resolved, both at the family's scale. + """Convert one shape and resolve its ports at the family's scale. - Returns ``(inner, w, h, ports, menu, series, aspect)``: the artwork, and - everything a :class:`~pandid.render.symbols.Symbol` is built from except its - id. Split out so the two states of a device (see :data:`CLOSED_SHAPES`) go - through exactly the same arithmetic, which is what lets :func:`render` insist - afterwards that the pair differs in ink and in nothing else. + A port spec given as a list declares alternate faces after its home + placement. + + Parameters + ---------- + el : xml.etree.ElementTree.Element + Parsed ```` element. + kind : str + Unit kind. + variant : str + Symbol variant. + port_map : dict[str, object] + Port specifications keyed by port name. + sx, sy : float + Horizontal and vertical scale factors. + + Returns + ------- + tuple + ``(inner, w, h, ports, menu, series, aspect)``: the SVG artwork, its + size, single-nozzle points, face menus, nozzle families, and the + stencil's aspect setting. + + Raises + ------ + SystemExit + If an alternate face is not an edge or point spec. """ # Emit a heavier stroke on scaled symbols so it renders at 2px after the # scale transform (2px matches streams + hand-drawn symbols exactly). @@ -2508,13 +2581,20 @@ def drawing(el, kind, variant, port_map, sx, sy): def render() -> str: - """The whole generated module, as a string, writing nothing. + """Return the text of the generated module without writing it. + + A test compares this text with the committed ``_vendored_symbols.py``. + + Returns + ------- + str + Python source with LF line endings. - Separate from :func:`main` so a test can regenerate the library in memory and - compare it against the committed ``_vendored_symbols.py``. That file is - regenerated wholesale, so a hand edit to it is lost the next time anyone runs - the generator and the drawing silently reverts; nothing but that comparison - notices, since a stale generated file still imports and still draws. + Raises + ------ + SystemExit + If a table names a shape or symbol the generator does not draw, or + the two states of a device differ in more than their artwork. """ # Index every shape once, correcting the ones STENCIL_PATCHES names, and # note the package each file declares, which is what its shapes are filed @@ -2708,6 +2788,7 @@ def render() -> str: def main(): + """Write the generated module and print a summary.""" OUT.write_text(render(), encoding="utf-8") print(f"wrote {OUT} ({len(KIND_MAP)} symbols, " f"{len(CLOSED_SHAPES)} of them in two positions)") diff --git a/tests/golden/18_fixed_bed_recycle.svg b/tests/golden/18_fixed_bed_recycle.svg index a6d369c..e71fe1b 100644 --- a/tests/golden/18_fixed_bed_recycle.svg +++ b/tests/golden/18_fixed_bed_recycle.svg @@ -230,7 +230,7 @@ - + @@ -338,8 +338,8 @@ 350-LG-314-CS 350-LG-304-LT - - 350-LG-305-LT + + 350-LG-305-LT 350-LG-306-CS diff --git a/tests/test_faces.py b/tests/test_faces.py index 08d00b5..86fd280 100644 --- a/tests/test_faces.py +++ b/tests/test_faces.py @@ -1,5 +1,4 @@ -"""Automatic port-face selection: the engine picking which face a movable port -is piped from, given where the unit at the other end of the stream landed.""" +"""Check automatic port-face selection against the placed peer unit.""" import pytest @@ -9,11 +8,24 @@ def _drum_fed_from(x, y, *, fs=None, **drum_pin): - """A horizontal separator at (200, 200) fed by a flag whose tip is (x, y). + """Build a horizontal separator fed by one boundary flag. - The drum's ``feed`` is the movable nozzle: its symbol authors a coordinate - on the west head, the north shell and the east head, so where the flag sits - is the only thing that can decide between them. + The drum's ``feed`` has west, north, and east placements, so the flag + position alone decides the face. + + Parameters + ---------- + x, y : float + Pinned position of the flag's nozzle. + fs : Flowsheet or None, optional + Sheet to add the units to; a new one by default. + **drum_pin + Extra ``pin()`` keywords for the drum. + + Returns + ------- + tuple[Flowsheet, Separator] + The sheet and the drum pinned at (200, 200). """ fs = fs if fs is not None else Flowsheet("faces") drum = fs.add(units.Separator("V-1", variant="horizontal")) @@ -23,41 +35,72 @@ def _drum_fed_from(x, y, *, fs=None, **drum_pin): return fs, drum -# --- taking the face the peer is actually on --------------------------------- +# --- Selection by peer position ---------------------------------------------- def test_a_peer_overhead_takes_the_north_shell(): + """Select the north placement for a peer above the drum. + + Returns + ------- + None + The feed is drawn on the north face. + """ fs, drum = _drum_fed_from(220, 60) fs.layout() assert port_anchor(drum, drum.frame, "feed")[2] == "N" def test_a_peer_astern_takes_the_far_head(): - """The east head, which no example names by hand: the engine reaches it on - the same evidence it reaches the north shell on.""" + """Select the east placement for a peer to the right of the drum. + + Returns + ------- + None + The feed is drawn on the east face. + """ fs, drum = _drum_fed_from(500, 215) fs.layout() assert port_anchor(drum, drum.frame, "feed")[2] == "E" def test_a_peer_ahead_leaves_the_home_nozzle_alone(): + """Keep the home placement for a peer to the left of the drum. + + Returns + ------- + None + The feed stays on the west face. + """ fs, drum = _drum_fed_from(20, 215) fs.layout() assert port_anchor(drum, drum.frame, "feed")[2] == "W" def test_the_face_is_named_in_drawn_space(): - """The alternate is authored on the symbol's north head, so on a unit flipped - top-to-bottom it is drawn on the south. A peer *below* is what reaches it.""" + """Name the selected face as drawn on a vertically mirrored unit. + + Returns + ------- + None + The symbol's north placement is drawn, and reported, on the south. + """ fs, drum = _drum_fed_from(220, 400, mirrored="y") fs.layout() assert port_anchor(drum, drum.frame, "feed")[2] == "S" -# --- what selection must not touch ------------------------------------------- +# --- Ports selection leaves alone -------------------------------------------- def test_an_explicit_nozzle_beats_the_engine(): + """Keep a face the author chose with ``nozzle()``. + + Returns + ------- + None + The authored face is drawn and no automatic choice is stored. + """ fs, drum = _drum_fed_from(220, 60) drum.nozzle("feed", "E") fs.layout() @@ -66,8 +109,13 @@ def test_an_explicit_nozzle_beats_the_engine(): def test_a_nozzle_fixed_by_physics_is_never_considered(): - """A column's bottoms leaves the bottom whatever is drawn above it: its - symbol authors one placement, so there is nothing to choose between.""" + """Leave a port with one declared placement on its face. + + Returns + ------- + None + A column's bottoms stays south although its peer is above. + """ fs = Flowsheet("gravity") col = fs.add(units.Column("T-1")).pin(x=300, y=400) sump = fs.add(units.Product("Bottoms")).pin(x=300, y=100) @@ -78,10 +126,13 @@ def test_a_nozzle_fixed_by_physics_is_never_considered(): def test_a_feed_family_is_fixed_the_way_a_single_nozzle_is(): - """Each member of a column's feed family has exactly one placement (the - family spreads *along* the shell wall, it does not offer other walls), so - selection has nothing to choose between and leaves every one of them where - the symbol put it.""" + """Leave every member of a column feed family on the west wall. + + Returns + ------- + None + Both feeds stay west at distinct anchors with no automatic choice. + """ fs = Flowsheet("extractive") col = fs.add(units.Column("T-302", n_feeds=2)).pin(x=300, y=200) solvent = fs.add(units.Feed("Solvent")).pin(x=80, y=250) @@ -92,13 +143,18 @@ def test_a_feed_family_is_fixed_the_way_a_single_nozzle_is(): assert port_anchor(col, col.frame, "feed_1")[2] == "W" assert port_anchor(col, col.frame, "feed_2")[2] == "W" assert col.frame.port_faces == {} - # ...and they are two nozzles, not one drawn twice. + # The family members occupy separate nozzles. assert port_anchor(col, col.frame, "feed_1") != port_anchor(col, col.frame, "feed_2") def test_a_draw_family_is_fixed_on_the_east_wall_a_feed_never_reaches(): - """The feed's mirror: a draw's family spreads down the east wall the - same way, and offers no other face to select between either.""" + """Leave every member of a column draw family on the east wall. + + Returns + ------- + None + Both draws stay east at distinct anchors with no automatic choice. + """ fs = Flowsheet("sidestream") col = fs.add(units.Column("T-401", n_draws=2)).pin(x=300, y=200) heavy = fs.add(units.Product("Heavy Naphtha")).pin(x=500, y=250) @@ -114,12 +170,16 @@ def test_a_draw_family_is_fixed_on_the_east_wall_a_feed_never_reaches(): def test_a_kettles_bottoms_draw_is_a_fixed_target_its_peer_aims_at(): - """The draw is on the shell bottom and offers nothing else, so it is what a - movable port at the other end of the line gets scored against.""" + """Score a movable port against its peer's fixed nozzle. + + Returns + ------- + None + The kettle's bottoms stays south and the drum feed turns north. + """ fs = Flowsheet("reboiler") reb = fs.add(units.HeatExchanger("E-702", variant="kettle")).pin(x=300, y=200) - # Hung so its north shell nozzle is directly under the draw, which is the - # face a straight drop reaches. + # Place the drum's north nozzle directly below the kettle's draw. drum = fs.add(units.Separator("V-1", variant="horizontal")).pin(x=365, y=380) fs.connect(reb.bottoms, drum.feed) fs.layout() @@ -129,13 +189,26 @@ def test_a_kettles_bottoms_draw_is_a_fixed_target_its_peer_aims_at(): def test_a_port_with_no_stream_keeps_its_home(): + """Select faces only for connected ports. + + Returns + ------- + None + The unconnected vapor and liquid ports have no automatic choice. + """ fs, drum = _drum_fed_from(220, 60) fs.layout() - # vapor and liquid are unconnected: no peer, so no evidence either way. assert set(drum.frame.port_faces) == {"feed_1"} def test_the_kill_switch_restores_the_symbols_own_nozzles(): + """Keep home placements when ``auto_faces`` is disabled. + + Returns + ------- + None + The feed stays west and no automatic choice is stored. + """ fs, drum = _drum_fed_from(220, 60, fs=Flowsheet("faces", auto_faces=False)) fs.layout() assert port_anchor(drum, drum.frame, "feed")[2] == "W" @@ -163,7 +236,7 @@ def test_recovered_escape_preserves_selected_and_authored_faces( Returns ------- None - The opt-in router leaves the face choice unchanged. + The router leaves the face choice unchanged. """ fs, drum = _drum_fed_from(220, 60, fs=Flowsheet("faces", auto_faces=auto_faces)) if explicit: @@ -173,18 +246,136 @@ def test_recovered_escape_preserves_selected_and_authored_faces( before = dict(drum.frame.port_faces) assert port_anchor(drum, drum.frame, "feed")[2] == face - fs.route(DefaultRouter(recover_exits=True)) + fs.route(DefaultRouter()) assert port_anchor(drum, drum.frame, "feed")[2] == face assert drum.frame.port_faces == before -# --- the choice is a result, not intent -------------------------------------- +# --- Reactor side outlets and blocked faces ---------------------------------- + + +def _reactor_discharging_to(x, y, *, blocker=None, **sheet): + """Build a pinned reactor whose outlet feeds one product flag. + + Parameters + ---------- + x, y : float + Pinned position of the flag's nozzle. + blocker : tuple[float, float] or None, optional + Pinned corner of a 40 px vessel placed beside the reactor. + **sheet + Extra ``Flowsheet`` keywords. + + Returns + ------- + tuple[Flowsheet, Reactor] + The sheet and the reactor pinned at (300, 200). + """ + fs = Flowsheet("reactor", **sheet) + reactor = fs.add(units.Reactor("R-1")).pin(x=300, y=200) + flag = fs.add(units.Product("P")).pin(x=x, y=y) + if blocker is not None: + fs.add(units.Vessel("V-1", width=40, height=40, label_pos="center")).pin( + x=blocker[0], y=blocker[1] + ) + fs.connect(reactor.outlet, flag.inlet) + return fs, reactor + + +@pytest.mark.parametrize( + ("x", "y", "face"), + [(600, 150, "E"), (60, 150, "W"), (331, 500, "S"), (600, 450, "S")], +) +def test_a_reactor_outlet_takes_a_side_only_when_it_saves_a_bend(x, y, face): + """Select a side outlet only when it needs no more bends than the floor. + + Parameters + ---------- + x, y : float + Pinned position of the product flag's nozzle. + face : str + Expected drawn face of the reactor outlet. + + Returns + ------- + None + A peer level with or above the shell takes a side; one below keeps + the floor outlet. + """ + fs, reactor = _reactor_discharging_to(x, y) + fs.layout() + assert port_anchor(reactor, reactor.frame, "outlet")[2] == face + + +def test_a_reactor_outlet_stays_on_the_floor_without_automatic_faces(): + """Keep the floor outlet when ``auto_faces`` is disabled. + + Returns + ------- + None + The outlet stays south although a side would save a bend. + """ + fs, reactor = _reactor_discharging_to(600, 150, auto_faces=False) + fs.layout() + assert port_anchor(reactor, reactor.frame, "outlet")[2] == "S" + assert reactor.frame.port_faces == {} + + +def test_an_explicit_reactor_outlet_face_beats_the_engine(): + """Keep a reactor outlet face the author chose with ``nozzle()``. + + Returns + ------- + None + The authored floor outlet is drawn and no automatic choice is stored. + """ + fs, reactor = _reactor_discharging_to(600, 150) + reactor.nozzle("outlet", "S") + fs.layout() + assert port_anchor(reactor, reactor.frame, "outlet")[2] == "S" + assert reactor.frame.port_faces == {} + + +@pytest.mark.parametrize( + ("x", "y", "blocker", "faces"), + [(600, 150, (370, 294), {"S"}), (331, 500, (311, 340), {"E", "W"})], +) +def test_a_face_whose_stub_crosses_a_unit_is_skipped(x, y, blocker, faces): + """Skip a face whose outward stub crosses another unit. + + Parameters + ---------- + x, y : float + Pinned position of the product flag's nozzle. + blocker : tuple[float, float] + Pinned corner of the vessel inside the preferred face's stub. + faces : set[str] + Clear faces the selection may take. + + Returns + ------- + None + A clear face is drawn and the route crosses no unit. + """ + fs, reactor = _reactor_discharging_to(x, y, blocker=blocker) + fs.layout() + assert port_anchor(reactor, reactor.frame, "outlet")[2] in faces + fs.route() + assert not any(issue.code == "route-crosses-unit" for issue in fs.validate()) + + +# --- A choice is derived geometry, not author intent ------------------------- def test_the_pick_never_becomes_the_authors_intent(): - """A spec written back out has to describe the sheet the author asked for, - not the one the engine drew, or reloading it would freeze this run's pick.""" + """Keep an automatic choice out of the serialized model. + + Returns + ------- + None + The choice is on the frame only, not in ``to_dict()`` output. + """ fs, drum = _drum_fed_from(220, 60) fs.layout() assert drum.frame.port_faces == {"feed_1": "N"} @@ -193,6 +384,13 @@ def test_the_pick_never_becomes_the_authors_intent(): def test_laying_the_sheet_out_twice_draws_it_the_same_way(): + """Repeat the same choice on a second layout and route. + + Returns + ------- + None + Both runs select the north face. + """ fs, drum = _drum_fed_from(220, 60) fs.layout() fs.route() @@ -203,8 +401,13 @@ def test_laying_the_sheet_out_twice_draws_it_the_same_way(): def test_a_balloons_pick_survives_being_re_placed_by_the_router(): - """An attached balloon is placed again once its host line is routed, which - replaces its frame, and its frame is where the pick lives.""" + """Keep an attached balloon's choice when routing replaces its frame. + + Returns + ------- + None + The controller's signal output stays on the west face. + """ fs = Flowsheet("loop") feed = fs.add(units.Feed("Feed")).pin(x=60, y=270) fv = fs.add(units.Valve("FV-1", variant="control")).pin(x=260, y=280) @@ -221,13 +424,17 @@ def test_a_balloons_pick_survives_being_re_placed_by_the_router(): assert port_anchor(lic, lic.frame, "sig_out")[2] == "W" -# --- the engine must not draw a contradiction -------------------------------- +# --- Selection never creates a validation error ------------------------------ def test_two_signals_from_the_same_side_do_not_land_on_one_point(): - """Both of a controller's signals come from above, and the cheapest face for - each is the same one. Two live connections on a single point is a hard - validation error, so the second takes the next-cheapest free face.""" + """Give two signals with the same cheapest face separate faces. + + Returns + ------- + None + The controller's ports differ and no ``coincident-ports`` is reported. + """ fs = Flowsheet("stack") fv = fs.add(units.Valve("FV-1", variant="control")).pin(x=300, y=180) feed = fs.add(units.Feed("F")).pin(x=100, y=175) @@ -244,13 +451,17 @@ def test_two_signals_from_the_same_side_do_not_land_on_one_point(): assert [i for i in fs.validate() if i.code == "coincident-ports"] == [] -# --- ordering within the layout run ------------------------------------------ +# --- Order within a layout run ----------------------------------------------- def test_the_label_dodges_the_face_the_engine_chose(): - """Label placement reads the faces connected nozzles occupy, so it has to run - after selection: on the home nozzle this drum's tag would go to the top, - which is exactly where the feed now enters.""" + """Place the label after face selection. + + Returns + ------- + None + The tag moves to the bottom because the feed enters at the top. + """ fs, drum = _drum_fed_from(220, 60) fs.layout() assert drum.frame.label_pos == "bottom" diff --git a/tests/test_layout_refinement.py b/tests/test_layout_refinement.py index df2946d..cfee8ad 100644 --- a/tests/test_layout_refinement.py +++ b/tests/test_layout_refinement.py @@ -1,4 +1,4 @@ -"""Integration checks for publishing a qualified completed drawing.""" +"""Check that default routing publishes only qualified completed drawings.""" from __future__ import annotations @@ -7,7 +7,8 @@ import pytest -from pandid import Flowsheet +from _layout_cases import build as build_biodiesel +from pandid import Flowsheet, GravitySeparator, Product, Reactor from pandid.layout.candidates import generate from pandid.layout.quality import admissible, measure_final from pandid.layout.trials import _evaluate_candidate @@ -21,7 +22,7 @@ "stem", ["10_ethanol_pfd", "16_demineralised_water", "17_stirred_reactor_train"] ) def test_default_route_preserves_the_qualified_settled_drawing(stem: str) -> None: - """The final search preserves the accepted trial and model identities. + """Preserve the accepted trial and model identities through the final search. Parameters ---------- @@ -54,7 +55,7 @@ def test_default_route_preserves_the_qualified_settled_drawing(stem: str) -> Non @pytest.mark.parametrize("stem", ["04_control_loop", "08_from_data", "16_demineralised_water"]) def test_refinement_repeats_from_author_intent_and_fresh_builds(stem: str) -> None: - """Rebuilds make the same choice without using prior derived geometry. + """Repeat the same choice from author intent and from a fresh build. Parameters ---------- @@ -138,7 +139,7 @@ def test_row_trial_does_not_replace_a_better_final_search( @pytest.mark.parametrize("stem", ["04_control_loop", "10_ethanol_pfd"]) def test_route_report_uses_the_published_graph(stem: str) -> None: - """Trial searches do not change the final route-quality report. + """Report route quality from the published graph, not from trials. Parameters ---------- @@ -163,7 +164,7 @@ def test_route_report_uses_the_published_graph(stem: str) -> None: def test_route_report_ignores_an_unpublished_trial_warning( monkeypatch: pytest.MonkeyPatch, ) -> None: - """Speculative warnings do not count as missing final routes. + """Ignore a warning raised only by an unpublished trial. Parameters ---------- @@ -199,7 +200,7 @@ def rejected(fs: Flowsheet) -> bool: def test_route_report_accepts_manual_intermediate_waypoints() -> None: - """A one-point manual ``via`` is a valid routed stream. + """Count a manual route with one waypoint as drawn. Returns ------- @@ -209,3 +210,103 @@ def test_route_report_accepts_manual_intermediate_waypoints() -> None: fs, _ = layout_quality.build("11_ethanol_pid", False) report = route_quality.measure_sheet(fs, "11_ethanol_pid") assert report.undrawn == [] + + +def _sealed_by_alignment(pinned: bool = False) -> tuple[Flowsheet, dict[str, object]]: + """Build a sheet whose row alignment seals a separator's south exit. + + Aligning S-2's feed with R-1's outlet lifts S-2 to 9 px below S-1, inside + the stub of S-1's connected underflow. + + Parameters + ---------- + pinned : bool, optional + Pin every unit at the sealed pixel positions instead of grid ranks. + + Returns + ------- + tuple[Flowsheet, dict[str, object]] + The sheet, with its units and the underflow stream by name. + """ + fs = Flowsheet("Sealed exit") + reactor = fs.add(Reactor("R-1")) + upper = fs.add(GravitySeparator("S-1")) + lower = fs.add(GravitySeparator("S-2")) + drain = fs.add(Product("Drain")) + if pinned: + reactor.pin(x=50, y=50) + upper.pin(x=232, y=56) + lower.pin(x=232, y=185) + drain.pin(x=432, y=91) + else: + reactor.pin(col=0, row=0) + upper.pin(col=1, row=0) + lower.pin(col=1, row=1) + drain.pin(col=2, row=0) + fs.connect(reactor.outlet, lower.feed) + underflow = fs.connect(upper.underflow, drain.inlet) + return fs, {"upper": upper, "lower": lower, "underflow": underflow} + + +def test_default_route_clears_an_exit_sealed_by_row_alignment() -> None: + """Undo the alignment that puts a unit inside a connected nozzle stub. + + Returns + ------- + None + The underflow is routed clear, grid ranks hold, and a second route + and a fresh build draw the same sheet. + """ + fs, parts = _sealed_by_alignment() + fs.layout() + fs.route(DefaultRouter()) + assert parts["underflow"].route.used_fallback + + fs.layout() + fs.route() + upper, lower = parts["upper"].frame, parts["lower"].frame + assert not parts["underflow"].route.used_fallback + assert not measure_final(fs).hard_conflicts + assert lower.y - upper.y_max >= 25 + assert (upper.row, lower.row) == (0, 1) + + fingerprint = _fingerprint(fs) + fs.route() + assert _fingerprint(fs) == fingerprint + fresh, _ = _sealed_by_alignment() + fresh.route() + assert _fingerprint(fresh) == fingerprint + + +def test_exit_repair_leaves_pinned_units_in_place() -> None: + """Keep pinned units where the author put them when an exit is sealed. + + Returns + ------- + None + Both separators stay on their pins and the blocked route is reported. + """ + fs, parts = _sealed_by_alignment(pinned=True) + fs.route() + + upper, lower = parts["upper"].frame, parts["lower"].frame + assert (upper.x, upper.y, lower.x, lower.y) == (232, 56, 232, 185) + assert parts["underflow"].route.used_fallback + assert any(issue.code == "route-crosses-unit" for issue in fs.validate()) + + +def test_default_route_clears_the_unpinned_biodiesel_sheet() -> None: + """Route the unpinned biodiesel sheet without a line through equipment. + + Returns + ------- + None + No stream uses the fallback and no hard conflict remains. + """ + fs, _ = build_biodiesel(pinned=False) + fs.route() + + quality = measure_final(fs) + assert not any(stream.route.used_fallback for stream in fs.streams) + assert not quality.hard_conflicts and not any(quality.hard) + assert not any(issue.code == "route-crosses-unit" for issue in fs.validate()) diff --git a/tests/test_routing.py b/tests/test_routing.py index c279069..69689ff 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -1,3 +1,5 @@ +"""Check obstacle geometry, path search, crossing pricing, and the default router.""" + import heapq from types import SimpleNamespace from typing import cast @@ -29,7 +31,7 @@ def test_router_uses_a_reachable_lane_before_a_blocked_projection() -> None: graph = VisibilityGraph(fs) assert graph.port_projs[("F", "outlet")] not in graph.nodes - fs.route(DefaultRouter(recover_exits=True)) + fs.route(DefaultRouter()) assert stream.route is not None and not stream.route.used_fallback assert feed.frame is not None and product.frame is not None and blocker.frame is not None @@ -41,6 +43,25 @@ def test_router_uses_a_reachable_lane_before_a_blocked_projection() -> None: assert not any(issue.code == "route-crosses-unit" for issue in fs.validate()) +def test_router_without_recovery_searches_only_the_nominal_projection() -> None: + """Keep the nominal-projection search available as an opt-out. + + Returns + ------- + None + The blocked projection draws the fallback when recovery is disabled. + """ + fs = Flowsheet("nominal escape") + feed = fs.add(Feed("F")).pin(x=60, y=100) + product = fs.add(Product("P")).pin(x=400, y=100) + fs.add(Pump("Blocker", width=44, height=44, label_pos="bottom")).pin(x=83, y=78) + stream = fs.connect(feed.outlet, product.inlet) + fs.layout() + fs.route(DefaultRouter(recover_exits=False)) + + assert stream.route is not None and stream.route.used_fallback + + def test_router_reports_a_sealed_fixed_nozzle() -> None: """Keep a fully blocked outward nozzle unresolved. @@ -55,7 +76,7 @@ def test_router_reports_a_sealed_fixed_nozzle() -> None: blocker = fs.add(Pump("Blocker", width=44, height=44, label_pos="bottom")).pin(x=60, y=78) stream = fs.connect(feed.outlet, product.inlet) fs.layout() - fs.route(DefaultRouter(recover_exits=True)) + fs.route(DefaultRouter()) assert stream.route is not None and stream.route.used_fallback assert feed.frame is not None and product.frame is not None and blocker.frame is not None @@ -81,66 +102,75 @@ def test_router_does_not_call_a_blocked_stub_successful() -> None: fs.add(Pump("Blocker", width=12, height=40, label_pos="center")).pin(x=68, y=80) stream = fs.connect(feed.outlet, product.inlet) fs.layout() - fs.route(DefaultRouter(recover_exits=True)) + fs.route(DefaultRouter()) assert stream.route is not None and stream.route.used_fallback assert any(issue.code == "route-crosses-unit" for issue in fs.validate()) def test_rect_intersection(): + """Distinguish strict containment from segment contact. + + Returns + ------- + None + An edge point is not contained; an edge segment intersects. + """ r = Rect(10, 20, 10, 20) - # Strictly inside + # Contain a point strictly inside. assert r.contains(15, 15) - # Edge is not strict containment + # Exclude a point on the edge. assert not r.contains(10, 15) - # Segment crossing through + # Intersect a segment that passes through. assert r.intersects_segment(5, 15, 25, 15) - # Segment on the edge + # Intersect a segment that lies on the edge. assert r.intersects_segment(10, 5, 10, 25) - # Segment completely outside + # Miss a segment that lies outside. assert not r.intersects_segment(5, 5, 25, 5) def test_clear_gaps_closes_the_run_a_span_reaches_into(): + """Close each lane gap that an obstacle span reaches into. + + Returns + ------- + None + Gap ``k`` lies between ``lane[k]`` and ``lane[k + 1]``. + """ lane = [0.0, 10.0, 20.0, 30.0, 40.0] - # Gap k lies between lane[k] and lane[k + 1]. A span reaching into the two - # middle gaps closes both and leaves the ones either side open. + # Close both middle gaps for a span that reaches into them. assert clear_gaps(lane, [(15.0, 25.0)]) == [True, False, False, True] - # Touching a node is not reaching past it: the test is strict on both ends. + # Leave a gap open when the span only touches its node. assert clear_gaps(lane, [(20.0, 30.0)]) == [True, True, False, True] assert clear_gaps(lane, [(0.0, 10.0)]) == [False, True, True, True] - # Spans clear of the lane's ends, and spans over the whole of it. + # Handle spans beyond the lane and spans covering all of it. assert clear_gaps(lane, [(50.0, 60.0)]) == [True] * 4 assert clear_gaps(lane, [(-5.0, 45.0)]) == [False] * 4 assert clear_gaps(lane, []) == [True] * 4 - # A lane with nothing to travel between has no gaps to report on. + # Report no gaps for a lane with fewer than two coordinates. assert clear_gaps([7.0], [(0.0, 10.0)]) == [] assert clear_gaps([], [(0.0, 10.0)]) == [] def test_the_obstacle_index_sees_what_an_exhaustive_scan_sees(): - """The graph narrows each test to the obstacles that can be in the way. - - It indexes them against the lane grid instead of scanning the whole list - for every point and every candidate edge, which is what routing spent - nearly all of its time on. The narrowing has to be exactly that: the same - nodes, and the same adjacency in the same order, as ``Rect.contains`` and - ``Rect.intersects_segment`` give when asked about every obstacle. Order is - part of it -- A* breaks equal-cost ties by the order neighbours were - inserted -- so a re-ordered adjacency list re-routes the sheet. - - The units are pinned rather than laid out, with labels on all four sides - and positions chosen so lanes land on obstacle edges: containment is strict - and the segment test is not, so an index that confused the two would pass - on a sheet whose lanes all fall clear. + """Match the indexed graph to an exhaustive obstacle scan. + + Nodes, edges, and neighbour order must all agree, because the search + breaks cost ties by neighbour order. Units are pinned with labels on all + four sides so lanes fall on obstacle edges, where strict containment and + segment contact differ. + + Returns + ------- + None + ``graph.nodes`` and ``graph.edges`` equal the scanned results. """ fs = Flowsheet("obstacle index") feed = fs.add(Feed("Raw Feed")).pin(x=40, y=200) pump = fs.add(Pump("P-101", label_pos="bottom")).pin(x=180, y=180) hx = fs.add(HeatExchanger("E-101", width=120, height=60, label_pos="left")).pin(x=320, y=170) - # x=440 is E-101's right edge (320 + 120) and y=290 its label band's, so the - # lanes those obstacles put on the grid fall on this vessel's own. + # Align the vessel with E-101's right edge (x=440) and label band (y=290). drum = fs.add(Vessel("V-101", width=90, height=140, label_pos="right")).pin(x=440, y=290) valve = fs.add(Valve("FV-101", label_pos="top")).pin(x=620, y=200) prod = fs.add(Product("To Unit 200")).pin(x=760, y=200) @@ -153,7 +183,7 @@ def test_the_obstacle_index_sees_what_an_exhaustive_scan_sees(): fs.layout() graph = VisibilityGraph(fs, margin=15.0) - assert len(graph.obstacles) >= 8 and len(graph.nodes) >= 500 # not a vacuous corpus + assert len(graph.obstacles) >= 8 and len(graph.nodes) >= 500 # Require a non-trivial graph. scanned_nodes = { (x, y) @@ -182,6 +212,13 @@ def test_the_obstacle_index_sees_what_an_exhaustive_scan_sees(): def test_outward_dir(): + """Name the box edge nearest a port coordinate. + + Returns + ------- + None + Each edge midpoint maps to its own face. + """ assert get_outward_dir(0, 50, 100, 100) == "W" assert get_outward_dir(100, 50, 100, 100) == "E" assert get_outward_dir(50, 0, 100, 100) == "N" @@ -189,19 +226,21 @@ def test_outward_dir(): def test_router_integration(): + """Route around a vessel that blocks both fallback corners. + + Returns + ------- + None + The route joins both nozzles, is orthogonal, turns, and clears + every obstacle. + """ fs = Flowsheet("test") f = fs.add(Feed("F")) p = fs.add(Product("P")) - # A column standing between the two, tall enough to reach both escape - # lanes. Without it the run the search finds and the L the router falls - # back to are the same five points, so nothing asserted about the drawn - # route could tell a working search from one that returned nothing: the - # sheet has to make the two differ before the assertions below mean - # anything. It spans both nozzle elevations, so *both* L's cross it and - # only a searched route gets past. + # Block both L-shaped fallbacks so only a searched route can pass. wall = fs.add(Vessel("V-101", width=50, height=250)) - # Force placement to bypass layout engine + # Pin every unit to fix the geometry. f.pin(x=0, y=0) p.pin(x=200, y=200) wall.pin(x=100, y=0) @@ -211,11 +250,7 @@ def test_router_integration(): assert s.route is not None wp = s.route.waypoints - # Asserting only that the list exists passes on a router that found nothing - # at all, so say what the run has to be: it starts on one nozzle, ends on - # the other, every segment lies on an axis, and nothing but the two boxes - # it is tied to is in its way. Diagonally offset ports cannot be joined by - # one straight line, so it also has to turn. + # Check the endpoints, orthogonality, and that the route turns. graph = VisibilityGraph(fs, margin=15.0) assert wp[0] == pytest.approx(graph.port_anchors[("F", "outlet")]) assert wp[-1] == pytest.approx(graph.port_anchors[("P", "inlet")]) @@ -228,25 +263,20 @@ def test_router_integration(): } assert len(turns) >= 2, f"expected an orthogonal step, got {wp}" - # No exemption for the end segments: a nozzle sits on its own box's edge - # and the stub off it leaves along the outward normal, which the segment - # test reads as touching rather than crossing. Every segment of this run, - # first and last included, is clear of every box on the sheet. + # Check every segment, including both nozzle stubs, against every box. for a, b in zip(wp, wp[1:]): hit = [o for o in graph.obstacles if o.intersects_segment(a[0], a[1], b[0], b[1])] assert not hit, f"segment {a}->{b} of {wp} runs through {hit}" def test_no_obstacle_intersection(): - """The searched run goes round what is in its way; the fallback L does not. - - The sheet needs something in the way for that to be a distinction. With - only the two ends on it, the route is four points, every one of the - exemptions below applies to every segment, and the fallback L is the - route the search finds anyway -- so the assertion was never reached and - could not have failed if it had been. The drum between them is what makes - the two differ: it stands across both nozzle elevations, so *both* L's - cross it and only a route that has been searched for gets past. + """Route around a drum that blocks both fallback corners. + + Returns + ------- + None + No segment crosses an obstacle other than the end units' own + boxes and labels on the two nozzle stubs. """ from pandid.units import Separator, Compressor @@ -255,7 +285,7 @@ def test_no_obstacle_intersection(): c = fs.add(Compressor("C1")) drum = fs.add(Vessel("V-102", width=60, height=120)) - # Place them such that a straight line intersects + # Place the drum across the straight line between the two nozzles. v.pin(x=0, y=100) c.pin(x=200, y=100) drum.pin(x=90, y=40) @@ -265,19 +295,13 @@ def test_no_obstacle_intersection(): graph = VisibilityGraph(fs) - # A route with no waypoints used to fall through to a bare source-to-dest - # straight line and be checked against the obstacles as though the router - # had drawn it, so a router that found nothing passed this test. + # Require a drawn route before checking it against obstacles. assert s.route is not None and s.route.waypoints, "V1 -> C1 was not routed" pts = s.route.waypoints assert pts[0] == pytest.approx(graph.port_anchors[("V1", "vapor")]) assert pts[-1] == pytest.approx(graph.port_anchors[("C1", "suction")]) - # The stub off a nozzle is the one segment that may touch something: the - # label band sits between the nozzle and the sheet, so a run leaving a - # top nozzle on a top-labelled unit crosses its own label whatever it - # does next. That is the *first* segment and the *last*, not the first - # two and the last two -- which on a four-point route is all of them. + # Exempt only the first and last segments from their own unit and label. stubs = {0, len(pts) - 2} for i, ((x1, y1), (x2, y2)) in enumerate(zip(pts, pts[1:])): for obs in graph.obstacles: @@ -294,63 +318,63 @@ def test_no_obstacle_intersection(): def test_the_fallback_l_is_checked_against_the_obstacles(): - """The L drawn when the search finds nothing is a choice, so make it one. + """Choose the fallback corner that crosses fewer obstacles. - Both corner orders reach the same two projections. Only one of them may - be clear, and the router used to take the across-first order whatever was - standing on it -- through a vessel as readily as through open sheet. + Returns + ------- + None + A clear corner wins, and across-first wins when both are equal. """ start, start_proj = (0.0, 0.0), (25.0, 0.0) goal_proj, goal = (100.0, 100.0), (100.0, 125.0) across = [start, start_proj, (100.0, 0.0), goal_proj, goal] down = [start, start_proj, (25.0, 100.0), goal_proj, goal] - # Nothing in the way: the across-first order, exactly as before. + # Take the across-first order when nothing is in the way. assert _fallback_path(start, start_proj, goal_proj, goal, []) == across - # A box on the across-first leg, and the other order goes round it. + # Take the other order when a box sits on the across-first leg. on_across = Rect(40.0, 60.0, -10.0, 10.0) assert _fallback_path(start, start_proj, goal_proj, goal, [on_across]) == down - # A box on each: the search has already said the grid has no way through, - # so the least bad L is still drawn -- and ``validate()`` reports it under - # ``route-crosses-unit`` rather than the sheet coming out with a gap in it. + # Draw the across-first order when both legs are blocked. on_down = Rect(40.0, 60.0, 90.0, 110.0) assert _fallback_path(start, start_proj, goal_proj, goal, [on_across, on_down]) == across def test_the_fallback_l_also_breaks_an_obstacle_tie_on_a_drawn_crossing(): - # Obstacles alone are not the only thing a fallback L can be checked - # against: two candidates can tie on obstacles (none in the way of - # either, as here) while one crosses a line an earlier stream already - # drew and the other does not. Left unchecked, ties always fell to - # across-first regardless. + """Break an obstacle tie by the number of recorded lines crossed. + + Returns + ------- + None + The corner avoiding a recorded line wins unless an obstacle blocks it. + """ start, start_proj = (0.0, 0.0), (25.0, 0.0) goal_proj, goal = (100.0, 100.0), (100.0, 125.0) across = [start, start_proj, (100.0, 0.0), goal_proj, goal] down = [start, start_proj, (25.0, 100.0), goal_proj, goal] - # No crossing_index: across-first, same tie-break as ever. + # Take the across-first order without a crossing index. assert _fallback_path(start, start_proj, goal_proj, goal, []) == across - # An earlier stream's vertical line at x=50, which only the - # across-first leg (y=0, x from 25 to 100) passes through. + # Record a vertical line at x=50 that only the across-first leg crosses. index = CrossingIndex() index.v[50.0] = [(-10.0, 10.0)] assert _fallback_path(start, start_proj, goal_proj, goal, [], index) == down - # Obstacles still take priority over lines: a box on the clear (down) - # leg sends it back to across even though across now crosses a line. + # Rank an obstacle on the down leg above the recorded crossing. on_down = Rect(10.0, 40.0, 90.0, 110.0) assert _fallback_path(start, start_proj, goal_proj, goal, [on_down], index) == across def test_the_fallback_drops_a_corner_that_repeats_the_projection(): - """Two projections in one column leave no corner to turn on. + """Omit a corner that coincides with an escape node (#282). - Kept from #282: a repeated point survives the caller's simplifier, which - never drops a projection, and reaches the separation pass as a zero-length - run on a track the stream does not occupy. + Returns + ------- + None + Two projections in one column give a path without repeated points. """ start, start_proj = (0.0, 0.0), (25.0, 0.0) goal_proj, goal = (25.0, 100.0), (25.0, 125.0) @@ -360,7 +384,18 @@ def test_the_fallback_drops_a_corner_that_repeats_the_projection(): def _straight_run(bad=None): - """Feed to product through a heat exchanger, optionally misplaced.""" + """Build a feed, heat exchanger, and product in one row. + + Parameters + ---------- + bad : float or None, optional + Replacement x coordinate for the heat exchanger. + + Returns + ------- + Flowsheet + Sheet with all three units pinned. + """ fs = Flowsheet("Non-finite") feed = fs.add(Feed("Raw Feed")).pin(x=0, y=100) hx = fs.add(HeatExchanger("E-101")) @@ -373,28 +408,29 @@ def _straight_run(bad=None): @pytest.mark.parametrize("bad", [float("nan"), float("inf"), float("-inf")]) def test_a_non_finite_placement_is_refused_rather_than_routed_for_ever(bad): - """A coordinate that does not compare used to hang the render outright. + """Reject a non-finite coordinate before the path search starts. - A* terminates because ``visited[state] <= g`` settles each state at most - once, and that comparison is false for every NaN: nothing was settled, - every state re-expanded, and the queue's paths grew until the process - died. ``route()`` never came back, so neither did ``to_svg()``. + Parameters + ---------- + bad : float + Non-finite x coordinate given to the heat exchanger. - This test cannot itself hang: the sheet is refused before the graph is - built, which is also the last point that can name the unit at fault. + Returns + ------- + None + ``route()`` raises and names the unit. """ with pytest.raises(ValueError, match="E-101 has a non-finite x="): _straight_run(bad).route() def test_the_search_is_bounded_by_the_size_of_the_graph(): - """Termination may not rest on the coordinates being well behaved. + """Stop a search that exceeds its expansion budget. - The endpoint check catches what we know how to name; the expansion ceiling - is what makes termination unconditional, so a graph poisoned somewhere - nobody thought to look raises instead of spinning. Driven by taking the - budget away rather than by finding a pathological sheet, so the test costs - what any other does. + Returns + ------- + None + A zero budget raises, and the default budget routes the same sheet. """ fs = _straight_run() fs.layout() @@ -406,13 +442,20 @@ def test_the_search_is_bounded_by_the_size_of_the_graph(): finally: astar.MAX_EXPANSIONS_PER_NODE, astar.MIN_EXPANSION_BUDGET = original - # The same sheet routes with the budget it is actually given. + # Route the same sheet with the default budget. fs = _straight_run() fs.route() assert all(s.route and s.route.waypoints for s in fs.streams) def test_find_path_names_the_endpoint_it_cannot_search_from(): + """Reject a non-finite start or goal by name. + + Returns + ------- + None + Each endpoint raises its own ``ValueError``. + """ fs = _straight_run() fs.layout() graph = VisibilityGraph(fs, margin=15.0) @@ -424,77 +467,83 @@ def test_find_path_names_the_endpoint_it_cannot_search_from(): # --------------------------------------------------------------------------- -# #425: charging a drawn crossing. +# Pricing drawn crossings (#425). # --------------------------------------------------------------------------- def test_committed_segments_merges_a_straight_run_into_one_span(): - # 602->615->640 is one drawn line, not two: a router-committed path keeps - # a collinear waypoint the simplifier would later drop (a stub's own - # projection point, kept verbatim -- see ``DefaultRouter.route()``'s - # ``simplified`` comment), so the axis a later crossing check needs has - # to be read off *segments*, not off the raw waypoint count. + """Merge collinear waypoints into one segment. + + Returns + ------- + None + Three points on one row give a single horizontal span. + """ path = [(602.0, 450.0), (615.0, 450.0), (640.0, 450.0)] assert committed_segments(path) == [("h", 450.0, 602.0, 640.0)] def test_committed_segments_merges_across_a_repeated_point_on_the_same_axis(): - # Two horizontal segments meeting at a duplicate waypoint (an escape - # stub ``share_escape_room`` shrank to nothing, say) are one continuous - # drawn line, not two that merely touch: a crossing exactly at the - # touch point (10, 0) has to read as interior to the recorded span, not - # as the shared endpoint of two separate ones. - # - # A different-axis pair either side of the repeat would pass this same - # assertion even if the repeat were never special-cased at all (an axis - # change already ends a run on its own), so it proves nothing about the - # repeat specifically -- this needs same-axis segments on both sides. + """Merge two same-axis segments that meet at a repeated waypoint. + + Returns + ------- + None + The repeated point is interior to a single span. + """ path = [(0.0, 0.0), (10.0, 0.0), (10.0, 0.0), (20.0, 0.0)] assert committed_segments(path) == [("h", 0.0, 0.0, 20.0)] def test_crossing_index_is_strict_interior_only(): + """Count a crossing only strictly inside a recorded span. + + Returns + ------- + None + Endpoints, other lanes, and the wrong axis do not count. + """ index = CrossingIndex() - index.record([(0.0, 0.0), (100.0, 0.0)]) # one horizontal span, y=0, x in [0, 100] + index.record([(0.0, 0.0), (100.0, 0.0)]) # Record y=0 for x in [0, 100]. - # Strictly inside: a vertical run through (50, 0), continuing past it on - # both sides, properly crosses the recorded line. + # Count a vertical run through the span's interior. assert index.crosses((50.0, 0.0), "v") - # At the recorded span's own endpoint: a T-junction, not a crossing -- - # the same distinction ``route_quality.py``'s ``crossing_point`` draws - # between a proper crossing and a branch tee. + # Treat a span endpoint as a tee, not a crossing. assert not index.crosses((100.0, 0.0), "v") assert not index.crosses((0.0, 0.0), "v") - # Off the recorded lane entirely. + # Ignore a point off the recorded lane. assert not index.crosses((50.0, 5.0), "v") - # Wrong axis: querying with "h" asks whether something crosses a - # *horizontal* run through this node, which looks in ``.v``, not ``.h``. + # Ignore the other axis, which reads vertical spans. assert not index.crosses((50.0, 0.0), "h") def test_crossing_index_crosses_counts_rather_than_answers_yes_or_no(): - # Two different earlier streams can each cross the same point (each - # drew its own span, and both happen to cover it) -- a route through - # there crosses both, not one, and ``crosses`` has to say so rather than - # collapsing "how many" to "any at all". + """Return the number of recorded spans crossed at a point. + + Returns + ------- + None + Two streams on one line count as two crossings; none counts as zero. + """ index = CrossingIndex() index.record([(0.0, 0.0), (100.0, 0.0)]) assert index.crosses((50.0, 0.0), "v") == 1 - index.record([(0.0, 0.0), (100.0, 0.0)]) # a second stream, same line + index.record([(0.0, 0.0), (100.0, 0.0)]) # Record a second stream on the line. assert index.crosses((50.0, 0.0), "v") == 2 - # Still falsy at zero, so a caller that only wants to know whether - # anything crosses at all keeps working unchanged. + # Return a falsy zero when nothing is crossed. assert not index.crosses((50.0, 5.0), "v") def test_a_route_that_crosses_two_streams_costs_twice(): - # The g-cost `find_path` actually settles the returned route at (see - # ``_final_g``) has to reflect crossing two earlier streams as twice - # the charge of crossing one -- not the same charge either way, which - # is what a boolean-valued ``crosses`` gave: a route crossing a second - # stream at an already-crossed point cost nothing more for it. + """Charge one crossing penalty per stream crossed. + + Returns + ------- + None + The route cost rises by one penalty, then two. + """ nodes = {(0.0, 0.0), (40.0, 0.0)} edges: dict = {n: [] for n in nodes} edges[(0.0, 0.0)].append((40.0, 0.0)) @@ -508,7 +557,7 @@ def test_a_route_that_crosses_two_streams_costs_twice(): crossing_one = _final_g(graph, (0.0, 0.0), (40.0, 0.0), None, "W", crossing_index=one) two = CrossingIndex() - two.v[40.0] = [(-10.0, 10.0), (-10.0, 10.0)] # a second earlier stream, same span + two.v[40.0] = [(-10.0, 10.0), (-10.0, 10.0)] # Add a second stream on the span. crossing_two = _final_g(graph, (0.0, 0.0), (40.0, 0.0), None, "W", crossing_index=two) assert crossing_one == pytest.approx(clear + astar.CROSSING_PENALTY) @@ -516,36 +565,41 @@ def test_a_route_that_crosses_two_streams_costs_twice(): def test_crossing_index_protects_a_stubs_whole_span_not_just_its_ends(): - # The bug #425 shipped and then caught on its own corpus: a fixed - # anchor->projection stub (or the shared mid-point ``share_escape_room`` - # gives two nozzles too close for both stand-offs) is recorded as *one* - # long span from a two-point path, with no graph node anywhere in its - # interior. A later stream sliding onto any lane inside that span -- - # 617 here, never itself a point on the recorded path -- still has to - # see the crossing; an index keyed on the recorded path's own points - # would have missed it, which is exactly what let - # ``17_stirred_reactor_train`` gain a crossing instead of losing one the - # first time this was tried. + """Count a crossing anywhere inside a span recorded from two points. + + Returns + ------- + None + A lane inside the span crosses although it is not a recorded point. + """ index = CrossingIndex() index.record([(602.0, 450.0), (640.0, 450.0)]) assert index.crosses((617.0, 450.0), "v") def _tie_graph() -> VisibilityGraph: - """Two routes from (0, 0) to (40, 0), tied on both bends and length: - north via (0, 10)->(20, 10)->(40, 10), south via (0, -10)->(20, - -10)->(40, -10). No direct east edge at y=0 exists (an obstacle would - put one there on a real sheet), so the search must pick one detour or - the other. Built by hand, not through ``Flowsheet``/``VisibilityGraph``, - so the tie is exact and nothing about placement can perturb it -- - ``find_path`` only ever reads ``graph.nodes``, ``graph.edges`` and - ``graph.recycle_y``, so a bare stand-in carries everything it needs; the - cast is only to tell the type checker that too. + """Build two routes from (0, 0) to (40, 0) tied on bends and length. + + One route runs north through (20, 10) and one south through (20, -10). + No direct edge exists. The graph is built by hand so the tie is exact. + + Returns + ------- + VisibilityGraph + Stand-in carrying the ``nodes``, ``edges``, and ``recycle_y`` that + ``find_path`` reads. """ nodes = {(0, 0), (40, 0), (0, 10), (20, 10), (40, 10), (0, -10), (20, -10), (40, -10)} edges: dict = {n: [] for n in nodes} def link(a, b): + """Join two nodes in both directions. + + Parameters + ---------- + a, b : tuple[float, float] + Graph nodes. + """ edges[a].append(b) edges[b].append(a) @@ -561,20 +615,23 @@ def link(a, b): def test_a_bend_and_length_tie_breaks_towards_the_route_that_does_not_cross(): + """Prefer the tied route that crosses no recorded line. + + Returns + ------- + None + The search avoids whichever side carries the recorded span. + """ graph = _tie_graph() - # An earlier stream's vertical run at x=20, y in [0, 20]: the north - # detour's horizontal leg passes straight through (20, 10), strictly - # inside that span, so it crosses; the south leg, through (20, -10), - # does not. + # Record a vertical run that only the north route crosses. crosses_north = CrossingIndex() crosses_north.v[20] = [(0.0, 20.0)] south = find_path(graph, (0, 0), (40, 0), "E", None, crossing_index=crosses_north) assert (20, 10) not in south, f"took the crossing route: {south}" assert (20, -10) in south, f"did not take the free alternative: {south}" - # Symmetric the other way: nothing about this favours south by - # construction, only whichever lane was actually marked crossed. + # Record the mirrored run to confirm the graph favours neither side. crosses_south = CrossingIndex() crosses_south.v[20] = [(-20.0, 0.0)] north = find_path(graph, (0, 0), (40, 0), "E", None, crossing_index=crosses_south) @@ -583,13 +640,16 @@ def test_a_bend_and_length_tie_breaks_towards_the_route_that_does_not_cross(): def test_a_crossing_is_kept_rather_than_bought_off_with_two_more_bends(): - # North (3 bends, length 60) is charged for a crossing; south is clear - # but costs two more bends to reach (5 bends, length 100) by way of an - # extra jog. CROSSING_PENALTY (20) sits two orders of magnitude under - # BEND_PENALTY (500), so paying it must always be cheaper than buying a - # bend to avoid it -- the corpus's own "costly" crossings, which needed - # 8px to 1589px to clear, are the real-world version of this same - # inequality holding. + """Accept a crossing rather than add two bends to avoid it. + + The north route has 3 bends and one crossing. The south route is clear + but has 5 bends. ``CROSSING_PENALTY`` is far below ``BEND_PENALTY``. + + Returns + ------- + None + The search takes the north route. + """ nodes = { (0, 0), (40, 0), @@ -604,6 +664,13 @@ def test_a_crossing_is_kept_rather_than_bought_off_with_two_more_bends(): edges: dict = {n: [] for n in nodes} def link(a, b): + """Join two nodes in both directions. + + Parameters + ---------- + a, b : tuple[float, float] + Graph nodes. + """ edges[a].append(b) edges[b].append(a) @@ -619,19 +686,30 @@ def link(a, b): graph = cast(VisibilityGraph, SimpleNamespace(nodes=nodes, edges=edges, recycle_y=[])) index = CrossingIndex() - index.v[20] = [(0.0, 20.0)] # crosses north's (20, 10) pass-through + index.v[20] = [(0.0, 20.0)] # Cross the north route at (20, 10). routed = find_path(graph, (0, 0), (40, 0), "E", None, crossing_index=index) assert (20, 10) in routed, f"detoured two extra bends to dodge a crossing: {routed}" def test_a_forced_crossing_still_routes(): - # No alternative at all -- the north lane above, alone. Charging a - # crossing must never turn a route the search would otherwise have - # found into no route. + """Find the only route although it crosses a recorded line. + + Returns + ------- + None + The crossing charge never removes the single available route. + """ nodes = {(0, 0), (40, 0), (0, 10), (20, 10), (40, 10)} edges: dict = {n: [] for n in nodes} def link(a, b): + """Join two nodes in both directions. + + Parameters + ---------- + a, b : tuple[float, float] + Graph nodes. + """ edges[a].append(b) edges[b].append(a) @@ -648,19 +726,44 @@ def link(a, b): def _final_g(graph, start, goal, start_dir, goal_dir, crossing_index=None): - """The g-cost of the state ``find_path`` actually returns on -- the last - thing popped, since a search returns the instant it pops a state at - ``goal`` rather than pushing anything further. Reading cost this way, - from the search's own internals, is what a *forced* graph (exactly one - route through it) needs: with no alternative to switch to, the returned - path is identical whether or not something is charged along it, so the - only way to see a charge landing at all is to read the number it landed - on, not which route won. + """Return the cost of the state ``find_path`` stops on. + + A graph with one route returns the same path whether or not a crossing + is charged, so the charge is read from the cost of the last popped + state. + + Parameters + ---------- + graph : VisibilityGraph + Search graph. + start, goal : tuple[float, float] + Search endpoints. + start_dir, goal_dir : str or None + Required endpoint directions. + crossing_index : CrossingIndex or None, optional + Routes already drawn. + + Returns + ------- + float + Accumulated cost of the returned route. """ original = heapq.heappop popped: list = [] def logging_pop(heap): + """Pop one search state and keep a copy. + + Parameters + ---------- + heap : list + Search priority queue. + + Returns + ------- + object + Item removed by the original heap operation. + """ item = original(heap) popped.append(item) return item @@ -675,21 +778,17 @@ def logging_pop(heap): def test_a_terminal_crossing_at_the_goal_itself_is_priced(): - # A path this search returns on reaches ``goal`` and stops -- the ``if - # current == goal: return path`` at the top of the loop -- without ever - # *expanding* it, which is the only place the ordinary per-node check - # (on ``current``, above) ever fires. So a route that arrives at ``goal`` - # already travelling the direction the caller's fixed goal_proj->port - # stub continues in (``OPPOSITE[goal_dir]``) is a strict-interior point - # of the drawn line exactly like any other, and used to cross for free. - # - # A single-route graph -- (0, 0) to goal (40, 0), goal_dir "W", one hop - # -- so there is no alternative arrival for the search to switch to; the - # only way to see the charge is in the cost the returned route settles - # at, read via ``_final_g`` rather than by which of two routes won (see - # ``test_a_crossing_at_the_terminal_is_never_worth_a_bend`` below for - # why a decision-flip test at the terminal specifically cannot be built - # fairly). + """Charge a crossing that lies on the goal node. + + The search returns on reaching the goal without expanding it. An + arrival that continues into the fixed goal stub makes the goal an + interior point of the drawn line. + + Returns + ------- + None + The one-route graph costs one penalty more with the crossing. + """ nodes = {(0.0, 0.0), (40.0, 0.0)} edges: dict = {n: [] for n in nodes} edges[(0.0, 0.0)].append((40.0, 0.0)) @@ -699,52 +798,52 @@ def test_a_terminal_crossing_at_the_goal_itself_is_priced(): g_clear = _final_g(graph, (0.0, 0.0), (40.0, 0.0), None, "W") index = CrossingIndex() - index.v[40.0] = [(-10.0, 10.0)] # goal (40, 0) sits strictly inside it + index.v[40.0] = [(-10.0, 10.0)] # Place the goal strictly inside the span. g_crossed = _final_g(graph, (0.0, 0.0), (40.0, 0.0), None, "W", crossing_index=index) assert g_crossed == pytest.approx(g_clear + astar.CROSSING_PENALTY) def test_a_crossing_at_the_terminal_is_never_worth_a_bend(): - # The one place a bend at the terminal can hide: an arrival that does - # not match the fixed goal_proj->port stub's own direction - # (``OPPOSITE[goal_dir]``) draws a real bend there -- the join between - # this search's last segment and that stub -- which this search never - # itself expands or prices, because the stub is added by the caller - # after this function returns. Charging the straight arrival for a - # crossing while leaving that side approach's bend unpriced would make - # a bend look free next to a 10px charge, which is exactly backwards. - # - # Perfectly symmetric square, (0, -20) to goal_proj (20, 0), goal_dir - # "W": one route arrives via (0, 0) heading east -- straight, matching - # the stub, chargeable if it crosses something; the other via (20, -20) - # heading north -- a side approach whose own hidden bend must now cost - # as much as any other. Both cost the same length (40) and the same one - # counted bend before this fix; the difference the fix adds is entirely - # in the north route's own uncounted turn. + """Price the bend a side arrival makes with the goal stub. + + Two routes reach the goal with equal length and counted bends. One + arrives in line with the stub and crosses a recorded line; the other + arrives from the side, which adds a bend at the stub. + + Returns + ------- + None + The in-line arrival wins, and without the index both remain tied. + """ nodes = {(0.0, -20.0), (0.0, 0.0), (20.0, 0.0), (20.0, -20.0)} edges: dict = {n: [] for n in nodes} def link(a, b): + """Join two nodes in both directions. + + Parameters + ---------- + a, b : tuple[float, float] + Graph nodes. + """ edges[a].append(b) edges[b].append(a) link((0.0, -20.0), (0.0, 0.0)) - link((0.0, 0.0), (20.0, 0.0)) # arrives at goal_proj heading east (straight) + link((0.0, 0.0), (20.0, 0.0)) # Arrive heading east, in line with the stub. link((0.0, -20.0), (20.0, -20.0)) - link((20.0, -20.0), (20.0, 0.0)) # arrives at goal_proj heading north (side) + link((20.0, -20.0), (20.0, 0.0)) # Arrive heading north, from the side. graph = cast(VisibilityGraph, SimpleNamespace(nodes=nodes, edges=edges, recycle_y=[])) index = CrossingIndex() - index.v[20.0] = [(-30.0, 30.0)] # goal_proj (20, 0) sits strictly inside it + index.v[20.0] = [(-30.0, 30.0)] # Place the goal strictly inside the span. routed = find_path(graph, (0.0, -20.0), (20.0, 0.0), None, "W", crossing_index=index) assert (0.0, 0.0) in routed, f"bought a bend to dodge a 10px crossing: {routed}" assert (20.0, -20.0) not in routed, f"bought a bend to dodge a 10px crossing: {routed}" - # Without the index the two arrivals are a genuine tie (confirms the - # graph above is not secretly biased towards the straight arrival - # already) -- which one wins is arbitrary and not asserted on. + # Confirm the graph is unbiased: either route may win without the index. tied = find_path(graph, (0.0, -20.0), (20.0, 0.0), None, "W") assert tied in ( [(0.0, -20.0), (0.0, 0.0), (20.0, 0.0)], @@ -753,6 +852,13 @@ def link(a, b): def test_a_forced_terminal_crossing_still_routes(): + """Find the only route although its goal lies on a recorded line. + + Returns + ------- + None + The direct route is returned. + """ nodes = {(0, 0), (40, 0)} edges: dict = {n: [] for n in nodes} edges[(0, 0)].append((40, 0)) @@ -766,15 +872,25 @@ def test_a_forced_terminal_crossing_still_routes(): def test_committed_segments_refuses_a_diagonal_pair(): - # A router-drawn path is orthogonal by construction; a diagonal pair - # reaching here is a caller bug, and guessing an axis for it (whichever - # of x or y the check happens to test first) would silently misreport - # geometry rather than say so. + """Reject a diagonal segment instead of guessing its axis. + + Returns + ------- + None + ``committed_segments`` raises ``ValueError``. + """ with pytest.raises(ValueError, match="diagonal"): committed_segments([(0.0, 0.0), (10.0, 10.0)]) def test_crossing_index_crosses_refuses_an_invalid_axis(): + """Reject an axis other than ``"h"`` or ``"v"``. + + Returns + ------- + None + ``crosses`` raises ``ValueError``. + """ index = CrossingIndex() index.record([(0.0, 0.0), (100.0, 0.0)]) with pytest.raises(ValueError, match="axis"): @@ -782,14 +898,20 @@ def test_crossing_index_crosses_refuses_an_invalid_axis(): def _obstacle_bypass_tie(fs): - """F -> P around a centered obstacle, added to *fs* last, with the - north and south bypass an exact tie: nothing yet gives the search a - reason to prefer one over the other, so which one it draws is the free - choice a stream connected earlier can load one side of. - - ``label_pos="center"`` matters: the vessel's tag block is otherwise - drawn above it, which makes the obstacle taller on that one side and - breaks the symmetry the tie depends on. + """Add a stream whose north and south bypasses are exactly tied. + + ``label_pos="center"`` keeps the vessel symmetric; a label above it + would make the north bypass longer. + + Parameters + ---------- + fs : Flowsheet + Sheet that receives the feed, product, and central vessel. + + Returns + ------- + Stream + The F -> P stream, connected last. """ f = fs.add(Feed("F")).pin(x=0, y=200) p = fs.add(Product("P")).pin(x=400, y=200) @@ -798,25 +920,17 @@ def _obstacle_bypass_tie(fs): def test_a_manual_routes_geometry_is_recorded_for_later_streams(): - # A hand-drawn (``.via()``) route is a line on the sheet too, and one an - # auto-routed stream later in this order can genuinely cross. It used - # to be invisible: ``DefaultRouter.route()`` skipped a manual stream - # without ever telling ``crossing_index`` it existed. - # - # A spy on ``CrossingIndex.record`` that merely asserts it was *called* - # passes just as well against a ``record`` that silently does nothing - # once called (a real regression this project's own review caught) -- - # the call happening proves nothing about whether it had any effect. So - # this checks the effect instead: F -> P (connected second, so it - # routes second) has a free, bend- and length-tied choice between an - # obstacle bypass to the north or the south (see - # ``_obstacle_bypass_tie``); the manual stream (connected first) draws - # its line squarely across the north one's path -- x=175 is a lane the - # bypass's own row already carries, so the search's raw path is - # guaranteed to pass through the exact node the crossing is checked at. - # If recording manual geometry ever regresses to a no-op, F -> P goes - # back to breaking this tie arbitrarily, and on this construction that - # arbitrary choice is north -- straight across the line just drawn. + """Record a manual route so a later stream avoids crossing it. + + The manual stream is drawn across the north bypass at x=175, a lane the + graph carries. The test checks the later route, because a call to + ``record`` does not prove that it had an effect. + + Returns + ------- + None + F -> P takes the south bypass. + """ fs = Flowsheet("manual-recorded") f1 = fs.add(Feed("F1")).pin(x=1000, y=1000) p1 = fs.add(Product("P1")).pin(x=1000, y=1000) @@ -833,24 +947,16 @@ def test_a_manual_routes_geometry_is_recorded_for_later_streams(): def test_a_diagonal_manual_route_is_recorded_around_its_own_slant(): - # Unlike a router-drawn path, a manual (``.via()``) one is not - # guaranteed orthogonal -- ``validate()``'s own ``route-diagonal`` - # finding exists because an author's waypoints can legitimately be - # drawn on the slant. Recording that geometry must not crash ``route()`` - # over a leg ``committed_segments`` (rightly) refuses to call an axis; - # the diagonal leg is left out, the orthogonal ones either side of it - # are not. - # - # "Does not crash, and validate() still reports route-diagonal" proves - # only that routing survives the diagonal leg -- a regression that - # skipped recording *every* manual route containing one (diagonal or - # not) would pass both checks just as well. Same fix as the other two - # recording tests: check the orthogonal legs' actual effect. The manual - # route here is an orthogonal leg across the north bypass (as in - # ``test_a_manual_routes_geometry_is_recorded_for_later_streams``) - # followed by a diagonal one that goes nowhere near it, so recording - # the orthogonal leg despite the diagonal one sitting right after it in - # the same waypoint list is exactly what's under test. + """Record the orthogonal legs of a manual route that has a diagonal leg. + + The manual route crosses the north bypass with an orthogonal leg and + then runs diagonally away from it. + + Returns + ------- + None + ``route-diagonal`` is reported and F -> P takes the south bypass. + """ fs = Flowsheet("diagonal-manual") f1 = fs.add(Feed("F1")).pin(x=1000, y=1000) p1 = fs.add(Product("P1")).pin(x=1000, y=1000) @@ -874,27 +980,17 @@ def test_a_diagonal_manual_route_is_recorded_around_its_own_slant(): def test_a_fallback_routes_geometry_is_recorded_for_later_streams(): - # The L drawn when a search comes back empty is still a line on the - # sheet. It used to be invisible too: only the ``if path:`` branch - # called ``crossing_index.record``, never the ``_fallback_path`` one, - # so a later stream's search could not see it -- #425's corpus already - # had two live crossings this let through (``AE-303-80-80-SS`` on - # ``11_ethanol_pid+auto``, a fallback-drawn stream). - # - # Same shape as the manual-route test above and for the same reason: a - # spy that only checks ``crossing_index.record`` was *called* cannot - # tell a working recording from a silent no-op, so this checks that a - # later stream's route actually changes. The fixture units sitting - # near the bypass (rather than far off-sheet, as the manual version - # can afford) perturb the tie by a small, fixed amount of their own -- - # empirically about 10px -- so ``CROSSING_PENALTY`` is raised for the - # duration of this test alone, comfortably clear of that perturbation - # without depending on its exact size. - # - # ``find_path`` is forced empty on its first call only, which is the - # first stream's -- the same seam ``route_quality.py``'s own ``Recorder`` - # patches, since ``DefaultRouter.route()`` re-imports it fresh every - # call. + """Record a fallback route so a later stream avoids crossing it. + + F1 -> P1 is forced onto the fallback across the north bypass. The + fixture units near the bypass shift the tie by about 10 px, so + ``CROSSING_PENALTY`` is raised for this test. + + Returns + ------- + None + F1 -> P1 draws a fallback and F -> P takes the south bypass. + """ fs = Flowsheet("fallback-recorded") f1 = fs.add(Feed("F1")).pin(x=175, y=100) p1 = fs.add(Product("P1")).pin(x=175, y=170) @@ -903,15 +999,29 @@ def test_a_fallback_routes_geometry_is_recorded_for_later_streams(): original_find_path = astar.find_path original_penalty = astar.CROSSING_PENALTY - calls = [0] - def forced_empty_once(*args, **kwargs): - calls[0] += 1 - if calls[0] == 1: + def forced_empty_for_first(graph, start, *args, **kwargs): + """Return no path for a search that starts on F1's outlet row. + + Parameters + ---------- + graph : VisibilityGraph + Graph whose port anchors identify the first stream. + start : tuple[float, float] + Search start node. + *args, **kwargs + Remaining ``find_path`` arguments. + + Returns + ------- + list[tuple[float, float]] + An empty path for F1 -> P1, otherwise the real search result. + """ + if start[1] == graph.port_anchors[("F1", "outlet")][1]: return [] - return original_find_path(*args, **kwargs) + return original_find_path(graph, start, *args, **kwargs) - astar.find_path = forced_empty_once + astar.find_path = forced_empty_for_first astar.CROSSING_PENALTY = 100.0 try: fs.layout() @@ -931,64 +1041,49 @@ def forced_empty_once(*args, **kwargs): def test_crossings_along_counts_a_crossing_strictly_inside_the_span_open_on_both_ends(): - # ``crosses`` answers "does a crossing land here" for one node a search - # actually stops at; ``crossings_along`` is the whole-edge version the - # new interior-edge check in ``find_path`` and ``_fallback_path`` both - # lean on, since neither a graph edge's own skipped-lane stretch nor a - # fallback L's corner is built from nodes at all. Two earlier spans - # crossing the same edge count as two, the same counting ``crosses`` - # already does; a span sitting exactly at the edge's own endpoint -- - # 100.0 here, not strictly inside (0.0, 100.0) -- is not a crossing of - # *this* edge and must not count (the endpoint is what the node-based - # checks price, when it is one of this path's own nodes). + """Count recorded spans crossed strictly inside one edge. + + Returns + ------- + None + Interior spans count once each; a span at the edge's endpoint, a + diagonal, and a zero-length edge count as zero. + """ index = CrossingIndex() index.v[50.0] = [(-10.0, 10.0)] - index.v[100.0] = [(-10.0, 10.0)] # at the edge's own endpoint, not inside it + index.v[100.0] = [(-10.0, 10.0)] # Lie on the edge's endpoint, not inside it. assert index.crossings_along((0.0, 0.0), (100.0, 0.0)) == 1 - index.v[50.0].append((-10.0, 10.0)) # a second earlier stream, same span + index.v[50.0].append((-10.0, 10.0)) # Add a second stream on the span. assert index.crossings_along((0.0, 0.0), (100.0, 0.0)) == 2 - # Neither a diagonal nor a zero-length pair is a segment this index's - # straight-line spans can properly cross. + # Count nothing for a diagonal or a zero-length edge. assert index.crossings_along((0.0, 0.0), (10.0, 10.0)) == 0 assert index.crossings_along((5.0, 5.0), (5.0, 5.0)) == 0 def test_crossings_along_sees_a_track_added_after_its_own_sorted_cache_was_built(): - # #483 round 6: the per-edge scan a search now runs (see above) was - # linear in every track this index had ever been given, on every edge - # -- +56% on a real corpus sheet, 154x on a synthetic worst case (a - # long path against 50,000 already-drawn tracks). Fixed by bisecting a - # cached, sorted view of ``v``'s keys instead of walking all of them, - # invalidated by a change in key *count* rather than kept in step with - # every mutation -- which only works if a query genuinely rebuilds - # after a new track appears, not just returns whatever it cached the - # first time it was asked. First query builds the cache over one - # track; a second track then arrives -- via ``record``, the way a real - # stream adds one, not another direct poke -- strictly inside a range - # the first query alone would not have crossed, so the second query - # only finds it if the cache actually refreshed. + """Refresh the sorted track cache when a new track is recorded. + + Returns + ------- + None + A track recorded after the first query is counted by the second. + """ index = CrossingIndex() index.v[10.0] = [(-10.0, 10.0)] - assert index.crossings_along((0.0, 0.0), (20.0, 0.0)) == 1 # builds the cache over {10.0} + assert index.crossings_along((0.0, 0.0), (20.0, 0.0)) == 1 # Build the cache. - index.record([(50.0, -10.0), (50.0, 10.0)]) # a new track, added after that build + index.record([(50.0, -10.0), (50.0, 10.0)]) # Add a track after the build. assert index.crossings_along((0.0, 0.0), (100.0, 0.0)) == 2 def test_find_path_prices_a_crossing_strictly_inside_a_long_edge_not_just_at_a_node(): - # #483's round-5 review, point 1: a graph edge is one hop, but not - # necessarily a short one -- the visibility grid skips lane coordinates - # an obstacle blocks, not the ones past it, so an edge can run well past - # several skipped coordinates with no node of its own anywhere inside. - # A hand-drawn (``.via()``) route is under no obligation to land on a - # lane at all, so an earlier crossing can sit at exactly such a - # coordinate -- x=50 here, strictly inside the graph's only edge from - # (0, 0) to (100, 0) and never itself a node -- where the old per-node - # checks alone never looked. Neither ``current`` (0, 0) nor ``goal`` - # (100, 0) is itself strictly interior to the recorded span (its fixed - # coordinate, 50, matches neither), so this isolates the new - # whole-edge check from the pre-existing node-based ones: they would - # find nothing here on their own. + """Charge a crossing that lies inside an edge and on no graph node. + + Returns + ------- + None + The one-route graph costs one penalty more with the crossing. + """ nodes = {(0.0, 0.0), (100.0, 0.0)} edges: dict = {n: [] for n in nodes} edges[(0.0, 0.0)].append((100.0, 0.0)) @@ -998,21 +1093,23 @@ def test_find_path_prices_a_crossing_strictly_inside_a_long_edge_not_just_at_a_n clear = _final_g(graph, (0.0, 0.0), (100.0, 0.0), None, "W") index = CrossingIndex() - index.v[50.0] = [(-10.0, 10.0)] # off-node: 50.0 is nowhere in ``nodes`` + index.v[50.0] = [(-10.0, 10.0)] # Use x=50, which is not a graph node. crossed = _final_g(graph, (0.0, 0.0), (100.0, 0.0), None, "W", crossing_index=index) assert crossed == pytest.approx(clear + astar.CROSSING_PENALTY) def test_a_manual_route_off_lane_still_breaks_the_tie(): - # #483's round-5 review, point 1, at the ``Flowsheet`` level: x=175 in - # ``test_a_manual_routes_geometry_is_recorded_for_later_streams`` above - # happens to be a lane the bypass's own row already carries, so that - # test was already passing on the pre-existing per-node check alone. - # x=176 is not a lane coordinate anywhere on this sheet's grid -- before - # the interior-edge check existed, F -> P's search edge sailed straight - # over it (confirmed directly against the pre-fix code) and drew the - # crossing unpriced. Same fixture, same assertions, one digit changed. + """Avoid a manual route drawn off every graph lane. + + The manual stream runs at x=176, which is not a lane coordinate, so + only the whole-edge crossing check can price it. + + Returns + ------- + None + F -> P takes the south bypass. + """ fs = Flowsheet("manual-recorded-off-lane") f1 = fs.add(Feed("F1")).pin(x=1000, y=1000) p1 = fs.add(Product("P1")).pin(x=1000, y=1000) @@ -1029,14 +1126,14 @@ def test_a_manual_route_off_lane_still_breaks_the_tie(): def test_preview_separated_waypoints_does_not_mutate_and_matches_the_real_pass(): - # ``DefaultRouter.route()`` needs to know where ``separate_streams`` - # will actually put a stream's waypoints *before* that pass has run - # (see the ``settle`` test below for why), without that preview - # becoming the drawn sheet itself -- ``_compute_offsets``'s own - # docstring covers why running the real, writing pass more than once - # is not an option. So the preview has to promise two things: it - # changes nothing on its own, and it agrees with the real pass once - # that does run. + """Preview separation without changing routes, and match the real pass. + + Returns + ------- + None + Routes are unchanged by the preview, equal it after separation, + and separation moves at least one of them. + """ from pandid.routing.separation import preview_separated_waypoints, separate_streams fs = Flowsheet("preview-vs-real") @@ -1050,26 +1147,21 @@ def test_preview_separated_waypoints_does_not_mutate_and_matches_the_real_pass() s2 = fs.connect(f2.outlet, p2.inlet).via( [(100.0, 50.0), (100.0, 100.0), (400.0, 100.0), (400.0, 150.0)] ) - assert s1.route is not None and s2.route is not None # ``.via()`` sets both directly + assert s1.route is not None and s2.route is not None # ``via()`` sets both routes. raw_s1, raw_s2 = list(s1.route.waypoints), list(s2.route.waypoints) preview = preview_separated_waypoints(fs.streams) - # Not applied to the real waypoints -- the whole point of a preview. + # Leave the stored waypoints unchanged. assert s1.route.waypoints == raw_s1 assert s2.route.waypoints == raw_s2 separate_streams(fs) - # But it has to match what the real, writing pass settles on, or a - # decision made from the preview would not match the sheet that gets - # drawn either. + # Match the waypoints the real pass writes. assert preview[id(s1)] == s1.route.waypoints assert preview[id(s2)] == s2.route.waypoints - # And separation actually moved something here, or none of the above - # proves anything: s1/s2's overlapping middle runs (both at y=100, - # neither "fixed" -- see ``_compute_offsets`` -- since neither is the - # first or last segment of its route) are exactly what triggers it. + # Confirm that the overlapping runs at y=100 were separated. assert s1.route.waypoints != raw_s1 or s2.route.waypoints != raw_s2 @@ -1085,6 +1177,20 @@ def test_a_later_streams_recording_uses_separated_not_raw_geometry(): original_record = CrossingIndex.record def spy(self, path): + """Record one path and keep a copy of it. + + Parameters + ---------- + self : CrossingIndex + Index receiving the path. + path : list[tuple[float, float]] + Route waypoints. + + Returns + ------- + None + Result of the original ``record``. + """ recorded.append(list(path)) return original_record(self, path) diff --git a/tests/test_show.py b/tests/test_show.py index 6765031..1828995 100644 --- a/tests/test_show.py +++ b/tests/test_show.py @@ -1,10 +1,7 @@ -"""``Flowsheet.show()``: the same keywords ``render()`` takes, a window -where there is one, and the browser where there is not. +"""Check ``Flowsheet.show()``: its keywords, its window, and its fallback. -Nothing here calls ``mainloop()``. A window that blocks until it is closed -is the point of the feature and the one thing a test suite must never -open, so the window is built and inspected and then destroyed, and every -test of the fallback drives the decision function rather than the display. +No test calls ``mainloop()``. A window is built, inspected, and closed, and +the browser fallback is tested through the decision function. """ import inspect @@ -18,7 +15,7 @@ from pandid import Flowsheet, units as U from pandid.render import preview as P -#: A 1x1 PNG, for the tests that need *an* image and not a drawing. +#: A 1x1 PNG for tests that need an image but not a drawing. _PNG = ( b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06" b"\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00\x01\x00\x00\x05" @@ -27,6 +24,13 @@ def _fs() -> Flowsheet: + """Build a three-unit sheet with one tabulated stream. + + Returns + ------- + Flowsheet + Feed, pump, and product connected in series. + """ fs = Flowsheet("Sheet 1") feed = fs.add(U.Feed("F")) pump = fs.add(U.Pump("P-101")) @@ -39,10 +43,35 @@ def _fs() -> Flowsheet: @pytest.fixture def caught(monkeypatch): - """``show()`` stopped at the display: the SVG it produced, unshown.""" + """Capture the SVG and title ``show()`` passes to the preview. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Replaces ``preview`` with a recorder. + + Returns + ------- + dict + The ``svg`` and ``title`` of the last call. + """ seen: dict = {} def fake(svg, *, title=""): + """Record one preview call without opening anything. + + Parameters + ---------- + svg : str + Rendered SVG markup. + title : str, optional + Flowsheet title. + + Returns + ------- + str + ``"window"``, as a successful preview returns. + """ seen["svg"], seen["title"] = svg, title return "window" @@ -50,17 +79,16 @@ def fake(svg, *, title=""): return seen -# --- the signature ------------------------------------------------------------ +# --- The signature ------------------------------------------------------------ def test_show_takes_exactly_the_keywords_render_takes(): - """The guard the docstring names. + """Hold ``show()`` to the keywords, kinds, and defaults of ``render()``. - ``show()`` had none of ``render()``'s nine keywords, so the one call an - author makes while drafting could not preview a stream table or a P&ID. - Restating the list is what let them drift apart in the first place, so - the list is restated *once* and held equal here: a keyword added to - ``render()`` and not to ``show()`` fails on the day it lands. + Returns + ------- + None + Every ``render()`` keyword except ``path`` matches on ``show()``. """ render = inspect.signature(Flowsheet.render).parameters show = inspect.signature(Flowsheet.show).parameters @@ -81,35 +109,73 @@ def test_show_takes_exactly_the_keywords_render_takes(): ("diagram", "p&id", True), ("page_size", "A3", True), ("debug", True, True), - # The three that this sheet cannot show a difference for, and it is the - # sheet rather than the forwarding: a PFD marks no joints whatever - # ``connections`` says, nothing on it crosses for ``jump_direction`` to - # hop, and ``check`` decides what lands on ``fs.warnings`` rather than - # what is drawn. Equality with ``to_svg()`` is still the contract. + # These three do not change this sheet: a PFD marks no joints, no + # lines cross, and ``check`` affects warnings rather than the drawing. ("connections", "flanged", False), ("jump_direction", "horizontal", False), ("check", False, False), ], ) def test_every_keyword_reaches_the_drawing(caught, keyword, value, visible): - """Taking the keyword is half of it; the sheet shown has to be the sheet - ``to_svg()`` would have returned for the same words.""" + """Forward each keyword to the renderer. + + Parameters + ---------- + caught : dict + Captured preview call. + keyword : str + ``show()`` keyword under test. + value : object + Value passed for the keyword. + visible : bool + Whether the value changes this sheet's drawing. + + Returns + ------- + None + The previewed SVG equals ``to_svg()`` for the same keyword. + """ _fs().show(**{keyword: value}) assert caught["svg"] == _fs().to_svg(**{keyword: value}) assert (caught["svg"] != _fs().to_svg()) is visible def test_the_sheet_is_named_to_whatever_shows_it(caught): + """Pass the flowsheet title to the preview. + + Parameters + ---------- + caught : dict + Captured preview call. + + Returns + ------- + None + The preview receives the sheet's title. + """ _fs().show() assert caught["title"] == "Sheet 1" -# --- choosing a window or the browser ----------------------------------------- +# --- Choosing a window or the browser ----------------------------------------- @pytest.fixture def browsed(monkeypatch, tmp_path): - """A preview directory under *tmp_path* and the browser stubbed out.""" + """Redirect preview files to a temporary directory and stub the browser. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Replaces the preview directory and ``webbrowser.open``. + tmp_path : pathlib.Path + Parent of the preview directory. + + Returns + ------- + list[str] + URLs the module asked the browser to open. + """ opened: list[str] = [] monkeypatch.setattr(P, "_dir", str(tmp_path / "preview")) (tmp_path / "preview").mkdir() @@ -118,6 +184,22 @@ def browsed(monkeypatch, tmp_path): def test_a_machine_with_no_display_falls_back_and_says_so(monkeypatch, browsed, capsys): + """Open the browser and print the reason when no display exists. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Reports a missing display. + browsed : list[str] + URLs sent to the browser. + capsys : pytest.CaptureFixture + Captured standard output. + + Returns + ------- + None + One URL is opened and the reason is printed. + """ monkeypatch.setattr(P, "_no_display", lambda: "no display ($DISPLAY is unset)") assert P.preview("", title="Sheet 1") == "browser" out = capsys.readouterr().out @@ -126,9 +208,22 @@ def test_a_machine_with_no_display_falls_back_and_says_so(monkeypatch, browsed, def test_a_machine_with_no_rasteriser_falls_back_and_names_the_extra(monkeypatch, browsed, capsys): - """The window needs the ``pdf`` extra to turn the SVG into pixels. Without - it there is still a drawing to look at, so the browser gets it and the - message says what would buy a window.""" + """Open the browser and name the ``pdf`` extra when it is missing. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Makes rasterisation raise ``ImportError``. + browsed : list[str] + URLs sent to the browser. + capsys : pytest.CaptureFixture + Captured standard output. + + Returns + ------- + None + One URL is opened and the message names ``pandid[pdf]``. + """ monkeypatch.setattr(P, "_no_display", lambda: "") monkeypatch.setattr(P, "_raster", lambda svg: (_ for _ in ()).throw(ImportError("no"))) assert P.preview("") == "browser" @@ -137,11 +232,37 @@ def test_a_machine_with_no_rasteriser_falls_back_and_names_the_extra(monkeypatch def test_a_rasteriser_that_fails_falls_back_rather_than_raising(monkeypatch, browsed, capsys): - """A window that cannot be drawn is not a render that failed: the SVG the - browser is handed is the renderer's own output and is unaffected.""" + """Open the browser when rasterisation raises. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Makes rasterisation raise ``RuntimeError``. + browsed : list[str] + URLs sent to the browser. + capsys : pytest.CaptureFixture + Captured standard output. + + Returns + ------- + None + The preview falls back and prints the error. + """ monkeypatch.setattr(P, "_no_display", lambda: "") def boom(svg): + """Fail as a broken PNG backend would. + + Parameters + ---------- + svg : str + Rendered SVG markup. + + Raises + ------ + RuntimeError + Always. + """ raise RuntimeError("the PDF backend could not read the rendered SVG") monkeypatch.setattr(P, "_raster", boom) @@ -150,11 +271,21 @@ def boom(svg): def test_the_display_check_answers_an_unset_display_without_importing_tkinter(monkeypatch): - """The headless case is answered from the environment, so nothing can - block on a socket to a display that is not listening.""" + """Report an unset ``DISPLAY`` without importing tkinter. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Sets a Linux platform, unsets ``DISPLAY``, and blocks the import. + + Returns + ------- + None + The reason names ``DISPLAY``. + """ monkeypatch.setattr(sys, "platform", "linux") monkeypatch.delenv("DISPLAY", raising=False) - monkeypatch.setitem(sys.modules, "tkinter", None) # an import would raise + monkeypatch.setitem(sys.modules, "tkinter", None) # Make an import raise. assert "DISPLAY" in P._no_display() @@ -188,53 +319,115 @@ def test_macos_with_a_gui_session_can_open_tkinter(monkeypatch): Returns ------- None - The display check opens and closes the root successfully. + The display check opens the root and closes it with no idle task left. """ - closed = [] + calls = [] class Root: """Stand in for a Tk root. Notes ----- - Destruction is recorded in ``closed``. + Each call is recorded in ``calls``. """ + def withdraw(self) -> None: + """Record that the temporary window was hidden. + + Returns + ------- + None + The call is appended to ``calls``. + """ + calls.append("withdraw") + + def update_idletasks(self) -> None: + """Record that pending idle tasks were run. + + Returns + ------- + None + The call is appended to ``calls``. + """ + calls.append("update_idletasks") + def destroy(self) -> None: """Record that the temporary window was closed. Returns ------- None - The root is marked closed. + The call is appended to ``calls``. """ - closed.append(True) + calls.append("destroy") monkeypatch.setattr(sys, "platform", "darwin") monkeypatch.setattr(P, "_macos_gui_session", lambda: True) monkeypatch.setitem(sys.modules, "tkinter", SimpleNamespace(Tk=Root, TclError=Exception)) assert P._no_display() == "" - assert closed == [True] + # Tk 9 on macOS aborts when a later root services an idle task left by a + # destroyed root, so idle tasks run before the root is destroyed. + assert calls == ["withdraw", "update_idletasks", "destroy"] -# --- the temporary file ------------------------------------------------------- +# --- The temporary file ------------------------------------------------------- def test_the_browser_gets_a_url_a_browser_can_open(browsed): - """``"file://" + r"C:\\Users\\..."`` is a URL whose *hostname* is ``C:``, - and a browser opens nothing at all from it.""" + """Give the browser a ``file:///`` URL without backslashes. + + Parameters + ---------- + browsed : list[str] + URLs sent to the browser. + + Returns + ------- + None + The URL is a valid file URI on every platform. + """ P._browser("", "Sheet 1", "testing") assert browsed[0].startswith("file:///") assert "\\" not in browsed[0] def test_previewing_twenty_drafts_leaves_one_file(browsed, tmp_path): + """Reuse one preview file for repeated previews of a sheet. + + Parameters + ---------- + browsed : list[str] + URLs sent to the browser. + tmp_path : pathlib.Path + Parent of the preview directory. + + Returns + ------- + None + Twenty previews leave a single file. + """ for _ in range(20): P._browser("", "Sheet 1", "testing") assert len(list((tmp_path / "preview").iterdir())) == 1 def test_the_file_is_dropped_on_the_way_out(monkeypatch, browsed, tmp_path): + """Remove the preview directory once the grace period has passed. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Expires the grace period. + browsed : list[str] + URLs sent to the browser. + tmp_path : pathlib.Path + Parent of the preview directory. + + Returns + ------- + None + The directory no longer exists. + """ P._browser("", "Sheet 1", "testing") monkeypatch.setattr(P, "_grace_until", time.monotonic() - 1) P._discard() @@ -242,15 +435,38 @@ def test_the_file_is_dropped_on_the_way_out(monkeypatch, browsed, tmp_path): def test_a_browser_just_launched_keeps_its_file(browsed, tmp_path): - """``webbrowser.open`` returns when the browser is *launched*, which on a - cold start is before it has read anything. Deleting then would blank the - tab that is opening, so the file is left for the next run's sweep.""" + """Keep the preview file while the browser may still be reading it. + + Parameters + ---------- + browsed : list[str] + URLs sent to the browser. + tmp_path : pathlib.Path + Parent of the preview directory. + + Returns + ------- + None + The file survives a discard inside the grace period. + """ P._browser("", "Sheet 1", "testing") P._discard() assert (tmp_path / "preview" / "Sheet-1.svg").exists() def test_the_sweep_takes_what_an_earlier_run_left(tmp_path): + """Remove only stale preview directories. + + Parameters + ---------- + tmp_path : pathlib.Path + Directory holding a stale, a fresh, and an unrelated directory. + + Returns + ------- + None + The stale directory is removed and the other two remain. + """ stale = tmp_path / f"{P._PREFIX}old" fresh = tmp_path / f"{P._PREFIX}new" other = tmp_path / "not-ours" @@ -274,25 +490,53 @@ def test_the_sweep_takes_what_an_earlier_run_left(tmp_path): ], ) def test_a_sheet_name_reaching_a_path_keeps_only_what_a_path_takes(title, stem): + """Reduce a sheet title to a safe filename stem. + + Parameters + ---------- + title : str + Flowsheet title. + stem : str + Expected filename stem. + + Returns + ------- + None + Path separators are removed and an empty result becomes ``sheet``. + """ assert P._slug(title) == stem -# --- the window itself -------------------------------------------------------- +# --- The window itself -------------------------------------------------------- @pytest.mark.parametrize( "image,into,fitted", [ - ((1000, 500), (400, 400), (400, 200)), # wide: width binds - ((500, 1000), (400, 400), (200, 400)), # tall: height binds - ((400, 300), (800, 600), (800, 600)), # smaller than the window: filled - ((1000, 500), (1000, 500), (1000, 500)), # exact - ((1000, 500), (3, 1), (2, 1)), # never rounded away to nothing + ((1000, 500), (400, 400), (400, 200)), # Width limits a wide image. + ((500, 1000), (400, 400), (200, 400)), # Height limits a tall image. + ((400, 300), (800, 600), (800, 600)), # A small image is enlarged. + ((1000, 500), (1000, 500), (1000, 500)), # An exact fit is unchanged. + ((1000, 500), (3, 1), (2, 1)), # Each side stays at least one pixel. ], ) def test_the_sheet_is_fitted_to_the_window_and_never_stretched(image, into, fitted): - """The window's whole geometry, checked where no display is needed. The - shape is the drawing's; only the size is the window's.""" + """Scale an image into a window while keeping its aspect ratio. + + Parameters + ---------- + image : tuple[int, int] + Source width and height. + into : tuple[int, int] + Available width and height. + fitted : tuple[int, int] + Expected scaled width and height. + + Returns + ------- + None + ``_fit`` returns the expected size. + """ assert P._fit(image, into) == fitted @@ -319,9 +563,22 @@ def _tk_or_skip(): def _drawn(root, canvas, width: int, height: int) -> tuple[int, int]: """Resize the window and return the size of the image it settles on. - The redraw is scheduled rather than immediate (see ``_SETTLE_MS``), so - the pending timer is let run -- ``update()`` services one that is due -- - instead of the event loop being entered, which would not return. + The redraw is delayed by ``_SETTLE_MS``, so ``update()`` is called again + after that delay instead of entering the event loop. + + Parameters + ---------- + root : tkinter.Tk + Preview window. + canvas : tkinter.Canvas + Canvas holding the sheet image. + width, height : int + Requested window size in pixels. + + Returns + ------- + tuple[int, int] + Width and height of the drawn image. """ root.geometry(f"{width}x{height}") root.update() @@ -334,13 +591,15 @@ def _drawn(root, canvas, width: int, height: int) -> tuple[int, int]: def test_the_window_draws_the_sheet_scaled_to_the_window(): - """A window and not a fixed-size picture: the image is remade for the - size the canvas has, so resizing the window resizes the drawing rather - than cropping it or leaving it alone. + """Redraw the sheet at the canvas size when the window is resized. + + This test opens a real window and closes it in ``finally``. - The one test here that opens a real window (briefly, and closed in a - ``finally``); everything it needs a display for is the wiring, since - the arithmetic is :func:`_fit`'s and is checked above without one. + Returns + ------- + None + The image fits each window size, keeps its shape, and the title + names the sheet. """ from pandid.render import export @@ -352,36 +611,57 @@ def test_the_window_draws_the_sheet_scaled_to_the_window(): large = _drawn(root, canvas, 700, 550) assert small[0] <= 400 and small[1] <= 300 assert large[0] > small[0] and large[1] > small[1] - # The same drawing, so the same shape: fitted, never stretched. + # Compare aspect ratios to confirm the image is not stretched. assert abs(small[0] / small[1] - large[0] / large[1]) < 0.05 assert "pandid" in root.title() and "Sheet 1" in root.title() finally: - root.destroy() + P._close(root) def test_the_window_closes_the_ways_an_image_viewer_does(): + """Bind Escape and ``q`` to close the window. + + Returns + ------- + None + Both key bindings exist on the root. + """ root = _tk_or_skip() try: P._window(root, _PNG, "t") assert root.bind("") and root.bind("q") finally: - root.destroy() + P._close(root) def test_the_module_never_writes_a_file_for_a_window(monkeypatch, tmp_path): - """A window is handed bytes. Only the browser needs somewhere on disk to - point at, so the window path leaves nothing behind at all.""" + """Create no preview directory when a window is shown. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Supplies a display, a raster, and a stand-in Tk root. + tmp_path : pathlib.Path + Unused temporary directory. + + Returns + ------- + None + The preview reports a window and ``_dir`` stays unset. + """ monkeypatch.setattr(P, "_dir", None) monkeypatch.setattr(P, "_no_display", lambda: "") monkeypatch.setattr(P, "_raster", lambda svg: _PNG) monkeypatch.setattr(P, "_window", lambda root, png, title: None) class Root: + """Stand in for a Tk root that returns from ``mainloop`` at once.""" + def mainloop(self): - pass + """Return immediately instead of running the event loop.""" def destroy(self): - pass + """Accept the close request and do nothing.""" monkeypatch.setattr("tkinter.Tk", Root) assert P.preview("") == "window"