From a2e108bc513182b14d27371848c9a49ac1cf8b25 Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Mon, 28 Sep 2026 16:17:28 -0600 Subject: [PATCH] Rollback on by default; --no-rollback keeps every rip-up Over 8 net orders it left 25 to 30 fewer connections unrouted on the dense sense boards and 16 to 18 fewer on two PCBench boards, and was never clearly worse. KiCad DRC on the sense harness: 41 opens to 28. --- src/freeroute/cli.py | 14 ++++++++------ src/freeroute/route/exact_router.py | 2 +- src/freeroute/route/grid_router.py | 8 +++++--- src/freeroute/route/pipeline.py | 8 ++++---- tests/route/test_rollback.py | 4 ++-- tests/test_cli.py | 2 +- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/freeroute/cli.py b/src/freeroute/cli.py index 7befa57..f16740a 100644 --- a/src/freeroute/cli.py +++ b/src/freeroute/cli.py @@ -46,7 +46,7 @@ _FLAG_NAMES = { "orders": "--orders", "promote": "--promote-failures", "access": "--access-cost", - "rollback": "--rollback", + "rollback": "--no-rollback", } @@ -168,10 +168,12 @@ def build_parser() -> argparse.ArgumentParser: "another pin's way out while that pin still has connections to route (default 0: off)", ) parser.add_argument( - "--rollback", - action="store_true", - help="grid and exact engines: reroute the connections a rip-up displaces at once, " - "and undo the rip-up if fewer connections end up routed", + "--no-rollback", + dest="rollback", + action="store_false", + help="grid and exact engines: keep every rip-up, as before. By default the " + "connections a rip-up displaces are rerouted at once and the rip-up is undone " + "if fewer connections end up routed", ) parser.add_argument( "--jobs", @@ -202,7 +204,7 @@ def main(argv: list[str] | None = None) -> int: ("orders", args.orders is not None), ("promote", args.promote_failures is not None), ("access", args.access_cost is not None), - ("rollback", args.rollback), + ("rollback", not args.rollback), ) if asked ] diff --git a/src/freeroute/route/exact_router.py b/src/freeroute/route/exact_router.py index acfe339..fb18ac9 100644 --- a/src/freeroute/route/exact_router.py +++ b/src/freeroute/route/exact_router.py @@ -98,7 +98,7 @@ def route_board_exact( seed: int | None = None, route_first: frozenset | None = None, access_cost: float = 0.0, - rollback: bool = False, + rollback: bool = True, ) -> ExactRouteResult: """Route ``board`` orthogonally and verify the result with exact geometry. diff --git a/src/freeroute/route/grid_router.py b/src/freeroute/route/grid_router.py index 5f89bc9..797ea31 100644 --- a/src/freeroute/route/grid_router.py +++ b/src/freeroute/route/grid_router.py @@ -162,7 +162,7 @@ class GridRouter: seed: int | None = None, route_first: frozenset | None = None, access_cost: float = 0.0, - rollback: bool = False, + rollback: bool = True, ) -> None: self.board = board # Pin access: while a pin still has connections to route, the cells @@ -174,7 +174,9 @@ class GridRouter: self._pending: dict[int, int] = {} # Rip-up as a transaction: the connections a rip-up route crosses are # rerouted at once, and if fewer connections are routed afterwards - # than before, everything goes back as it was. + # than before, everything goes back as it was. On by default: over + # 8 net orders it left 25 to 30 fewer connections unrouted on dense + # boards and was never clearly worse. self.rollback = bool(rollback) # None keeps the deterministic order; a seed shuffles which connection # routes first, to measure how much a result owes to the order @@ -1228,7 +1230,7 @@ def route_board( seed: int | None = None, route_first: frozenset | None = None, access_cost: float = 0.0, - rollback: bool = False, + rollback: bool = True, ) -> RouteResult: """Convenience wrapper: build a :class:`GridRouter` and route the board.""" router = GridRouter( diff --git a/src/freeroute/route/pipeline.py b/src/freeroute/route/pipeline.py index 1815093..b2b046e 100644 --- a/src/freeroute/route/pipeline.py +++ b/src/freeroute/route/pipeline.py @@ -83,7 +83,7 @@ def route_dsn_board( seed: int | None = None, route_first: frozenset | None = None, access_cost: float = 0.0, - rollback: bool = False, + rollback: bool = True, ) -> tuple[RouteResult, int, list[str]]: """Route a parsed :class:`DsnBoard`; return the board-unit result, the scale, and the layer names. ``layers`` restricts routing to those signal-layer @@ -151,7 +151,7 @@ def build_routing_result( seed: int | None = None, route_first: frozenset | None = None, access_cost: float = 0.0, - rollback: bool = False, + rollback: bool = True, ) -> RoutingResult: """Route ``dsn`` (grid track) and convert to a DSN-unit RoutingResult.""" route, scale, layer_names = route_dsn_board( @@ -178,7 +178,7 @@ def route_dsn_board_exact( seed: int | None = None, route_first: frozenset | None = None, access_cost: float = 0.0, - rollback: bool = False, + rollback: bool = True, ) -> tuple[ExactRouteResult, int, list[str]]: """Route a parsed :class:`DsnBoard` with the exact-geometry (orthogonal, DRC-verified) track; return the exact result, the scale, and layer names. @@ -225,7 +225,7 @@ def build_exact_routing_result( seed: int | None = None, route_first: frozenset | None = None, access_cost: float = 0.0, - rollback: bool = False, + rollback: bool = True, ) -> RoutingResult: """Route ``dsn`` (exact track) and convert to a DSN-unit RoutingResult.""" exact, scale, layer_names = route_dsn_board_exact( diff --git a/tests/route/test_rollback.py b/tests/route/test_rollback.py index 86c8dac..38ab250 100644 --- a/tests/route/test_rollback.py +++ b/tests/route/test_rollback.py @@ -38,6 +38,6 @@ def test_occupancy_only_holds_routed_connections(name): @pytest.mark.parametrize("name", ["ripup_needed.dsn", "true_density.dsn", "kicad_routable.dsn"]) def test_rollback_never_routes_fewer_than_plain_rip_up(name): text = (FIXTURES / name).read_text() - plain, _, _ = route_dsn_board(parse_dsn(text)) - undo, _, _ = route_dsn_board(parse_dsn(text), rollback=True) + plain, _, _ = route_dsn_board(parse_dsn(text), rollback=False) + undo, _, _ = route_dsn_board(parse_dsn(text)) assert undo.unrouted <= plain.unrouted diff --git a/tests/test_cli.py b/tests/test_cli.py index e466725..5e56191 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -158,7 +158,7 @@ def test_cli_supported_combinations_are_accepted(tmp_path): "orders": ["--orders", "2"], "promote": ["--promote-failures", "2"], "access": ["--access-cost", "5"], - "rollback": ["--rollback"], + "rollback": ["--no-rollback"], } for engine, options in ENGINE_OPTIONS.items(): for option in options: