Cube embedding by mahdiehmalekian · Pull Request #280 · dwavesystems/minorminer

mahdiehmalekian · 2025-11-07T00:28:08Z

This PR contains the cube embedder together with the needed tools.

cube_embedding contains perfect_cube_embedding.py which has the cube embedder function for graphs with Zephyr topology.
The rest of cube_embedding folder contains the necessary tools for performing the algorithm.

_lattice_utils contains some tools that can be used for finding the embedding of a variety of lattices (2d or 3d) on Zephyr. These tools are also used in cube_embedding.

SebastianGitt

Looking good, just did a first pass highlighting mostly minor issues before I take a deeper look. I did find what may be a couple of small bugs.

SebastianGitt · 2026-07-03T21:28:13Z

+    _fields = ("u", "w", "k", "j")
+
+    def to_tuple(self) -> tuple[int]:
+        """Rerurns the tuple corresponding to the coordinate."""


Suggested change

"""Rerurns the tuple corresponding to the coordinate."""

"""Returns the tuple corresponding to the coordinate."""

SebastianGitt · 2026-07-03T21:28:33Z

+    _fields = ("u", "w", "j")
+
+    def to_tuple(self) -> tuple[int]:
+        """Rerurns the tuple corresponding to the coordinate."""


Suggested change

"""Rerurns the tuple corresponding to the coordinate."""

"""Returns the tuple corresponding to the coordinate."""

SebastianGitt · 2026-07-03T21:41:26Z

+
+    Provides convenient representations and helpers for embedding-related algorithms.
+    Example:
+    >>> from burnaby.lattice_embedding.auxiliary_coordinates import UWKJ


Suggested change

>>> from burnaby.lattice_embedding.auxiliary_coordinates import UWKJ

>>> from minorminer._lattice_utils.auxiliary_coordinates import UWKJ

SebastianGitt · 2026-07-03T21:41:55Z

+    It captures the full coordinate of a Zephyr node.
+
+    Example:
+    >>> from burnaby.lattice_embedding.auxiliary_coordinates import UWKJZ


Suggested change

>>> from burnaby.lattice_embedding.auxiliary_coordinates import UWKJZ

>>> from minorminer._lattice_utils.auxiliary_coordinates import UWKJZ

SebastianGitt · 2026-07-07T23:01:44Z

+
+    # To make the node label consistent with ``G``'s
+    # (i.e. "int" or "coordinates")
+    if label == "coordinates":


should this be "coordinate" instead of "coordinates"? If so, should add a test using this branch since it wasn't caught, and update references to this in the comments here and elsewhere

SebastianGitt · 2026-07-08T16:22:45Z

+                if isinstance(val, int):
+                    copied_dict[var] = val
+                elif isinstance(val, list):
+                    copied_dict[var] = [k for k in val]


This is a shallow copy but the docstring says it returns a deep copy

SebastianGitt · 2026-07-08T16:27:24Z

+    Returns:
+        dict[Edge, int]: The dictionary after removing all items with value zero.
+    """
+    return {chain: chain_freq for chain, chain_freq in chain_freq.items() if chain_freq != 0}


chain_freq is used for both the dictionary and the dictionary values, consider renaming one of them to make it more clear

SebastianGitt · 2026-07-08T16:30:08Z

+from typing import Generator, NamedTuple
+
+from minorminer.utils.zephyr.node_edge import Edge, NodeKind
+


inconsistent spacing here and elsewhere, I recommend running a code formatter like Black if you haven't already

SebastianGitt · 2026-07-08T16:33:56Z

+    """
+    A helper to represent some information of a path of chains of node indices (within a :class:`QuoTile`).
+    edge (Edge): Edge of node indices represnting the index-chain
+    pos (tuple[int, int]): pair of positions, the i-th element representing the number of occurence of i-th element of the index-chain when visiting the index-chain


should break this up over two lines so it doesn't exceed 100 characters (here and elsewhere)

SebastianGitt · 2026-07-08T16:42:25Z

+   connectivity checks based on z-coupling.
+4. A lattice survey module that provides utilities for embedding lattices
+   on partially-yielded Zephyr graphs.
+5. A qoutient tile object to facilitate embedding lattice-type graphs on Zephyr.


Suggested change

5. A qoutient tile object to facilitate embedding lattice-type graphs on Zephyr.

5. A quotient tile object to facilitate embedding lattice-type graphs on Zephyr.

SebastianGitt

I just did a more in-depth review and it's looking really good overall. I do think this PR would greatly benefit from having some more comprehensive docstrings. I also think there are some opportunities to use more clear/descriptive variable names. Also, make sure to include a release note.

SebastianGitt · 2026-07-08T18:54:18Z

+
+    Args:
+        tile_kind (TileKind): The kind of tile forming the floor.
+        sub_kind (Literal[&quot;main&quot;, &quot;anti&quot;]): The sub-kind of tile forming the floor.


the quotes here need to be fixed (and other occurrences of sub_kind)

Actually, instead of fixing the types in docstrings, please remove them completely (here and elsewhere), since they're not needed when arguments are type-hinted. See the Documentation section in Ocean contributing guide (we follow Google Python Style for docstrings).

Thanks Radomir, I did mention that elsewhere but I missed that these ones should just be removed instead of fixed.

Yes, I saw your comment later below, but only after I posted this comment 🙂

SebastianGitt · 2026-07-08T20:21:38Z

+
+        if z_coupling is ZCoupling.EITHER:
+            alt_edge_pos: dict[EdgePos, EdgePos] = {}
+            for i, ep_01 in enumerate(con_01):


consider using clearer variable names like edge_position instead of ep

SebastianGitt · 2026-07-08T23:16:33Z

+        Edge(5, 7),
+        Edge(0, 1),
+        Edge(1, 3),
+    },


is there a reason "anti" is a set and "main" is a list?

SebastianGitt · 2026-07-08T23:17:01Z

+        Edge(4, 5),
+        Edge(0, 2),
+        Edge(6, 7),
+        Edge(2, 3),


is there a meaningful order to these edges?

SebastianGitt · 2026-07-08T23:19:03Z

+    },
+}
+"""A dictionary whose keys are the two subkinds of ``TileKind.LADDER``--"main", "anti".
+    The value is a dictionary whose


this description should be updated since the values are not dictionaries

SebastianGitt · 2026-07-13T17:14:04Z

+    Returns:
+        int: The maximum number of times ``chain`` can be constructed given the current supply constraints.
+    """
+    used_supply = used_supply or {}


minor, but I would go with
'if used_supply is None:
used_supply = {}'

SebastianGitt · 2026-07-13T17:40:29Z

+            list[ZEdge]: List of edges of the graph induced on the tile, when restricted by ``nbr_kind`` and ``where``.
+        """
+        zns = self._zns
+        tile_coords = [zn.zcoord for zn in zns] if self.convert_to_z else [zn.ccoord for zn in zns]


convert_to_z should be defined as an attribute of QuoTile, this could raise an AttributeError

SebastianGitt · 2026-07-13T17:58:51Z

+    >>> print(len(emb))
+    576
+    """
+


I think it would be worth adding some input validation here such as making sure z_periodic is a boolean, etc.

SebastianGitt · 2026-07-13T18:03:05Z

+            - Defaults to None.
+            - Note: Passing it saves computation time. 
+
+        **kwargs: Optional arguments to specify the type of tiles or paths inside the tiles used for finding embedding.


My recommendation would be to make these explicit keyword arguments instead

SebastianGitt · 2026-07-13T18:04:51Z

+
+def find_cube_embedding(
+    G: nx.Graph | DWaveSampler,
+    dim: tuple[int, int, int] | None=None,


Suggested change

dim: tuple[int, int, int] | None=None,

dim: tuple[int, int, int] | None = None,

Initial commit

2a5604e

mahdiehmalekian force-pushed the cube_embedding branch from 6d0ebcc to 2a5604e Compare January 2, 2026 21:40

SebastianGitt reviewed Jul 8, 2026

View reviewed changes

SebastianGitt reviewed Jul 13, 2026

View reviewed changes

	"""Rerurns the tuple corresponding to the coordinate."""
	"""Returns the tuple corresponding to the coordinate."""

	>>> from burnaby.lattice_embedding.auxiliary_coordinates import UWKJ
	>>> from minorminer._lattice_utils.auxiliary_coordinates import UWKJ

		from typing import Generator, NamedTuple

		from minorminer.utils.zephyr.node_edge import Edge, NodeKind

	5. A qoutient tile object to facilitate embedding lattice-type graphs on Zephyr.
	5. A quotient tile object to facilitate embedding lattice-type graphs on Zephyr.

	dim: tuple[int, int, int] \| None=None,
	dim: tuple[int, int, int] \| None = None,

Uh oh!

Conversation

mahdiehmalekian commented Nov 7, 2025

Uh oh!

SebastianGitt 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!

SebastianGitt Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

SebastianGitt 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!

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

SebastianGitt Jul 7, 2026 •

edited

Loading