Add example and tests for fully yielded Zephyr subgraph search by VolodyaCO · Pull Request #45 · dwavesystems/dwave-experimental

VolodyaCO · 2026-04-01T16:56:52Z

Continuation of dwavesystems/dwave-graphs#261

Adds a routine to find embeddings of Zephyr[m, tp] into Zephyr[m, t] with tp < t.

kevinchern

Added minor questions and comments

kevinchern · 2026-04-01T23:07:05Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+
+
+def _extract_graph_properties(source: nx.Graph, target: nx.Graph) -> tuple[int, int, int]:
+    """Extract and validate Zephyr graph properties, returning ``(m, tp, t)``.


I suggest something more descriptive rather than variable names, e.g.,

Suggested change

"""Extract and validate Zephyr graph properties, returning ``(m, tp, t)``.

"""Extract and validate Zephyr graph properties (rows, tile count, and target tile count).

kevinchern · 2026-04-01T23:11:36Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+        tuple[int, int, int]: ``(m, tp, t)`` where ``m`` is rows,
+            ``tp`` is source tile count, and ``t`` is target tile count.


Suggested change

tuple[int, int, int]: ``(m, tp, t)`` where ``m`` is rows,

``tp`` is source tile count, and ``t`` is target tile count.

tuple[int, int, int]: source Zephyr rows, source tile count, and target tile count.

kevinchern · 2026-04-01T23:12:38Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+
+    Raises:
+        TypeError: If metadata values are not integers.
+        ValueError: If graph metadata is missing or incompatible.


I this should detail what it means to be incompatible; incompatibility is implied by ValueError, e.g., positive int or ...

kevinchern · 2026-04-02T02:53:10Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+            for n in _source.nodes()
+            if n in working_embedding
+        )
+    else:


Suggested change

else:

elif yield_type == "edges":

# do stuff

else:

raise ValueError("blah blah")

I recommend being explicit about checking yield type.

Edit: maybe this is fine since validity of yield type has been validated previously...

Other two options are edge and rail-edge. This is fine, as you say, because the type has been validated.

kevinchern · 2026-04-02T03:05:59Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+YieldType = Literal["node", "edge", "rail-edge"]
+QuotientSearchType = Literal["by_quotient_rail", "by_quotient_node", "by_rail_then_node"]


Question!!
I think most of these are only ever used in type-hinting. Should we still define these constant type hints here? Is there a best-practice or principle that recommends this pattern?
Subjectively, I find it less readable than writing the types out explicitly in the type hints.

cc @thisac

Yep. in vscode all it takes is to hover over the constant type hint and it will show more details, though.

kevinchern · 2026-04-02T03:09:33Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+__all__ = ["zephyr_quotient_search"]
+
+ZephyrNode = tuple[int, int, int, int, int]  # (u, w, k, j, z) coordinate tuple
+Embedding = dict[ZephyrNode, ZephyrNode]  # Internal single-node format


Note for considering a different name: dwave.embedding has an EmbeddedStructure class. Might be worth following that pattern but tailored for Zephyr. e.g., ZephyrEmbeddedStructure (ok maybe this is too long lol)

This Embedding is just a short-hand typing tool. The EmbeddedStructure is a bit more convoluted.

kevinchern · 2026-04-02T15:43:12Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+
+    pruned_embedding = {
+        to_source(k): to_target(v) for k, v in working_embedding.items() if v in target_nodeset
+    }  # TODO:?: why would a target node in the working_embedding not be in the target_nodeset?


@VolodyaCO has this question been addressed with @jackraymond ?

It hasn't AFAIR.

SebastianGitt

Looks good! Just have some minor comments.

SebastianGitt · 2026-04-02T18:32:58Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+    tiles. It is designed for defective targets where a direct identity map may lose
+    nodes or edges.
+
+    The search is organised around the **quotient graph** of the Zephyr topology, formed by


Suggested change

The search is organised around the **quotient graph** of the Zephyr topology, formed by

The search is organized around the **quotient graph** of the Zephyr topology, formed by

minor issue but US English should be used

Oh I will have a bunch of these. thanks

SebastianGitt · 2026-04-02T18:55:37Z

tests/test_zephyr_quotient_search.py

+    # Phase 1: uniform random deletion
+    n_uniform = round(proportion * uniform_proportion * N)
+    uniform_indices = rng.choice(N, size=n_uniform, replace=False)
+    deleted = {all_nodes[i] for i in uniform_indices}


consider renaming this to something more clear

Changed to deleted_nodes.

SebastianGitt · 2026-04-02T20:45:02Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+    """Validate that source and target are Zephyr NetworkX graphs.
+
+    Both source and target graphs must be networkx graph instances with a 'family' metadata key
+    set to 'zephyr'. Each graph must also contain 'rows', 'tile' and 'labels metadata keys.


Suggested change

set to 'zephyr'. Each graph must also contain 'rows', 'tile' and 'labels metadata keys.

set to 'zephyr'. Each graph must also contain 'rows', 'tile' and 'labels' metadata keys.

SebastianGitt · 2026-04-02T20:45:32Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+        if expand_boundary_search:
+            if w == 0:
+                ksymmetric = False
+                # brrow candidates from adjacent internal column


Suggested change

# brrow candidates from adjacent internal column

# borrow candidates from adjacent internal column

SebastianGitt · 2026-04-02T20:46:05Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+        \{(u, w_t, k_t, j, z) : j \in \{0,1\},\ z \in \{0,\dots,m-1\}\}.
+
+    We can define its objective for ``yield_type='edge'`` as the number of edges preserved within
+    that rail, i.e., the numbe of edges in the target subgraph induced by the proposed rail, or


Suggested change

that rail, i.e., the numbe of edges in the target subgraph induced by the proposed rail, or

that rail, i.e., the number of edges in the target subgraph induced by the proposed rail, or

SebastianGitt · 2026-04-02T20:46:43Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+
+        F_q = \{m \in V(S) \setminus B_q : m \in \operatorname{dom}(\phi)\},
+
+    where :math:`\phi` is the current embedding`. Then


Suggested change

where :math:`\phi` is the current embedding`. Then

where :math:`\phi` is the current embedding. Then

SebastianGitt · 2026-04-02T20:48:47Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+
+    Args:
+        quotient_search (str): Search mode.
+        yield_type (str): Optimization objective.


Suggested change

yield_type (str): Optimization objective.

yield_type (YieldType): Optimization objective.

This should be a str, as it validates the search parameters. If it is not a YieldType, then an error is raised.

SebastianGitt · 2026-04-02T20:49:07Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+            :math:`w` is at a boundary. Defaults to ``True``.
+        ksymmetric (bool): If ``True``, treat source :math:`k` order as interchangeable when scoring
+            rails. Defaults to ``False``.
+        yield_type (str): ``"node"``, ``"edge"``, or ``"rail-edge"``. Defaults to ``"edge"``.


Suggested change

yield_type (str): ``"node"``, ``"edge"``, or ``"rail-edge"``. Defaults to ``"edge"``.

yield_type (YieldType): ``"node"``, ``"edge"``, or ``"rail-edge"``. Defaults to ``"edge"``.

done, thanks

SebastianGitt · 2026-04-02T21:03:39Z

dwave/experimental/zephyr_embedding_methods/zephyr_quotient_embedding_search.py

+)
+
+
+def _validate_graph_inputs(source: nx.Graph, target: nx.Graph):


Suggested change

def _validate_graph_inputs(source: nx.Graph, target: nx.Graph):

def _validate_graph_inputs(source: nx.Graph, target: nx.Graph) -> None:

SebastianGitt · 2026-04-02T21:10:52Z

tests/test_zephyr_quotient_search.py

+    ZephyrSearchMetadata, zephyr_quotient_search)
+
+
+def generate_faulty_zephyr_graph(m, t, proportion, uniform_proportion, seed=None):


consider adding type hinting here since this function is non-trivial and used in multiple places

Added type hinting

kevinchern

Did a few passes with focus on usability and some design patterns 🚀

Add example and tests for fully yielded Zephyr subgraph search

c18cc51

VolodyaCO mentioned this pull request Apr 1, 2026

Routine to find zephyr[m,tp] in possibly-faulty zephyr[m,t] with tp < t dwavesystems/dwave-graphs#261

Closed

thisac requested review from SebastianGitt, jackraymond and kevinchern April 1, 2026 21:38

kevinchern reviewed Apr 7, 2026

View reviewed changes

SebastianGitt approved these changes Apr 7, 2026

View reviewed changes

Improve documentation of parameters and type hinting

e1bec8f

kevinchern approved these changes Apr 8, 2026

View reviewed changes



		def _extract_graph_properties(source: nx.Graph, target: nx.Graph) -> tuple[int, int, int]:
		"""Extract and validate Zephyr graph properties, returning ``(m, tp, t)``.

		tuple[int, int, int]: ``(m, tp, t)`` where ``m`` is rows,
		``tp`` is source tile count, and ``t`` is target tile count.

	tuple[int, int, int]: ``(m, tp, t)`` where ``m`` is rows,
	``tp`` is source tile count, and ``t`` is target tile count.
	tuple[int, int, int]: source Zephyr rows, source tile count, and target tile count.

-    else:
+    elif yield_type == "edges":
+        # do stuff
+    else:
+        raise ValueError("blah blah")

		YieldType = Literal["node", "edge", "rail-edge"]
		QuotientSearchType = Literal["by_quotient_rail", "by_quotient_node", "by_rail_then_node"]

	The search is organised around the quotient graph of the Zephyr topology, formed by
	The search is organized around the quotient graph of the Zephyr topology, formed by

	set to 'zephyr'. Each graph must also contain 'rows', 'tile' and 'labels metadata keys.
	set to 'zephyr'. Each graph must also contain 'rows', 'tile' and 'labels' metadata keys.

	# brrow candidates from adjacent internal column
	# borrow candidates from adjacent internal column

	that rail, i.e., the numbe of edges in the target subgraph induced by the proposed rail, or
	that rail, i.e., the number of edges in the target subgraph induced by the proposed rail, or


		F_q = \{m \in V(S) \setminus B_q : m \in \operatorname{dom}(\phi)\},

		where :math:`\phi` is the current embedding`. Then

	yield_type (str): Optimization objective.
	yield_type (YieldType): Optimization objective.

	yield_type (str): ``"node"``, ``"edge"``, or ``"rail-edge"``. Defaults to ``"edge"``.
	yield_type (YieldType): ``"node"``, ``"edge"``, or ``"rail-edge"``. Defaults to ``"edge"``.

		)


		def _validate_graph_inputs(source: nx.Graph, target: nx.Graph):

		ZephyrSearchMetadata, zephyr_quotient_search)


		def generate_faulty_zephyr_graph(m, t, proportion, uniform_proportion, seed=None):

Conversation

VolodyaCO commented Apr 1, 2026

Uh oh!

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

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!

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!

kevinchern left a comment

Choose a reason for hiding this comment

Uh oh!