Skip to content

Commit d928901

Browse files
authored
Merge pull request #1561 from seqeralabs/fix/1545-vertical-flow-reversal
fix(parser): split flow-reversal and axis-query concerns for vertical flows
2 parents 8628766 + 952f7f6 commit d928901

4 files changed

Lines changed: 165 additions & 21 deletions

File tree

.test_durations

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23421,4 +23421,4 @@
2342123421
"tests/test_validate_flag_idempotent.py::test_validate_flag_does_not_change_geometry[topologies/wide_fan_out]": 0.09497337485663593,
2342223422
"tests/test_validate_flag_idempotent.py::test_validate_flag_does_not_change_geometry[topologies/wide_label_fan]": 0.057290040189400315,
2342323423
"tests/test_validate_flag_idempotent.py::test_validate_flag_does_not_change_geometry[topologies/wrapped_label_trunk]": 0.02588058402761817
23424-
}
23424+
}

src/nf_metro/parser/resolve.py

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -587,22 +587,44 @@ def _resolve_sections(graph: MetroGraph) -> None:
587587
"TB": PortSide.BOTTOM,
588588
"BT": PortSide.TOP,
589589
}
590-
_FLIP_HORIZONTAL = {"LR": "RL", "RL": "LR"}
590+
# Horizontal flows only: a vertical reversal re-seats the trailing exit on the
591+
# far edge, and the route out of it wraps around the section and back through
592+
# its target's interior, which the re-anchor remedy avoids.
593+
_HORIZONTAL_FLOW_REVERSAL = {"LR": "RL", "RL": "LR"}
591594

592595

