Add Grid for surveying Zephyr topologies by mahdiehmalekian · Pull Request #291 · dwavesystems/minorminer

mahdiehmalekian · 2026-08-19T00:04:28Z

Adds minorminer.utils._clique.grid, another internal piece of the Zephyr clique embedder, after el_geometry (#289) and maximum_bipartite_matching (#290).

Grid surveys a possibly-faulty Zephyr topology once, at construction, into tables the clique embedder then queries directly. Tests are in tests/utils/clique/test_grid.py.

AI use: I used Claude to clean up and edit documentation of the grid.py and to generate the test_grid.py. Checked and edited myself.

Note to reviewers: This is part of a larger Zephyr clique embedder effort and subsequent clique-embedder PRs depend on this PR and can only follow once this merges. Would appreciate a speedy review.

SebastianGitt

Looks good, but I think many of the methods would be more readable if they had more descriptive variable names.

SebastianGitt

Just took another pass with some higher-level observations and suggestions.

thisac

Thanks @mahdiehmalekian. I'd like to do a more thorough review.

thisac · 2026-09-14T23:08:02Z

+    runs, ...) that the rest of the algorithm queries, rather than re-walking the graph on every
+    lookup.
+
+    Build with Grid.from_graph(graph). See the module docstring for the full list of attributes and


Suggested change

Build with Grid.from_graph(graph). See the module docstring for the full list of attributes and

Build with ``Grid.from_graph(graph)``. See the module docstring for the full list of attributes and

It would make more sense to add the attributes here instead of at the module level. Also the __init__() args are missing.

thisac · 2026-09-14T23:14:43Z