593-
def _connecting_flow_side(near: Section, far: Section) -> PortSide | None:
594-
"""The flow-axis side ``far`` sits on relative to ``near``.
596+
def _flow_axis_is_x(direction: str) -> bool:
597+
"""Whether *direction* runs its flow along X (LR/RL) rather than Y (TB/BT)."""
598+
from nf_metro.layout.geometry import AxisFrame
595599

596-
``LEFT``/``RIGHT`` for a horizontal ``near`` (by grid column), ``TOP``/
600+
return AxisFrame.axes_for_direction(direction)[0] == "x"
601+
602+
603+
def _connecting_flow_side(
604+
graph: MetroGraph, near_id: str, far_id: str
605+
) -> PortSide | None:
606+
"""The flow-axis side ``far_id`` sits on relative to ``near_id``.
607+
608+
``LEFT``/``RIGHT`` for a horizontal ``near_id`` (by grid column), ``TOP``/
597609
``BOTTOM`` for a vertical one (by grid row); ``None`` when the two share the
598610
axis coordinate, so neither side is implied.
611+
612+
Reads positions through :func:`_effective_grid_pos`: an explicit
613+
``%%metro grid:`` directive lands in ``graph.grid_overrides`` at parse
614+
time, and only reaches ``Section.grid_col``/``grid_row`` later, in
615+
section placement, so this stage must not read those fields directly.
599616
"""
600-
if near.direction in _FLIP_HORIZONTAL:
617+
from nf_metro.layout.auto_layout import _effective_grid_pos
618+
619+
near = graph.sections[near_id]
620+
near_col, near_row, *_ = _effective_grid_pos(graph, near_id)
621+
far_col, far_row, *_ = _effective_grid_pos(graph, far_id)
622+
if _flow_axis_is_x(near.direction):
601623
low, high = PortSide.LEFT, PortSide.RIGHT
602-
near_pos, far_pos = near.grid_col, far.grid_col
624+
near_pos, far_pos = near_col, far_col
603625
else:
604626
low, high = PortSide.TOP, PortSide.BOTTOM
605-
near_pos, far_pos = near.grid_row, far.grid_row
627+
near_pos, far_pos = near_row, far_row
606628
if far_pos < near_pos:
607629
return low
608630
if far_pos > near_pos:
@@ -710,7 +732,7 @@ def _reside_folded_flow_ports_to_grid(
710732

711733
for sec_id in relocated:
712734
section = graph.sections[sec_id]
713-
if section.direction not in _FLIP_HORIZONTAL: # LR/RL carry flow on x
735+
if not _flow_axis_is_x(section.direction):
714736
continue
715737
col = section.grid_col
716738
for hints, cols_by_line, is_entry in (
@@ -767,16 +789,12 @@ def _reanchor_flow_axis_ports(
767789
if tgt_sec:
768790
consumers[tgt_sec][e.line_id].add(e.target)
769791
if src_sec:
770-
side = _connecting_flow_side(
771-
graph.sections[tgt_sec], graph.sections[src_sec]
772-
)
792+
side = _connecting_flow_side(graph, tgt_sec, src_sec)
773793
consumer_sides[tgt_sec][e.line_id][e.target].add(side)
774794
if src_sec:
775795
producers[src_sec][e.line_id].add(e.source)
776796
if tgt_sec:
777-
side = _connecting_flow_side(
778-
graph.sections[src_sec], graph.sections[tgt_sec]
779-
)
797+
side = _connecting_flow_side(graph, src_sec, tgt_sec)
780798
producer_sides[src_sec][e.line_id][e.source].add(side)
781799

782800
for sec_id, section in graph.sections.items():
@@ -839,11 +857,11 @@ def _reanchor_flow_axis_ports(
839857
# double back) becomes a with-flow port once flipped, so it does not
840858
# block re-orientation.
841859
if (
842-
section.direction in _FLIP_HORIZONTAL
860+
section.direction in _HORIZONTAL_FLOW_REVERSAL
843861
and sec_id not in graph._explicit_directions
844862
and not with_flow
845863
):
846-
new_dir = _FLIP_HORIZONTAL[section.direction]
864+
new_dir = _HORIZONTAL_FLOW_REVERSAL[section.direction]
847865
warnings.warn(
848866
f"Section '{sec_id}': flow re-oriented {section.direction}->"
849867
f"{new_dir} so its declared port faces its connecting section "

tests/test_orientation_equivalence.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,15 @@
104104
)
105105
for stem in ("orbit_perp_exit_back_row_entry",)
106106
},
107-
# A folded flow-axis port is resolved either by reversing the section's flow
108-
# or by re-anchoring the port, but _FLIP_HORIZONTAL makes the reversal
109-
# reachable only for LR/RL, so a vertical flow takes the other remedy.
107+
# _infer_flow_exit_hints_with_drops's perpendicular-drop exception tests only
108+
# whether the TARGET section is vertical, so a vertical-flow source feeding a
109+
# same-row horizontal target keeps a flow-aligned exit instead of turning
110+
# toward its neighbour.
110111
**{
111-
(stem, family): "flow reversal unavailable to vertical flows (#1545)"
112+
(stem, family): (
113+
"auto-inferred exit has no drop exception for a vertical-flow "
114+
"source facing a same-row horizontal target (#1545)"
115+
)
112116
for stem in ("lr_to_tb_top_drop", "top_entry_header_clash")
113117
for family in ("port_side", "port_perpendicular")
114118
},
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
"""Which remedy a folded flow-axis port gets, per flow axis.
2+
3+
A port declared on the edge opposite its connecting section makes the leg run
4+
the length of the trunk and double back through every station in between.
5+
``_reanchor_flow_axis_ports`` has two remedies: reverse the section's flow so
6+
the declared edge becomes the right one, or move the port to the edge its
7+
connecting station is actually on.
8+
9+
The choice is keyed to the flow axis, and deliberately so. A horizontal
10+
section is reversed; a vertical one has its port re-anchored, because reversing
11+
it re-seats the trailing exit on the far edge and the route out then wraps
12+
around the section and back through its target's interior. These tests pin that
13+
split, so the asymmetry is a stated contract.
14+
"""
15+
16+
from __future__ import annotations
17+
18+
import warnings
19+
20+
import pytest
21+
22+
from nf_metro.parser.model import Edge, MetroGraph, PortSide, Section, Station
23+
from nf_metro.parser.resolve import (
24+
_LEADING_SIDE,
25+
_TRAILING_SIDE,
26+
_flow_axis_is_x,
27+
_reanchor_flow_axis_ports,
28+
)
29+
30+
_HIGH_SIDE = (PortSide.RIGHT, PortSide.BOTTOM)
31+
_REVERSED = {"LR": "RL", "RL": "LR", "TB": "BT", "BT": "TB"}
32+
33+
34+
def _folded_section_graph(direction: str) -> tuple[MetroGraph, list[Edge]]:
35+
"""A 'mid' section flowing *direction*, whose exit folds.
36+
37+
'mid' holds m1 -> m2. Its entry sits on the trailing edge and feeds m2,
38+
'mid's own flow-sink, so the entry does not itself fold. Its exit sits on
39+
the leading edge and is fed by that same m2 rather than by the flow-source
40+
m1, so the exit folds. Neither port runs with the flow, which is the
41+
precondition a reversal needs.
42+
"""
43+
graph = MetroGraph()
44+
feed = Section(id="feed", name="Feed", direction="LR")
45+
mid = Section(id="mid", name="Mid", direction=direction)
46+
sink = Section(id="sink", name="Sink", direction="LR")
47+
graph.sections = {"feed": feed, "mid": mid, "sink": sink}
48+
49+
graph.stations = {
50+
"f1": Station(id="f1", label="F1", section_id="feed"),
51+
"m1": Station(id="m1", label="M1", section_id="mid"),
52+
"m2": Station(id="m2", label="M2", section_id="mid"),
53+
"s1": Station(id="s1", label="S1", section_id="sink"),
54+
}
55+
feed.station_ids = ["f1"]
56+
mid.station_ids = ["m1", "m2"]
57+
sink.station_ids = ["s1"]
58+
mid.internal_edges = [Edge(source="m1", target="m2", line_id="a")]
59+
entry_side = _TRAILING_SIDE[direction]
60+
exit_side = _LEADING_SIDE[direction]
61+
mid.entry_hints = [(entry_side, ["a"])]
62+
mid.exit_hints = [(exit_side, ["a"])]
63+
64+
# Each neighbour sits on the side of 'mid' its port faces, so the fold is
65+
# the section's own flow being backwards rather than a misplaced port.
66+
axis_is_x = _flow_axis_is_x(direction)
67+
feed_pos = 2 if entry_side in _HIGH_SIDE else 0
68+
for section, pos in ((feed, feed_pos), (mid, 1), (sink, 2 - feed_pos)):
69+
section.grid_col, section.grid_row = (pos, 0) if axis_is_x else (0, pos)
70+
71+
inter_section_edges = [
72+
Edge(source="f1", target="m2", line_id="a"),
73+
Edge(source="m2", target="s1", line_id="a"),
74+
]
75+
return graph, inter_section_edges
76+
77+
78+
def _resolve(direction: str) -> tuple[MetroGraph, list[str]]:
79+
graph, edges = _folded_section_graph(direction)
80+
with warnings.catch_warnings(record=True) as caught:
81+
warnings.simplefilter("always")
82+
_reanchor_flow_axis_ports(graph, edges)
83+
return graph, [str(w.message) for w in caught]
84+
85+
86+
@pytest.mark.parametrize("direction", ["LR", "RL"])
87+
def test_horizontal_fold_reverses_the_section(direction: str) -> None:
88+
"""A horizontal section's flow is reversed, leaving its port hints alone."""
89+
graph, messages = _resolve(direction)
90+
mid = graph.sections["mid"]
91+
92+
assert mid.direction == _REVERSED[direction]
93+
assert "mid" in graph._fold_reoriented_sections
94+
assert mid.entry_hints == [(_TRAILING_SIDE[direction], ["a"])]
95+
assert mid.exit_hints == [(_LEADING_SIDE[direction], ["a"])]
96+
assert any("flow re-oriented" in m for m in messages), messages
97+
98+
99+
@pytest.mark.parametrize("direction", ["TB", "BT"])
100+
def test_vertical_fold_reanchors_the_port(direction: str) -> None:
101+
"""A vertical section keeps its flow and has the folded port moved.
102+
103+
Reversing it instead would wrap the route out of the re-seated exit around
104+
the section and back through its target's interior, so the port-side
105+
remedy is the one that applies on this axis.
106+
"""
107+
graph, messages = _resolve(direction)
108+
mid = graph.sections["mid"]
109+
110+
assert mid.direction == direction
111+
assert "mid" not in graph._fold_reoriented_sections
112+
assert mid.exit_hints == [(_TRAILING_SIDE[direction], ["a"])]
113+
assert any("re-anchored" in m for m in messages), messages
114+
115+
116+
@pytest.mark.parametrize(
117+
("direction", "expected"),
118+
[("LR", True), ("RL", True), ("TB", False), ("BT", False)],
119+
)
120+
def test_flow_axis_is_x_covers_every_direction(direction: str, expected: bool) -> None:
121+
"""The axis question is answered from the frame, for all four flows."""
122+
assert _flow_axis_is_x(direction) is expected

0 commit comments

Comments
 (0)