+
+        Coordinates range over ``[0, 4m]``. This only allocates the empty containers; from_graph
+        calls _classify and _run_survey to populate them. Prefer the from_graph factory.
+        """


Should be detailed in the class docstring, along with the arguments. Not here.

thisac · 2026-09-14T23:15:06Z

+        "pos",
+    )
+
+    # ------------------------------------------------------------------ build


Suggested change

# ------------------------------------------------------------------ build

thisac · 2026-09-14T23:24:08Z

+    # attribute types (bare annotations; the storage is __slots__ below)
+    m: int
+    t: int
+    abs_min: int
+    abs_max: int
+    labels: str  # output mode: "int"/"coordinates"/"cartesian"
+    present_qubits: dict[str, dict[Node, int]]  # present_qubits[kind][node] -> linear index r
+    missing_qubits: dict[str, frozenset[Node] | set[Node]]
+    edges: set[tuple[int, int]] | None  # present couplers as (lo, hi) r-pairs
+    missing_internal_couplers: dict | None  # see _missing_internal_couplers
+    runs: Runs | None
+    _el_cache: dict  # el_reachable memo
+    pos: Pos | None


I don't think you need these. Better to add types in __init__() instead.

thisac · 2026-09-14T23:25:54Z

+    __slots__ = (
+        "m",
+        "t",
+        "abs_min",
+        "abs_max",
+        "labels",
+        "present_qubits",
+        "missing_qubits",
+        "edges",
+        "missing_internal_couplers",
+        "runs",
+        "_el_cache",
+        "pos",
+    )


Do you really need __slots__ here? Is it for some gain in attribute access (have you benchmarked this) or something else?

thisac · 2026-09-14T23:53:13Z

+        elif labels == "cartesian":  # nodes are cartesian (x, y, k) already
+            grid._classify_from_present_nodes(set(graph.nodes()))
+            # cartesian edges -> linear r pairs (internal edge set stays linear)
+            edges = set()
+            for ccoord_a, ccoord_b in graph.edges():
+                lcoord_a = grid.cartesian_to_linear(ccoord_a)
+                lcoord_b = grid.cartesian_to_linear(ccoord_b)
+                if lcoord_a is None or lcoord_b is None:
+                    continue
+                edges.add((lcoord_a, lcoord_b) if lcoord_a < lcoord_b else (lcoord_b, lcoord_a))
+            grid.edges = edges
+        else:  # coordinates: nodes are Zephyr 5-tuples
+            grid._classify_from_present_nodes({zephyr_to_cartesian(z) for z in graph.nodes()})
+            # convert coordinate edges -> cartesian -> linear r pairs
+            edges = set()
+            for zcoord_a, zcoord_b in graph.edges():
+                lcoord_a = grid.cartesian_to_linear(zephyr_to_cartesian(zcoord_a))
+                lcoord_b = grid.cartesian_to_linear(zephyr_to_cartesian(zcoord_b))
+                if lcoord_a is None or lcoord_b is None:
+                    continue
+                edges.add((lcoord_a, lcoord_b) if lcoord_a < lcoord_b else (lcoord_b, lcoord_a))
+            grid.edges = edges


This can likely be deduplicated (and a bit cleaned up). E.g., something like

Suggested change

elif labels == "cartesian": # nodes are cartesian (x, y, k) already

grid._classify_from_present_nodes(set(graph.nodes()))

# cartesian edges -> linear r pairs (internal edge set stays linear)

edges = set()

for ccoord_a, ccoord_b in graph.edges():

lcoord_a = grid.cartesian_to_linear(ccoord_a)

lcoord_b = grid.cartesian_to_linear(ccoord_b)

if lcoord_a is None or lcoord_b is None:

continue

edges.add((lcoord_a, lcoord_b) if lcoord_a < lcoord_b else (lcoord_b, lcoord_a))

grid.edges = edges

else: # coordinates: nodes are Zephyr 5-tuples

grid._classify_from_present_nodes({zephyr_to_cartesian(z) for z in graph.nodes()})

# convert coordinate edges -> cartesian -> linear r pairs

edges = set()

for zcoord_a, zcoord_b in graph.edges():

lcoord_a = grid.cartesian_to_linear(zephyr_to_cartesian(zcoord_a))

lcoord_b = grid.cartesian_to_linear(zephyr_to_cartesian(zcoord_b))

if lcoord_a is None or lcoord_b is None:

continue

edges.add((lcoord_a, lcoord_b) if lcoord_a < lcoord_b else (lcoord_b, lcoord_a))

grid.edges = edges

else: # labels either cartesian or 5-tuple

if labels == "coordinates":

convert = zephyr_to_cartesian

else:

convert = lambda n: n

grid._classify_from_present_nodes({convert(n) for n in graph.nodes()})

to_linear = grid.cartesian_to_linear

edges = set()

for node_a, node_b in graph.edges():

lcoord_a = to_linear(convert(node_a))

lcoord_b = to_linear(convert(node_b))

if lcoord_a is None or lcoord_b is None:

continue

if lcoord_a < lcoord_b:

edges.add((lcoord_a, lcoord_b))

else:

edges.add((lcoord_a, lcoord_b))

grid.edges = edges

Note, test before accepting.

thisac · 2026-09-14T23:53:52Z

+        self.abs_min = 0
+        self.abs_max = 4 * m


Should these just be constants instead?

thisac · 2026-09-14T23:55:36Z

+        H_KIND = {1: "h1", 3: "h3"}
+        krange = range(t)
+        OFFS = ((-1, -1), (-1, 1), (1, -1), (1, 1))


Since H_KIND and OFFS are named as (and are) constants, they should be declared outside of the function scope (at the top of the file alongside other constants, and probably renamed to something clearer).

thisac · 2026-09-14T23:57:19Z

+                    f"cannot infer zephyr label mode from tuple node {sample!r}; "
+                    f"expected a 5-tuple (Zephyr) or 3-tuple (cartesian)"
+                )
+        elif isinstance(sample, (int,)) and not isinstance(sample, bool):


Should this also accept NumPy integers? If so, use number.Integral instead of int.

thisac · 2026-09-15T00:03:06Z

+        self.m = m
+        self.t = t
+        self.abs_min = 0
+        self.abs_max = 4 * m
+        self.labels = "int"  # output mode: "int", "coordinates", or "cartesian"
+        self.present_qubits = {k: {} for k in _KINDS}
+        self.missing_qubits = {k: set() for k in _KINDS}
+        self.edges = None
+        # survey outputs (filled by _run_survey)
+        self.missing_internal_couplers = None
+        self.runs = None
+        self._el_cache = {}
+        self.pos = None


Some of these (might) cause errors if from_graph() hasn't been run. E.g., self.edges = None if has_edge() is called before. Probably better to have reasonable values here instead of None or enforce that from_graph() has been called.

Add Grid for surveying Zephyr topologies

d962d9b

mahdiehmalekian force-pushed the grid branch from 38fe2c4 to d962d9b Compare August 19, 2026 00:13

SebastianGitt reviewed Aug 27, 2026

View reviewed changes

Address reviewer's comments

7372533

SebastianGitt reviewed Aug 31, 2026

View reviewed changes

Address reviewer's comments

8e16ff3

SebastianGitt approved these changes Sep 14, 2026

View reviewed changes

thisac requested changes Sep 15, 2026

View reviewed changes

	Build with Grid.from_graph(graph). See the module docstring for the full list of attributes and
	Build with ``Grid.from_graph(graph)``. See the module docstring for the full list of attributes and

Conversation

mahdiehmalekian commented Aug 19, 2026

Uh oh!

SebastianGitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SebastianGitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thisac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants