Compiler for Code Switching #524
Conversation
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a graph-based code-switching compiler and utilities, command-line scripts to generate and simulate random QASM circuits, orchestration shell scripts, tests, documentation, and a dependency/config update (networkx and .gitignore). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Gen as Generator\n(generate_random_circuits.py)
participant FS as FileSystem
participant Sim as Simulator\n(simulate_circuit_performance.py)
participant Compiler as MinimalCodeSwitchingCompiler
participant Qiskit as Qiskit
User->>Gen: run --n --num_circuits --distr_type
loop per-seed
Gen->>Qiskit: random_universal_circuit(...) -> QuantumCircuit
Qiskit-->>Gen: QuantumCircuit
Gen->>FS: write .qasm file (skip if exists)
end
User->>Sim: run --qasm_path --n --seed --distr_type
Sim->>FS: read .qasm
FS-->>Sim: qasm content
Sim->>Qiskit: loads -> QuantumCircuit
Sim->>Sim: naive_switching(circuit) => naive_count
Sim->>Compiler: build_from_qiskit(circuit, ...)
Compiler->>Compiler: build graph (temporal/bias/idle)
Compiler->>Compiler: compute_min_cut()
Compiler-->>Sim: mincut_count + switch_positions
Sim->>Sim: compute savings, append_to_csv
Sim-->>User: CSV updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-02T07:37:46.860ZApplied to files:
🔇 Additional comments (1)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
50-68: Add networkx to [project.dependencies] as explicit direct dependency.While networkx is currently available transitively through pymatching and qecsim, the code directly imports it (
src/mqt/qecc/circuit_compilation/code_switching_compiler.py:14). Best practice is to explicitly declare all direct imports to avoid fragility from transitive dependency changes.Apply the suggested diff with
networkx>=3.0(covers tested versions 3.4.2 and 3.5 from lock file).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
pyproject.toml(1 hunks)src/mqt/qecc/circuit_compilation/__init__.py(1 hunks)src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/mqt/qecc/circuit_compilation/__init__.py (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (1)
CodeSwitchGraph(20-179)
🪛 Ruff (0.14.1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
178-178: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
🔇 Additional comments (3)
pyproject.toml (1)
156-157: mypy override for networkx looks fine.Adding "networkx.*" to ignore_missing_imports aligns with new NX usage.
src/mqt/qecc/circuit_compilation/__init__.py (1)
12-14: Public re-export looks good.
CodeSwitchGraphis correctly exposed via__all__.src/mqt/qecc/circuit_compilation/code_switching_compiler.py (1)
115-127: Redundant edge additions in add_cnot_links.
add_infinite_edgealready adds both directions; calling it twice duplicates work. Keep one call.Apply this diff:
- self.add_infinite_edge(control_node, target_node) - self.add_infinite_edge(target_node, control_node) + self.add_infinite_edge(control_node, target_node)Likely an incorrect or invalid review comment.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
For this we fixed the arbitrary convention that the 2D Code corresponds to the source and the 3D code to the sink.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (2)
112-133: Fix inline comments that contradict implementation.The inline comments claim the opposite of what the code does:
- Line 129 says "Source code can perform T" but line 130 connects T to sink
- Line 132 says "Sink code can perform H" but line 133 connects H to source
The docstring at lines 115-116 correctly describes the intended behavior (source=2D Color Code with H, sink=3D Surface Code with T).
Apply this diff to fix the comments:
- # Source code can perform T and CNOT gates + # Sink code (3D Surface Code) can perform T gates if gate_type == "T": self.add_infinite_edge(self.sink, node_id) - # Sink code can perform H and CNOT gates + # Source code (2D Color Code) can perform H gates if gate_type == "H": self.add_infinite_edge(node_id, self.source)
186-198: Remove unusednoqadirective.The
# noqa: N806on line 197 is unnecessary because the N806 rule (non-lowercase variable names) is not enabled in your Ruff configuration.Apply this diff:
- cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")Based on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
70-70: Boolean-typed positional argument in function definition
(FBT001)
70-70: Boolean default positional argument in function definition
(FBT002)
86-86: Boolean-typed positional argument in function definition
(FBT001)
86-86: Boolean default positional argument in function definition
(FBT002)
98-98: Boolean-typed positional argument in function definition
(FBT001)
98-98: Boolean default positional argument in function definition
(FBT002)
135-135: Boolean-typed positional argument in function definition
(FBT001)
135-135: Boolean default positional argument in function definition
(FBT002)
147-147: Boolean-typed positional argument in function definition
(FBT001)
147-147: Boolean default positional argument in function definition
(FBT002)
197-197: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
this helps to distinguish between unary edges used for bias and temporal edges used for actual switching locations
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (3)
88-98: Document thebidirectionalparameter.The
bidirectionalparameter is missing from the docstring and should be keyword-only for clarity.This issue was previously flagged in past reviews.
152-162: Document theone_way_transversal_cnotparameter.The
one_way_transversal_cnotparameter is missing from the docstring and should be keyword-only for clarity.This issue was previously flagged in past reviews.
145-150: Fix inline comments that contradict the implementation.The inline comments at lines 145 and 148 are inconsistent with the implementation and the detailed explanation in lines 132-133:
- Line 145 says "Source code can perform T and CNOT gates" but then connects T to sink (line 147)
- Line 148 says "Sink code can perform H and CNOT gates" but then connects H to source (line 150)
According to your convention (lines 132-133), source = 2D Color Code (performs H), sink = 3D Surface Code (performs T), which matches the implementation but not these comments.
Apply this diff to fix the inline comments:
- # Source code can perform T and CNOT gates + # Sink code (3D Surface Code) can perform T gates if gate_type == "T": self.add_infinite_edge(self.sink, node_id) - # Sink code can perform H and CNOT gates + # Source code (2D Color Code) can perform H gates if gate_type == "H": self.add_infinite_edge(node_id, self.source)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
71-71: Boolean-typed positional argument in function definition
(FBT001)
71-71: Boolean default positional argument in function definition
(FBT002)
88-88: Boolean-typed positional argument in function definition
(FBT001)
88-88: Boolean default positional argument in function definition
(FBT002)
100-100: Boolean-typed positional argument in function definition
(FBT001)
100-100: Boolean default positional argument in function definition
(FBT002)
152-152: Boolean-typed positional argument in function definition
(FBT001)
152-152: Boolean default positional argument in function definition
(FBT002)
165-165: Boolean-typed positional argument in function definition
(FBT001)
165-165: Boolean default positional argument in function definition
(FBT002)
165-165: Boolean-typed positional argument in function definition
(FBT001)
165-165: Boolean default positional argument in function definition
(FBT002)
208-208: Boolean-typed positional argument in function definition
(FBT001)
208-208: Boolean default positional argument in function definition
(FBT002)
219-219: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
🔇 Additional comments (2)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (2)
220-231: LGTM: Edge-counting logic correctly handles bidirectional edges.The deduplication using
tuple(sorted((u, v)))as a key (lines 226-227) properly avoids double-counting bidirectional edges when determining the number of code switches. The filtering by edge type and cut-crossing check are also correct.
20-68: Well-structured API for code-switching graph construction.The
CodeSwitchGraphclass provides a clean, composable API with separate methods for adding different edge types (infinite, regular, bias) and node types. The use of NetworkX for graph representation and min-cut computation is appropriate for this domain.
| def add_edge_with_capacity( | ||
| self, u: str, v: str, capacity: float, edge_type: str = "temporal", bidirectional: bool = True | ||
| ) -> None: | ||
| """Add a directed edge with specified capacity between two nodes. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| u : str | ||
| Source node identifier. | ||
| v : str | ||
| Target node identifier. | ||
| capacity : float | ||
| Edge capacity. | ||
| """ | ||
| self.G.add_edge(u, v, capacity=capacity, edge_type=edge_type) | ||
| if bidirectional: | ||
| self.G.add_edge(v, u, capacity=capacity, edge_type=edge_type) |
There was a problem hiding this comment.
Document missing parameters edge_type and bidirectional.
The docstring is missing documentation for two parameters:
edge_type: controls the edge label/categorybidirectional: controls whether edges are added in both directions
Additionally, consider making the bidirectional parameter keyword-only for clarity at call sites.
Apply this diff to complete the documentation:
- def add_edge_with_capacity(
- self, u: str, v: str, capacity: float, edge_type: str = "temporal", bidirectional: bool = True
- ) -> None:
+ def add_edge_with_capacity(
+ self, u: str, v: str, capacity: float, edge_type: str = "temporal", *, bidirectional: bool = True
+ ) -> None:
"""Add a directed edge with specified capacity between two nodes.
Parameters
----------
u : str
Source node identifier.
v : str
Target node identifier.
capacity : float
Edge capacity.
+ edge_type : str, optional
+ Category label for the edge (e.g., "temporal", "entangling", "unary"). Default is "temporal".
+ bidirectional : bool, optional
+ If True, adds edges in both directions (u→v and v→u) with the same capacity.
+ If False, adds only a single directed edge (u→v). Default is True.
"""Based on static analysis hints.
🧰 Tools
🪛 Ruff (0.14.3)
71-71: Boolean-typed positional argument in function definition
(FBT001)
71-71: Boolean default positional argument in function definition
(FBT002)
🤖 Prompt for AI Agents
In src/mqt/qecc/circuit_compilation/code_switching_compiler.py around lines 70
to 86, the add_edge_with_capacity docstring omits documentation for the
edge_type and bidirectional parameters and should also make bidirectional
keyword-only for clarity; update the docstring to include entries for edge_type
(describe it as the edge label/category/default "temporal") and bidirectional
(bool, whether to add the reverse edge/default True), and modify the function
signature to enforce bidirectional as keyword-only (e.g., place a * before it or
use explicit keyword-only syntax) so callers must name that argument.
| def add_regular_edge(self, u: str, v: str, capacity: float = 100.0, bidirectional: bool = True) -> None: | ||
| """Add a regular (finite-capacity) directed edge. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| u : str | ||
| Source node identifier. | ||
| v : str | ||
| Target node identifier. | ||
| capacity : float, optional | ||
| Edge capacity (default is 1.0). | ||
| """ | ||
| self.add_edge_with_capacity(u, v, capacity=capacity, edge_type="temporal", bidirectional=bidirectional) |
There was a problem hiding this comment.
Fix docstring: default capacity is 100.0, not 1.0.
The implementation sets the default capacity to 100.0 (line 100), but the docstring states "default is 1.0" (line 110).
Additionally, the bidirectional parameter is missing from the docstring (previously flagged in past reviews).
Apply this diff to fix the capacity documentation:
capacity : float, optional
- Edge capacity (default is 1.0).
+ Edge capacity (default is 100.0).🧰 Tools
🪛 Ruff (0.14.3)
100-100: Boolean-typed positional argument in function definition
(FBT001)
100-100: Boolean default positional argument in function definition
(FBT002)
🤖 Prompt for AI Agents
In src/mqt/qecc/circuit_compilation/code_switching_compiler.py around lines 100
to 112, the docstring for add_regular_edge incorrectly states the default
capacity is 1.0 and omits the bidirectional parameter; update the docstring to
reflect the actual default capacity of 100.0 and add a brief description of the
bidirectional: bool parameter (what it controls and its default True), keeping
the rest of the parameter descriptions intact and consistent with the function
signature.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (7)
26-33: Correct source/sink mapping in class docstring.Doc claims SRC anchors T/CNOT and SNK anchors H/CNOT, but connect_to_code() actually ties T nodes to SNK (Line 152) and H nodes to SRC (Line 155), leaving CNOT unanchored. Please align the docstring with the implementation so downstream users don’t build the wrong mental model.
- - Source (SRC) and sink (SNK) nodes represent two different codes: - * Source-connected nodes (T, CNOT) → operations that can be done transversally in code A. - * Sink-connected nodes (H, CNOT) → operations that can be done transversally in code B. + - Source (SRC) and sink (SNK) nodes represent two different codes: + * Source-connected nodes (H) → operations transversal in the 2D Color Code. + * Sink-connected nodes (T) → operations transversal in the 3D Surface Code. + * CNOT operations are supported by both codes and remain unanchored.
103-117: Fix add_regular_edge documentation to match implementation.The doc still advertises “a regular directed edge” with default capacity 1.0, but the function defaults to DEFAULT_TEMPORAL_EDGE_CAPACITY (100.0) and, unless bidirectional=False, inserts both directions. Please correct the wording and defaults so the public API description is accurate.
- """Add a regular (finite-capacity) directed edge. + """Add regular (finite-capacity) temporal edge(s) between two nodes. @@ - capacity : float, optional - Edge capacity (default is 1.0). + capacity : float, optional + Edge capacity (default is 100.0 via DEFAULT_TEMPORAL_EDGE_CAPACITY). + bidirectional : bool, optional + If True (default), adds both u→v and v→u with the given capacity; otherwise only u→v.
119-126: Document biased_code correctly.biased_code is a str flag choosing between "SRC" and "SNK", yet the docstring still declares it as float/capacity. Please update the parameter section so users understand which values are valid and what effect they have.
- biased_code : float - Capacity of the biased_code edges to be added. + node_id : str + Node to bias toward one of the terminal codes. + biased_code : str, optional + Either "SRC" or "SNK", indicating which terminal should receive the stronger unary edge (default "SRC").
157-167: Document one_way_transversal_cnot parameter.The optional one_way_transversal_cnot flag flips edge directionality but is absent from the docstring. Please describe its behavior (True ⇒ only control→target, False ⇒ bidirectional) so callers understand when to enable it.
target_node : str Node representing the target qubit's CNOT operation. + one_way_transversal_cnot : bool, optional + If True, adds only the control→target infinite edge. If False (default), + adds both directions to keep the qubits in the same code.
234-245: Document all build_from_qiskit feature flags.The docstring covers one_way_transversal_cnot and code_bias but leaves idle_bonus undocumented, and the descriptions are very high-level. Please add explicit entries so users know what enabling idle_bonus does (idle-aware temporal capacities) and reiterate how code_bias affects unary edges.
code_bias : bool, optional - If True, add bias edges for CNOT nodes. + If True, add bias edges for CNOT nodes to prefer one code over the other. + idle_bonus : bool, optional + If True, reduce temporal edge capacity after idle stretches via _edge_capacity_with_idle_bonus.
284-311: Fix compute_min_cut signature, docs, and unused noqa.With return_raw_data=False (the default), the method returns (num_switches: int, S, T), not a float. The signature and Returns section should reflect this union, and the parameter needs documenting. While touching the block, please drop the now-unused
# noqa.- def compute_min_cut(self, return_raw_data: bool = False) -> tuple[float, set[str], set[str]]: + def compute_min_cut(self, *, return_raw_data: bool = False) -> tuple[int | float, set[str], set[str]]: @@ - Returns: + Parameters + ---------- + return_raw_data : bool, optional + If True, return the raw NetworkX cut capacity (float). If False (default), + return the counted number of switching edges (int). + + Returns ------- - Tuple[float, Set[str], Set[str]] - A tuple (cut_value, S, T) where: - - cut_value is the total capacity of the minimum cut, + tuple[int | float, set[str], set[str]] + A tuple (cut_value_or_count, S, T) where: + - cut_value_or_count is the raw capacity (return_raw_data=True) or number of switches (False), @@ - cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")
73-90: Docstring must reflect bidirectional edge behavior.add_edge_with_capacity() says it adds “a directed edge”, yet with bidirectional=True (the default) it installs both u→v and v→u and also accepts an
edge_typelabel. Please update the docstring (and ideally the summary line) so call sites know the method creates reciprocal edges unless they explicitly pass bidirectional=False.- """Add a directed edge with specified capacity between two nodes. + """Add edges with specified capacity between two nodes. @@ capacity : float Edge capacity. + edge_type : str, optional + Edge category stored on the graph (default "temporal"). + bidirectional : bool, optional + If True (default), adds both u→v and v→u with the same capacity and edge_type. + If False, adds only the directed edge u→v.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
74-74: Boolean-typed positional argument in function definition
(FBT001)
74-74: Boolean default positional argument in function definition
(FBT002)
91-91: Boolean-typed positional argument in function definition
(FBT001)
91-91: Boolean default positional argument in function definition
(FBT002)
104-104: Boolean-typed positional argument in function definition
(FBT001)
104-104: Boolean default positional argument in function definition
(FBT002)
157-157: Boolean-typed positional argument in function definition
(FBT001)
157-157: Boolean default positional argument in function definition
(FBT002)
224-224: Boolean-typed positional argument in function definition
(FBT001)
224-224: Boolean default positional argument in function definition
(FBT002)
225-225: Boolean-typed positional argument in function definition
(FBT001)
225-225: Boolean default positional argument in function definition
(FBT002)
226-226: Boolean-typed positional argument in function definition
(FBT001)
226-226: Boolean default positional argument in function definition
(FBT002)
284-284: Boolean-typed positional argument in function definition
(FBT001)
284-284: Boolean default positional argument in function definition
(FBT002)
295-295: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (8)
26-33: Fix class docstring to reflect actual code anchors.The docstring still claims T/CNOT attach to the source and H/CNOT to the sink, and mentions a 1.0 default capacity. The implementation anchors T nodes to the sink, H nodes to the source, and uses
DEFAULT_TEMPORAL_EDGE_CAPACITY = 100.0. Please sync the documentation with the code.@@ - - Source (SRC) and sink (SNK) nodes represent two different codes: - * Source-connected nodes (T, CNOT) → operations that can be done transversally in code A. - * Sink-connected nodes (H, CNOT) → operations that can be done transversally in code B. + - Source (SRC) and sink (SNK) nodes represent two different codes: + * Source-connected nodes (H) → operations that can be done transversally in the 2D Color Code. + * Sink-connected nodes (T) → operations that can be done transversally in the 3D Surface Code. + * CNOT operations are supported by both codes and remain unanchored. @@ - - Finite-capacity edges (default 1.0) represent potential code transitions along qubit timelines. + - Finite-capacity edges (default 100.0) represent potential code transitions along qubit timelines.
73-90: Document edge_type/bidirectional and clarify bidirectional behavior.
add_edge_with_capacityadds both directions by default and acceptsedge_type, but neither parameter is documented. Please update the docstring and make it clear this method creates reciprocal edges whenbidirectional=True.- def add_edge_with_capacity( - self, u: str, v: str, capacity: float, edge_type: str = "temporal", bidirectional: bool = True - ) -> None: - """Add a directed edge with specified capacity between two nodes. + def add_edge_with_capacity( + self, u: str, v: str, capacity: float, edge_type: str = "temporal", bidirectional: bool = True + ) -> None: + """Add an edge (optionally bidirectional) with specified capacity between two nodes. @@ capacity : float Edge capacity. + edge_type : str, optional + Label describing the edge category (e.g., "temporal", "entangling", "unary"). Default is "temporal". + bidirectional : bool, optional + If True (default), also creates the reverse edge (v→u) with the same capacity and edge_type. + If False, only the edge u→v is added.
91-101: Explain bidirectional semantics for infinite edges.
add_infinite_edgeforwardsbidirectionaltoadd_edge_with_capacity, but the docstring omits it. Documenting this parameter avoids misuse.def add_infinite_edge(self, u: str, v: str, bidirectional: bool = True) -> None: """Add an edge of infinite capacity between two nodes. @@ v : str Target node identifier. + bidirectional : bool, optional + If True (default), also adds the reverse infinite-capacity edge (v→u). If False, only u→v is added.
103-118: Fix default capacity note and document bidirectional flag.
DEFAULT_TEMPORAL_EDGE_CAPACITYis 100.0 andbidirectionalcontrols reverse edges, yet the docstring still states a 1.0 default and omits the flag.def add_regular_edge( self, u: str, v: str, capacity: float = DEFAULT_TEMPORAL_EDGE_CAPACITY, bidirectional: bool = True ) -> None: """Add a regular (finite-capacity) directed edge. @@ capacity : float, optional - Edge capacity (default is 1.0). + Edge capacity (default is 100.0). + bidirectional : bool, optional + If True (default), also adds the reverse edge (v→u) with the same capacity.
119-132: Correctadd_bias_edgesparameter docs.The docstring lists only
biased_code(wrongly typed as float) and omitsnode_id. Please match the signature and explain the string options.- def add_bias_edges(self, node_id: str, biased_code: str = "SRC") -> None: - """Add biased_code unary edges to the terminal nodes slightly preferring one code over the other. + def add_bias_edges(self, node_id: str, biased_code: str = "SRC") -> None: + """Add unary edges to bias a node toward one code or the other. @@ - biased_code : float - Capacity of the biased_code edges to be added. + node_id : str + Node identifier receiving the bias edges. + biased_code : str, optional + Which code to favor. Use "SRC" (default) to bias toward the source/2D Color Code, + or "SNK" to bias toward the sink/3D Surface Code.
150-155: Align inline comments with actual anchoring behavior.These comments say the source handles T and the sink handles H, but the implementation (and surrounding docstring) do the opposite. Please update the comments to avoid misleading future maintainers.
- # Source code can perform T and CNOT gates + # Sink code anchors T gates @@ - # Sink code can perform H and CNOT gates + # Source code anchors H gates
157-167: Documentone_way_transversal_cnot.This flag changes the topology (unidirectional vs bidirectional edges) yet the docstring never mentions it. Add a brief description so callers know when to enable it.
def add_cnot_links(self, control_node: str, target_node: str, one_way_transversal_cnot: bool = False) -> None: """Add bidirectional infinite-capacity edges between two CNOT-related nodes to enforce that both qubits remain in the same code. @@ target_node : str Node representing the target qubit's CNOT operation. + one_way_transversal_cnot : bool, optional + If True, add only the control→target edge (one-way transversal CNOT). If False (default), + add edges in both directions to keep the qubits in the same code.
284-311: Aligncompute_min_cutsignature/docs with behavior and drop unused noqa.
return_raw_dataisn’t documented, the return type changes between raw/processed modes, and the# noqais redundant. Please update the signature, docstring, and remove the directive.- def compute_min_cut(self, return_raw_data: bool = False) -> tuple[float, set[str], set[str]]: + def compute_min_cut(self, return_raw_data: bool = False) -> tuple[float | int, set[str], set[str]]: """Compute the minimum s-t cut between the source and sink. + Parameters + ---------- + return_raw_data : bool, optional + If True, return the raw min-cut capacity from NetworkX (float). + If False (default), return the number of temporal/entangling edges crossing the cut (int). + Returns: ------- - Tuple[float, Set[str], Set[str]] - A tuple (cut_value, S, T) where: - - cut_value is the total capacity of the minimum cut, + tuple[float | int, set[str], set[str]] + A tuple (cut_value, S, T) where: + - cut_value is either the raw cut capacity (return_raw_data=True) or the number of switches (return_raw_data=False), - S is the set of nodes reachable from the source, - T is the complementary set of nodes. """ - cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
74-74: Boolean-typed positional argument in function definition
(FBT001)
74-74: Boolean default positional argument in function definition
(FBT002)
91-91: Boolean-typed positional argument in function definition
(FBT001)
91-91: Boolean default positional argument in function definition
(FBT002)
104-104: Boolean-typed positional argument in function definition
(FBT001)
104-104: Boolean default positional argument in function definition
(FBT002)
157-157: Boolean-typed positional argument in function definition
(FBT001)
157-157: Boolean default positional argument in function definition
(FBT002)
224-224: Boolean-typed positional argument in function definition
(FBT001)
224-224: Boolean default positional argument in function definition
(FBT002)
225-225: Boolean-typed positional argument in function definition
(FBT001)
225-225: Boolean default positional argument in function definition
(FBT002)
226-226: Boolean-typed positional argument in function definition
(FBT001)
226-226: Boolean default positional argument in function definition
(FBT002)
284-284: Boolean-typed positional argument in function definition
(FBT001)
284-284: Boolean default positional argument in function definition
(FBT002)
295-295: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
prior every edge with nodes in different codes were counted but this included one-way
This reverts commit 73ffd1c.
now infinite edges are of type "fixed" and won't be count
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (2)
120-121: Specify the default capacity value in the docstring.The docstring for the
capacityparameter doesn't mention its default value, making it unclear to users what capacity will be used if not specified.Apply this diff:
capacity : float, optional - Edge capacity. + Edge capacity (default is 100.0).
127-140: Fix parameter documentation: wrong type and missing parameter.The docstring has two critical issues:
biased_codeis documented asfloatbut the signature shows it'sstrwith default"SRC"- The
node_idparameter is completely missing from the docstring- The description "Capacity of the biased_code edges" is confusing—it should describe which code to bias toward
Apply this diff:
"""Add biased_code unary edges to the terminal nodes slightly preferring one code over the other. Parameters ---------- - biased_code : float - Capacity of the biased_code edges to be added. + node_id : str + Node identifier to which bias edges will be added. + biased_code : str, optional + Which code to bias toward: "SRC" (source/2D Color Code) or "SNK" (sink/3D Surface Code). + Default is "SRC". """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
74-74: Boolean-typed positional argument in function definition
(FBT001)
74-74: Boolean default positional argument in function definition
(FBT002)
95-95: Boolean-typed positional argument in function definition
(FBT001)
95-95: Boolean default positional argument in function definition
(FBT002)
110-110: Boolean-typed positional argument in function definition
(FBT001)
110-110: Boolean default positional argument in function definition
(FBT002)
165-165: Boolean-typed positional argument in function definition
(FBT001)
165-165: Boolean default positional argument in function definition
(FBT002)
234-234: Boolean-typed positional argument in function definition
(FBT001)
234-234: Boolean default positional argument in function definition
(FBT002)
235-235: Boolean-typed positional argument in function definition
(FBT001)
235-235: Boolean default positional argument in function definition
(FBT002)
236-236: Boolean-typed positional argument in function definition
(FBT001)
236-236: Boolean default positional argument in function definition
(FBT002)
297-297: Boolean-typed positional argument in function definition
(FBT001)
297-297: Boolean default positional argument in function definition
(FBT002)
308-308: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
| def compute_min_cut(self, return_raw_data: bool = False) -> tuple[float, set[str], set[str]]: | ||
| """Compute the minimum s-t cut between the source and sink. | ||
|
|
||
| Returns: | ||
| ------- | ||
| Tuple[float, Set[str], Set[str]] | ||
| A tuple (cut_value, S, T) where: | ||
| - cut_value is the total capacity of the minimum cut, | ||
| - S is the set of nodes reachable from the source, | ||
| - T is the complementary set of nodes. | ||
| """ | ||
| cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 | ||
| num_switches = 0 | ||
| switch_cost = 0 | ||
| seen = set() | ||
| for u, v, data in self.G.edges(data=True): | ||
| if data["edge_type"] == "temporal": | ||
| # needed to avoid double counting edges in undirected sense | ||
| key = tuple(sorted((u, v))) | ||
| if key not in seen: | ||
| if (u in S and v in T) or (v in S and u in T): | ||
| num_switches += 1 | ||
| switch_cost += data["capacity"] | ||
| seen.add(key) | ||
| if return_raw_data: | ||
| return cut_value, S, T | ||
| return num_switches, S, T |
There was a problem hiding this comment.
Document missing parameter, fix return type, and remove unused noqa.
Three issues in this method:
- The
return_raw_dataparameter (line 297) is missing from the docstring - The return type annotation
tuple[float, set[str], set[str]]is incomplete—the method returns different first elements depending onreturn_raw_data:- If
True: returns(cut_value: float, S, T) - If
False: returns(num_switches: int, S, T)
- If
- Line 308 has an unused
noqa: N806directive (flagged by Ruff RUF100)
Apply this diff:
- def compute_min_cut(self, return_raw_data: bool = False) -> tuple[float, set[str], set[str]]:
+ def compute_min_cut(self, return_raw_data: bool = False) -> tuple[float | int, set[str], set[str]]:
"""Compute the minimum s-t cut between the source and sink.
+ Parameters
+ ----------
+ return_raw_data : bool, optional
+ If True, returns the raw cut value (float) from NetworkX.
+ If False (default), returns the number of code switches (int).
+
Returns:
-------
- Tuple[float, Set[str], Set[str]]
+ tuple[float | int, set[str], set[str]]
A tuple (cut_value, S, T) where:
- - cut_value is the total capacity of the minimum cut,
+ - cut_value is either the raw cut capacity (float, if return_raw_data=True)
+ or the count of temporal edges crossing the cut (int, if return_raw_data=False),
- S is the set of nodes reachable from the source,
- T is the complementary set of nodes.
"""
- cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806
+ cut_value, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")Based on static analysis hints.
🧰 Tools
🪛 Ruff (0.14.3)
297-297: Boolean-typed positional argument in function definition
(FBT001)
297-297: Boolean default positional argument in function definition
(FBT002)
308-308: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In src/mqt/qecc/circuit_compilation/code_switching_compiler.py around lines 297
to 323, update the docstring to document the return_raw_data parameter and its
effect, change the return type annotation to reflect that the first tuple
element can be float (cut_value) or int (num_switches) — e.g. use Union[float,
int] or PEP 604 syntax (float | int) — and remove the unused "# noqa: N806" on
the nx.minimum_cut line; keep the logic unchanged but ensure the docstring
explains both return shapes and the type annotation matches both branches.
now we can add to a given qiskit circuit the switching operations as placeholder operations with a given depth to simulate how much switching operations increase circuit depth
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/mqt/qecc/circuit_compilation/compilation_utils.py (2)
84-170: Remove stale parameter documentation.Lines 105-108: The docstring documents an
expand_placeholderparameter that doesn't exist in the function signature (line 84). This was flagged in a previous review (claimed fixed in commit 2e4e588) but is still present.Apply this diff:
placeholder_depth : int, optional Virtual depth (single-qubit layers) the placeholder should represent. - expand_placeholder : bool, optional - If True, expand each placeholder into `placeholder_depth` calls to - `QuantumCircuit.id(qubit)` so that `QuantumCircuit.depth()` increases. - If False, append a single `SwitchGate` marker (informational only). Returns:Otherwise, the implementation is solid:
- ✅ Correctly builds DAG and extracts layers
- ✅ Groups placeholders by qubit and sorts for determinism
- ✅ Inserts placeholders after target layers
- ✅ Handles out-of-bounds layers by appending at end
- ✅ Preserves circuit registers and naming
15-81: Fix docstring mismatch: returns list, not dict.Line 25: The docstring claims
"dict: final code assignment per qubit"but the function returnstuple[int, list[str | None]]. This was flagged in a previous review (addressed in commit 2e4e588) but the docstring still hasn't been corrected.Apply this diff:
Returns: int: total number of code switches - dict: final code assignment per qubit + list[str | None]: final code assignment per qubit (indexed by qubit number)Otherwise, the implementation is correct:
- ✅ Properly initializes codes from first gates
- ✅ Synchronizes codes for multi-qubit gates
- ✅ Handles ID gates as neutral operations
- ✅ Raises ValueError for unsupported gates
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (1)
375-433: Remove unusednoqadirectives.The
# noqa: N806(line 387) and# noqa: N803(line 391) directives are unused because the N806 and N803 naming convention rules are not enabled in the project's Ruff configuration. Remove them:- _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")- def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]: # noqa: N803 + def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]:Regarding the node extraction at lines 420–422: the current implementation is correct. Temporal edges are bidirectional, and the deduplication logic properly handles both directions. Either endpoint of a crossing edge is a valid insertion point; the comment's "e.g." appropriately acknowledges this choice is approximate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
scripts/cs_compiler/generate_random_circuits.py(1 hunks)scripts/cs_compiler/run_generate_circuits.sh(1 hunks)scripts/cs_compiler/simulate_circuit_performance.py(1 hunks)src/mqt/qecc/circuit_compilation/__init__.py(1 hunks)src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)src/mqt/qecc/circuit_compilation/compilation_utils.py(1 hunks)tests/circuit_compilation/test_code_switching_compilation.py(1 hunks)tests/circuit_compilation/test_compilation_utils.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.
Applied to files:
tests/circuit_compilation/test_code_switching_compilation.pysrc/mqt/qecc/circuit_compilation/code_switching_compiler.py
📚 Learning: 2025-11-27T21:26:39.677Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 846
File: python/mqt/qmap/plugins/qiskit/sc/load_calibration.py:34-34
Timestamp: 2025-11-27T21:26:39.677Z
Learning: In the qmap project, the Ruff linter has the "PL" (pylint) rule category enabled, which includes PLC0415 (import-outside-top-level). Therefore, `# noqa: PLC0415` directives on lazy imports are appropriate and necessary, not unused.
Applied to files:
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
🧬 Code graph analysis (6)
tests/circuit_compilation/test_compilation_utils.py (2)
src/mqt/qecc/circuit_compilation/compilation_utils.py (2)
insert_switch_placeholders(84-170)naive_switching(15-81)src/mqt/qecc/circuit_synthesis/circuits.py (1)
depth(333-344)
src/mqt/qecc/circuit_compilation/__init__.py (2)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (2)
CompilerConfig(27-33)MinimalCodeSwitchingCompiler(36-433)src/mqt/qecc/circuit_compilation/compilation_utils.py (2)
insert_switch_placeholders(84-170)naive_switching(15-81)
src/mqt/qecc/circuit_compilation/compilation_utils.py (1)
src/mqt/qecc/circuit_synthesis/circuits.py (1)
num_qubits(250-257)
scripts/cs_compiler/generate_random_circuits.py (2)
src/mqt/qecc/circuit_synthesis/circuits.py (2)
num_qubits(250-257)depth(333-344)scripts/cs_compiler/simulate_circuit_performance.py (1)
main(88-106)
tests/circuit_compilation/test_code_switching_compilation.py (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (2)
CompilerConfig(27-33)parse_node_id(426-433)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (1)
src/mqt/qecc/circuit_synthesis/circuits.py (2)
depth(333-344)num_qubits(250-257)
🪛 Ruff (0.14.6)
tests/circuit_compilation/test_code_switching_compilation.py
45-45: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
65-65: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
75-75: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
307-307: Boolean-typed positional argument in function definition
(FBT001)
308-308: Boolean-typed positional argument in function definition
(FBT001)
343-343: Boolean-typed positional argument in function definition
(FBT001)
387-387: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
391-391: Unused noqa directive (non-enabled: N803)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
🔇 Additional comments (11)
scripts/cs_compiler/run_generate_circuits.sh (1)
1-32: LGTM! Script is well-structured and all previous issues resolved.The bash script correctly:
- Constructs n_values dynamically (64 to 1024 in steps of 64)
- Properly quotes array expansions in the parallel invocation
- Forwards distr_type to the Python script as requested in previous reviews
- Exports necessary variables and functions for parallel execution
scripts/cs_compiler/generate_random_circuits.py (2)
124-172: LGTM! Circuit generation function is well-implemented.The function correctly:
- Validates gate distribution type against allowed options (addressed previous review)
- Creates output directories with
parents=True, exist_ok=True- Skips existing QASM files to support incremental generation
- Uses corrected progress logging (
seed + 1as fixed in previous reviews)- Writes QASM 2.0 format via
qiskit.qasm2.dumps
175-192: LGTM! CLI entry point is clean and well-documented.The argument parser provides sensible defaults and clear help text for all parameters. The integration with
generate_circuitsis straightforward.src/mqt/qecc/circuit_compilation/__init__.py (1)
1-20: Public API exports are clean and well-structured.The package correctly exposes:
CompilerConfigandMinimalCodeSwitchingCompilerfrom the compiler moduleinsert_switch_placeholdersandnaive_switchingfrom utilitiesThe
__all__list matches the imports, ensuring a clear public API surface.Note: A previous review suggested the module name
circuit_compilationmight be confusing given the existingcircuit_synthesismodule, proposingcode_switchinginstead. This is a valid architectural consideration but doesn't block the current implementation.src/mqt/qecc/circuit_compilation/code_switching_compiler.py (3)
26-90: LGTM! Configuration and initialization are well-structured.The
CompilerConfigdataclass provides clear configuration parameters with sensible defaults. TheMinimalCodeSwitchingCompiler.__init__correctly:
- Initializes config with a default
CompilerConfig()when None- Computes common gates as the intersection of source and sink gate sets
- Initializes the NetworkX directed graph with source and sink nodes
- Calculates
base_unary_capacityfrom config parameters
92-184: LGTM! Node and edge management methods are correctly implemented.The graph construction methods are well-designed:
_add_gate_nodecreates properly formatted node IDs (e.g., "H_q0_d3")_add_edge_with_capacitysupports both directed and bidirectional edges_add_infinite_edgeand_add_regular_edgeprovide convenient wrappers_add_bias_edgescorrectly applies differential capacities based onbiased_codeAll boolean parameters are correctly keyword-only (following the
*separator in signatures), addressing previous review feedback.
247-373: LGTM! Graph construction logic is well-structured.The
build_from_qiskitmethod correctly:
- Extracts DAG layers from the input circuit
- Counts total temporal edges for idle bonus calculations
- Delegates gate processing to
_process_gate_operation- Applies code constraints via
_apply_code_constraintsKey behaviors:
- One-way gates: Multi-qubit gates in
one_way_gatesuse unidirectional infinite edges- Code bias: Adds bias edges to common gates when enabled
- Idle bonus: Reduces temporal edge capacities based on idle durations
The separation of concerns across helper methods enhances readability.
Note: Static analysis flags boolean parameters in private methods (
_process_gate_operation,_apply_code_constraints), but keyword-only enforcement for internal APIs is optional.tests/circuit_compilation/test_compilation_utils.py (2)
23-84: LGTM! Naive switching tests provide good coverage.The test suite validates:
- ✅ Simple single-qubit transitions (H→T)
- ✅ No-switch scenarios (staying in one code)
- ✅ CNOT synchronization forcing switches
- ✅ Invalid gate error handling
- ✅ ID gate ignoring
The assertions are appropriate. As noted in past reviews, you could optionally assert
final_codesin the CNOT and ID tests for more complete verification, but current coverage is sufficient.
91-163: LGTM! Placeholder insertion tests are thorough and well-implemented.The test suite correctly validates:
- ✅ Depth increases by placeholder_depth
- ✅ Per-qubit targeting (addressed previous review—now checks
id_targets == {1})- ✅ Out-of-bounds layers append at end
- ✅ Multiple placeholders maintain correct operation order
The assertions are precise and will catch regressions in placeholder logic.
tests/circuit_compilation/test_code_switching_compilation.py (1)
87-166: LGTM! Integration tests provide excellent coverage of compiler behavior.The test suite validates key scenarios:
- ✅ Forced switches (H→T requires 1 switch)
- ✅ No switches (H→H→CX stays in source code)
- ✅ One-way transversal (verifies switch reduction when enabled)
- ✅ Code bias (verifies bias affects switch placement)
Test assertions are precise and verify both switch counts and positions. The one-way transversal test (lines 121-141) nicely demonstrates the feature by comparing with/without the flag.
scripts/cs_compiler/simulate_circuit_performance.py (1)
1-110: LGTM! Performance simulation script is well-implemented.All previous review issues have been addressed:
✅ Lines 8-17: Module docstring updated with correct script name and paths
✅ Line 37:append_to_csvcreates parent directories before writing
✅ Lines 61-63: Variable names (t0_naive,t1_naive) match the output key (t_naive)The script correctly:
- Loads QASM circuits and deduplicates by seed
- Computes both naive switching and min-cut based switching
- Records timing, absolute, and relative savings
- Handles CSV I/O safely with header generation
The comparison workflow integrates well with
run_performance_simulations.shorchestration.
| def compute_idle_bonus(self, previous_depth: int, current_depth: int, total_edges: int) -> float: | ||
| """Compute a normalized bonus (capacity reduction) for idling qubits. | ||
|
|
||
| Formula: idle_time / (total_edges * (idle_time + 1)) | ||
|
|
||
| This creates a 'micro-bias'. It ensures that the bonus is always | ||
| significantly smaller than the base capacity, acting only as a | ||
| tie-breaker for the min-cut algorithm rather than forcing a cut. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| previous_depth : int | ||
| The depth index of the previous active gate. | ||
| current_depth : int | ||
| The depth index of the current active gate. | ||
| total_edges : int | ||
| The total count of temporal edges (qubit-operations) in the circuit. | ||
|
|
||
| Returns: | ||
| ------- | ||
| float | ||
| The capacity reduction to apply. | ||
| """ | ||
| idle_length = max(0, current_depth - previous_depth - 1) | ||
|
|
||
| if idle_length <= self.config.switching_time: | ||
| return 0.0 | ||
|
|
||
| if total_edges == 0: | ||
| return 0.0 | ||
|
|
||
| return idle_length / (total_edges * (idle_length + 1)) | ||
|
|
||
| def _edge_capacity_with_idle_bonus( | ||
| self, depths: list[int], total_edges: int, base_capacity: float | None = None | ||
| ) -> float: | ||
| """Compute the effective temporal edge capacity. | ||
|
|
||
| Optionally reduced by an idle bonus if the qubit has been inactive for several layers. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| depths : list[int] | ||
| The ordered list of depth indices for a given qubit's gates. | ||
| base_capacity : float, optional | ||
| The default temporal edge capacity. | ||
|
|
||
| Returns: | ||
| ------- | ||
| float | ||
| The adjusted edge capacity. | ||
| """ | ||
| if base_capacity is None: | ||
| base_capacity = self.config.default_temporal_edge_capacity | ||
| if len(depths) < 2: | ||
| return base_capacity | ||
|
|
||
| prev_depth, curr_depth = depths[-2], depths[-1] | ||
| bonus = self.compute_idle_bonus(prev_depth, curr_depth, total_edges) | ||
| return base_capacity - bonus |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Idle bonus logic is sound, but consider adding safeguards.
The normalized idle bonus formula idle_length / (total_edges * (idle_length + 1)) ensures the bonus is always less than 1/total_edges, preventing excessive capacity reduction in typical cases.
However, _edge_capacity_with_idle_bonus returns base_capacity - bonus without clamping. While the formula bounds the bonus appropriately for default parameters, edge cases could theoretically produce negative capacities if base_capacity is manually set to a very small value. Since NetworkX's min-cut algorithms assume non-negative capacities, consider adding a safeguard:
prev_depth, curr_depth = depths[-2], depths[-1]
bonus = self.compute_idle_bonus(prev_depth, curr_depth, total_edges)
- return base_capacity - bonus
+ return max(0.0, base_capacity - bonus)This ensures capacities remain non-negative without changing behavior for well-configured instances.
🤖 Prompt for AI Agents
In src/mqt/qecc/circuit_compilation/code_switching_compiler.py around lines 186
to 245, the edge capacity returned from _edge_capacity_with_idle_bonus subtracts
the idle bonus from base_capacity but does not ensure a non-negative result; to
fix, clamp the final capacity to a minimum of 0.0 (e.g., replace direct
subtraction with a max(base_capacity - bonus, 0.0)), and optionally validate or
coerce negative/None base_capacity inputs to a sensible non-negative default
before applying the bonus so NetworkX min-cut algorithms never receive negative
capacities.
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (5)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (2)
387-387: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100) because the N806 rule is not enabled in the project configuration.Apply this diff:
- _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")Based on static analysis hints.
391-391: Remove unusednoqadirective.Ruff flags
# noqa: N803as unused (RUF100) because the N803 rule is not enabled in the project configuration.Apply this diff:
- def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]: # noqa: N803 + def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]:Based on static analysis hints.
src/mqt/qecc/circuit_compilation/compilation_utils.py (3)
105-108: Remove stale parameter documentation.The
expand_placeholderparameter is documented but does not exist in the function signature (line 84). This will confuse users.Apply this diff:
placeholder_depth : int, optional Virtual depth (single-qubit layers) the placeholder should represent. - expand_placeholder : bool, optional - If True, expand each placeholder into `placeholder_depth` calls to - `QuantumCircuit.id(qubit)` so that `QuantumCircuit.depth()` increases. - If False, append a single `SwitchGate` marker (informational only). Returns:
23-26: Fix return type documentation.The docstring states the function returns a
dict(line 25), but the implementation and type hint show it returnstuple[int, list[str | None]]. The second element is a list, not a dict.Apply this diff:
Returns: - int: total number of code switches - dict: final code assignment per qubit + tuple[int, list[str | None]]: + - int: total number of code switches + - list: final code assignment per qubit (index = qubit index, value = code or None)
119-124: Consider validating qubit indices for better error messages.Invalid
qidxvalues inswitch_positionswill cause anIndexErrorwhen accessingcircuit.qubits[q_index]at line 138. For a public API, explicit validation would provide clearer error messages.Apply this diff to add validation:
# Normalize and group requested placeholders by qubit placeholders_by_qubit: dict[int, list[int]] = {} for qidx, layer_idx in switch_positions: + if not 0 <= qidx < circuit.num_qubits: + msg = f"Invalid qubit index {qidx} (circuit has {circuit.num_qubits} qubits)" + raise ValueError(msg) if layer_idx < 0: continue placeholders_by_qubit.setdefault(qidx, []).append(layer_idx)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
scripts/cs_compiler/generate_random_circuits.py(1 hunks)scripts/cs_compiler/run_generate_circuits.sh(1 hunks)scripts/cs_compiler/simulate_circuit_performance.py(1 hunks)src/mqt/qecc/circuit_compilation/__init__.py(1 hunks)src/mqt/qecc/circuit_compilation/code_switching_compiler.py(1 hunks)src/mqt/qecc/circuit_compilation/compilation_utils.py(1 hunks)tests/circuit_compilation/test_code_switching_compilation.py(1 hunks)tests/circuit_compilation/test_compilation_utils.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.
Applied to files:
tests/circuit_compilation/test_code_switching_compilation.pysrc/mqt/qecc/circuit_compilation/code_switching_compiler.py
📚 Learning: 2025-11-27T21:26:39.677Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 846
File: python/mqt/qmap/plugins/qiskit/sc/load_calibration.py:34-34
Timestamp: 2025-11-27T21:26:39.677Z
Learning: In the qmap project, the Ruff linter has the "PL" (pylint) rule category enabled, which includes PLC0415 (import-outside-top-level). Therefore, `# noqa: PLC0415` directives on lazy imports are appropriate and necessary, not unused.
Applied to files:
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
🧬 Code graph analysis (4)
tests/circuit_compilation/test_code_switching_compilation.py (1)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (6)
CompilerConfig(27-33)parse_node_id(426-433)compute_idle_bonus(186-217)_edge_capacity_with_idle_bonus(219-245)build_from_qiskit(247-297)compute_min_cut(375-389)
src/mqt/qecc/circuit_compilation/__init__.py (2)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (2)
CompilerConfig(27-33)MinimalCodeSwitchingCompiler(36-433)src/mqt/qecc/circuit_compilation/compilation_utils.py (2)
insert_switch_placeholders(84-170)naive_switching(15-81)
src/mqt/qecc/circuit_compilation/compilation_utils.py (1)
src/mqt/qecc/circuit_synthesis/circuits.py (1)
num_qubits(250-257)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py (1)
src/mqt/qecc/circuit_synthesis/circuits.py (2)
depth(333-344)num_qubits(250-257)
🪛 Ruff (0.14.6)
tests/circuit_compilation/test_code_switching_compilation.py
45-45: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
65-65: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
75-75: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
src/mqt/qecc/circuit_compilation/code_switching_compiler.py
307-307: Boolean-typed positional argument in function definition
(FBT001)
308-308: Boolean-typed positional argument in function definition
(FBT001)
343-343: Boolean-typed positional argument in function definition
(FBT001)
387-387: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
391-391: Unused noqa directive (non-enabled: N803)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
🔇 Additional comments (5)
tests/circuit_compilation/test_compilation_utils.py (1)
1-163: LGTM! Comprehensive test coverage.The test suite provides excellent coverage of the compilation utilities:
naive_switching: validates single-qubit transitions, no-switch scenarios, CNOT synchronization, error handling for unsupported gates, and ID gate handlinginsert_switch_placeholders: verifies depth calculations, per-qubit targeting, boundary conditions, and multiple insertionsAll test assertions are clear and meaningful.
tests/circuit_compilation/test_code_switching_compilation.py (1)
87-166: LGTM! Solid integration test coverage.The integration tests effectively validate:
- Required switches for gate type transitions
- No-switch scenarios when staying in one code
- One-way transversal gate handling
- Code bias effects on switch placement
Test assertions are clear and verify expected behavior.
src/mqt/qecc/circuit_compilation/__init__.py (1)
1-20: LGTM! Clean package initialization.The package init properly exposes the public API:
CompilerConfigandMinimalCodeSwitchingCompilerfrom the code switching compilerinsert_switch_placeholdersandnaive_switchingutilitiesThe
__all__declaration ensures clear public API boundaries.scripts/cs_compiler/simulate_circuit_performance.py (1)
1-110: LGTM! Well-structured simulation script.The script provides a clean CLI interface for performance benchmarking:
- Proper CSV handling with headers and deduplication
- Integration with
MinimalCodeSwitchingCompilerandnaive_switchingAPIs- Clear timing measurements for both approaches
- Appropriate error handling and skip logic
All previously flagged issues have been addressed.
scripts/cs_compiler/generate_random_circuits.py (1)
175-192: LGTM!The CLI argument parsing is clean and the parameter mapping to
generate_circuitsis correct.
| #!/bin/bash | ||
| # Copyright (c) 2023 - 2025 Chair for Design Automation, TUM | ||
| # All rights reserved. | ||
| # | ||
| # SPDX-License-Identifier: MIT | ||
| # | ||
| # Licensed under the MIT License | ||
|
|
||
| # Generate random universal circuits for different system sizes in parallel. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding shell safety options.
Adding set -euo pipefail after the shebang would catch errors early—if any parallel job fails, the script will exit rather than silently continue.
#!/bin/bash
+set -euo pipefail
# Copyright (c) 2023 - 2025 Chair for Design Automation, TUM📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/bin/bash | |
| # Copyright (c) 2023 - 2025 Chair for Design Automation, TUM | |
| # All rights reserved. | |
| # | |
| # SPDX-License-Identifier: MIT | |
| # | |
| # Licensed under the MIT License | |
| # Generate random universal circuits for different system sizes in parallel. | |
| #!/bin/bash | |
| set -euo pipefail | |
| # Copyright (c) 2023 - 2025 Chair for Design Automation, TUM | |
| # All rights reserved. | |
| # | |
| # SPDX-License-Identifier: MIT | |
| # | |
| # Licensed under the MIT License | |
| # Generate random universal circuits for different system sizes in parallel. |
🤖 Prompt for AI Agents
In scripts/cs_compiler/run_generate_circuits.sh around lines 1 to 10, the script
currently lacks shell safety flags; add `set -euo pipefail` immediately after
the shebang to make the script exit on errors, undefined variables, and failing
pipeline commands so any failing `parallel` job causes the whole script to fail;
optionally also set a safe IFS (e.g. IFS=$'\n\t') if handling filenames/word
splitting is needed.
| run_and_generate() { | ||
| local n=$1 | ||
| local distr_type=$2 | ||
| python "${SCRIPT_DIR}/generate_random_circuits.py" --n "$n" --num_circuits "$num_circuits" --distr_type "$distr_type" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use python3 explicitly for portability.
On some systems, python may not exist or may point to Python 2. Using python3 ensures the correct interpreter is invoked.
- python "${SCRIPT_DIR}/generate_random_circuits.py" --n "$n" --num_circuits "$num_circuits" --distr_type "$distr_type"
+ python3 "${SCRIPT_DIR}/generate_random_circuits.py" --n "$n" --num_circuits "$num_circuits" --distr_type "$distr_type"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python "${SCRIPT_DIR}/generate_random_circuits.py" --n "$n" --num_circuits "$num_circuits" --distr_type "$distr_type" | |
| python3 "${SCRIPT_DIR}/generate_random_circuits.py" --n "$n" --num_circuits "$num_circuits" --distr_type "$distr_type" |
🤖 Prompt for AI Agents
In scripts/cs_compiler/run_generate_circuits.sh around line 26, the script
invokes the generator with "python" which can be Python 2 or missing on some
systems; update the command to call "python3" explicitly so the correct
interpreter is used (replace the python invocation with python3 in that line,
keeping the same arguments and quoting).
| # --- Large Graph Scaling (Safety Check) --- | ||
| # In a huge circuit, the bonus should become very small to avoid distorting the graph, | ||
| # but it must remain non-zero to act as a tie-breaker. | ||
| huge_N = 1_000_000 # noqa: N806 |
There was a problem hiding this comment.
Remove unused noqa directive.
Ruff flags # noqa: N806 as unused (RUF100) because the N806 rule is not enabled in the project configuration.
Apply this diff:
- huge_N = 1_000_000 # noqa: N806
+ huge_N = 1_000_000Based on static analysis hints.
🧰 Tools
🪛 Ruff (0.14.6)
75-75: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In tests/circuit_compilation/test_code_switching_compilation.py around line 75,
the assignment "huge_N = 1_000_000 # noqa: N806" contains an unused `# noqa:
N806` directive; remove the `# noqa: N806` suffix so the line reads simply
"huge_N = 1_000_000" (no other changes), then run the linter to confirm the
RUF100 warning is resolved.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (12)
.gitignore (1)
177-178: Use local git configuration for personal files instead of modifying project.gitignore.
pytest.inishould be ignored via your local.git/info/excludeor global~/.gitignorerather than the project's.gitignore. See GitHub docs on ignoring files.scripts/cs_compiler/generate_random_circuits.py (2)
167-168: Progress message conflates processed and generated counts.When resuming a partial run with existing files,
seed + 1includes skipped files, making "Generated X" misleading. Consider clarifying the message.if (seed + 1) % 50 == 0 or seed == 0: - print(f" → Generated {seed + 1}/{num_circuits}") + print(f" → Processed {seed + 1}/{num_circuits} (new: {generated})")
57-59: Remove unusedlast_gatevariable.
last_gateis assigned at lines 58, 103, and 116 but never read. Onlylast_non_id_gateis used for the back-to-back gate restriction logic.- # Track last gate and last non-id single-qubit gate - last_gate = ["id"] * num_qubits + # Track last non-id single-qubit gate to avoid back-to-back repeats last_non_id_gate = ["id"] * num_qubitsAlso remove the assignments at lines 103 and 116:
available_qubits.discard(q) available_qubits.discard(target) - last_gate[q] = last_gate[target] = "cx" # don't update last_non_id_gate (CX isn't a single-qubit gate) continueavailable_qubits.discard(q) - last_gate[q] = gate if gate != "id": last_non_id_gate[q] = gatescripts/cs_compiler/run_generate_circuits.sh (1)
22-26: Consider usingpython3for portability.On some systems,
pythonmay not exist or may point to Python 2. Usingpython3explicitly ensures the correct interpreter.run_and_generate() { local n=$1 local distr_type=$2 - python "${SCRIPT_DIR}/generate_random_circuits.py" --n "$n" --num_circuits "$num_circuits" --distr_type "$distr_type" + python3 "${SCRIPT_DIR}/generate_random_circuits.py" --n "$n" --num_circuits "$num_circuits" --distr_type "$distr_type" }tests/code_switching/test_code_switching_compilation.py (3)
45-45: Remove unusednoqadirective.The
# noqa: N806directive is unused because the N806 rule is not enabled in the project configuration.- N = 100 # noqa: N806 + N = 100Based on static analysis hints.
65-67: Remove unusednoqadirective.The
# noqa: SLF001directive is unused because the SLF001 rule is not enabled.- eff_cap = simple_graph._edge_capacity_with_idle_bonus( # noqa: SLF001 + eff_cap = simple_graph._edge_capacity_with_idle_bonus(Based on static analysis hints.
75-75: Remove unusednoqadirective.The
# noqa: N806directive is unused because the N806 rule is not enabled.- huge_N = 1_000_000 # noqa: N806 + huge_N = 1_000_000Based on static analysis hints.
src/mqt/qecc/code_switching/compilation_utils.py (3)
23-26: Docstring return type mismatch.The docstring says
dict: final code assignment per qubitbut the function returnslist[str | None]. Update the docstring to match the actual return type.Returns: int: total number of code switches - dict: final code assignment per qubit + list: final code assignment per qubit (index = qubit index, value = "A", "B", or None)
105-108: Remove stale parameter documentation.The docstring documents an
expand_placeholderparameter that doesn't exist in the function signature (line 87).placeholder_depth : int, optional Virtual depth (single-qubit layers) the placeholder should represent. - expand_placeholder : bool, optional - If True, expand each placeholder into `placeholder_depth` calls to - `QuantumCircuit.id(qubit)` so that `QuantumCircuit.depth()` increases. - If False, append a single `SwitchGate` marker (informational only). Returns:
119-124: Add qubit index validation to prevent crypticIndexError.Invalid
qidxvalues (negative or >=circuit.num_qubits) will cause anIndexErrorat line 138 when accessingcircuit.qubits[q_index]. Consider validating upfront with a descriptive error message.# Normalize and group requested placeholders by qubit placeholders_by_qubit: dict[int, list[int]] = {} for qidx, layer_idx in switch_positions: + if not 0 <= qidx < circuit.num_qubits: + msg = f"Invalid qubit index {qidx}; circuit has {circuit.num_qubits} qubits" + raise ValueError(msg) if layer_idx < 0: continue placeholders_by_qubit.setdefault(qidx, []).append(layer_idx)Also consider documenting that negative
layer_idxvalues are silently ignored.src/mqt/qecc/code_switching/code_switching_compiler.py (2)
375-389: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100) since N806 is not enabled in this project's configuration.Based on static analysis hints.
- _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")
391-423: Remove unusednoqadirective and fix misleading comment.
- Ruff flags
# noqa: N803as unused (RUF100).- The comment on line 420 says "take the 'earlier' node in time", but
uisn't guaranteed to be the earlier node since edges can be traversed in either direction. Consider using the node with the smaller depth.Based on static analysis hints.
- def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]: # noqa: N803 + def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]: """Return a list of (qubit, depth) pairs where switches should be inserted. ... """ switch_positions = [] seen = set() for u, v, data in self.G.edges(data=True): if data.get("edge_type") == "temporal": key = tuple(sorted((u, v))) if key in seen: continue seen.add(key) if (u in S and v in T) or (v in S and u in T): - # We can take e.g. the 'earlier' node in time as the insertion point - qubit, depth = self.parse_node_id(u) + # Take the earlier node (smaller depth) as the insertion point + q_u, d_u = self.parse_node_id(u) + q_v, d_v = self.parse_node_id(v) + qubit, depth = (q_u, d_u) if d_u < d_v else (q_v, d_v) switch_positions.append((qubit, depth)) return len(switch_positions), switch_positions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.gitignore(1 hunks)pyproject.toml(3 hunks)scripts/cs_compiler/generate_random_circuits.py(1 hunks)scripts/cs_compiler/run_generate_circuits.sh(1 hunks)src/mqt/qecc/code_switching/__init__.py(1 hunks)src/mqt/qecc/code_switching/code_switching_compiler.py(1 hunks)src/mqt/qecc/code_switching/compilation_utils.py(1 hunks)tests/code_switching/__init__.py(1 hunks)tests/code_switching/test_code_switching_compilation.py(1 hunks)tests/code_switching/test_compilation_utils.py(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-04T14:26:25.420Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/conftest.py:11-19
Timestamp: 2025-11-04T14:26:25.420Z
Learning: In the munich-quantum-toolkit/core repository, Qiskit is always available as a dependency during testing, so import guards for qiskit-dependent imports in test files (e.g., test/python/qdmi/qiskit/*.py) are not necessary.
Applied to files:
tests/code_switching/__init__.pypyproject.toml
📚 Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.
Applied to files:
tests/code_switching/test_code_switching_compilation.pysrc/mqt/qecc/code_switching/code_switching_compiler.py
📚 Learning: 2025-11-27T21:26:39.677Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 846
File: python/mqt/qmap/plugins/qiskit/sc/load_calibration.py:34-34
Timestamp: 2025-11-27T21:26:39.677Z
Learning: In the qmap project, the Ruff linter has the "PL" (pylint) rule category enabled, which includes PLC0415 (import-outside-top-level). Therefore, `# noqa: PLC0415` directives on lazy imports are appropriate and necessary, not unused.
Applied to files:
tests/code_switching/test_code_switching_compilation.pysrc/mqt/qecc/code_switching/code_switching_compiler.py
📚 Learning: 2025-11-04T15:22:19.558Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/conftest.py:155-157
Timestamp: 2025-11-04T15:22:19.558Z
Learning: The munich-quantum-toolkit/core repository requires Python 3.10 or later, so Python 3.10+ features (such as `zip(..., strict=...)`, pattern matching, etc.) are acceptable and should not be flagged as compatibility issues.
Applied to files:
pyproject.toml
🧬 Code graph analysis (4)
tests/code_switching/test_compilation_utils.py (2)
src/mqt/qecc/code_switching/compilation_utils.py (2)
insert_switch_placeholders(84-170)naive_switching(15-81)src/mqt/qecc/circuit_synthesis/circuits.py (1)
depth(333-344)
tests/code_switching/test_code_switching_compilation.py (1)
src/mqt/qecc/code_switching/code_switching_compiler.py (6)
CompilerConfig(27-33)parse_node_id(426-433)compute_idle_bonus(186-217)_edge_capacity_with_idle_bonus(219-245)build_from_qiskit(247-297)compute_min_cut(375-389)
src/mqt/qecc/code_switching/__init__.py (2)
src/mqt/qecc/code_switching/code_switching_compiler.py (2)
CompilerConfig(27-33)MinimalCodeSwitchingCompiler(36-433)src/mqt/qecc/code_switching/compilation_utils.py (2)
insert_switch_placeholders(84-170)naive_switching(15-81)
src/mqt/qecc/code_switching/code_switching_compiler.py (1)
src/mqt/qecc/circuit_synthesis/circuits.py (2)
depth(333-344)num_qubits(250-257)
🪛 Ruff (0.14.7)
tests/code_switching/test_code_switching_compilation.py
45-45: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
65-65: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
75-75: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
src/mqt/qecc/code_switching/code_switching_compiler.py
307-307: Boolean-typed positional argument in function definition
(FBT001)
308-308: Boolean-typed positional argument in function definition
(FBT001)
343-343: Boolean-typed positional argument in function definition
(FBT001)
387-387: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
391-391: Unused noqa directive (non-enabled: N803)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
🔇 Additional comments (27)
pyproject.toml (3)
67-68: LGTM! NetworkX dependency properly added.The networkx>=3.4.2 dependency has been correctly added, addressing the previous review concern about the missing runtime dependency for the code-switching compiler.
128-130: Coverage configuration reorganization looks correct.
155-158: Mypy override for networkx correctly added.scripts/cs_compiler/generate_random_circuits.py (1)
174-191: LGTM!Clean CLI implementation with sensible defaults.
scripts/cs_compiler/run_generate_circuits.sh (1)
1-31: LGTM! Well-structured parallelization script.Previous review concerns have been addressed: shell safety options added, array properly quoted, and
distr_typecorrectly passed to the Python script.tests/code_switching/__init__.py (1)
1-8: LGTM!Minimal
__init__.pyfor the test package is appropriate.tests/code_switching/test_code_switching_compilation.py (5)
1-16: LGTM!Imports and module structure are clean. The fixture provides a reusable compiler instance for tests.
31-38: LGTM!Good coverage of both valid parsing and error handling for invalid node IDs.
87-103: LGTM!Test correctly validates that an H→T sequence requires exactly one code switch.
105-118: LGTM!Correctly validates that circuits staying within one code require no switches.
144-166: LGTM!Good integration test validating that
biased_codeconfiguration affects switching positions.src/mqt/qecc/code_switching/__init__.py (1)
1-20: LGTM!Clean public API definition with explicit
__all__. The module properly re-exports the core classes and utilities, providing a well-defined interface for the code-switching package.tests/code_switching/test_compilation_utils.py (3)
1-16: LGTM!Imports are clean and properly reference the code_switching module utilities.
23-84: LGTM!Comprehensive test coverage for
naive_switching: single-qubit transitions, no-switch scenarios, CNOT synchronization, unsupported gates, and identity gate handling.
91-163: LGTM!Well-structured tests for
insert_switch_placeholderscovering depth changes, correct qubit targeting, out-of-bounds handling, and multiple placeholder insertion order.src/mqt/qecc/code_switching/compilation_utils.py (2)
15-81: LGTM!The
naive_switchingimplementation correctly handles:
- Code initialization on first gate per qubit
- Multi-qubit gate synchronization
- Identity gate skipping
- Unsupported gate detection
130-170: LGTM!Implementation correctly:
- Builds output circuit preserving registers
- Tracks inserted placeholders to avoid duplicates
- Handles layer iteration and placeholder insertion ordering
- Appends out-of-bounds placeholders at the end
src/mqt/qecc/code_switching/code_switching_compiler.py (10)
1-24: LGTM!Clean imports with appropriate use of
TYPE_CHECKINGfor runtime-only type hints and module-level regex compilation for efficiency.
26-34: LGTM!Clean configuration dataclass with sensible defaults.
36-87: LGTM!Well-documented class with a clear graph-based model for code-switching analysis. The initialization properly handles optional config and sets up the directed graph with source/sink nodes.
88-91: LGTM!Simple and correct set intersection.
92-111: LGTM!Node creation with consistent naming convention that matches the module-level regex pattern.
113-165: LGTM!Well-structured edge addition methods with a clean abstraction hierarchy. The bidirectional edge support and infinite capacity handling are appropriate for min-cut algorithms.
186-217: LGTM!Well-documented idle bonus calculation with proper edge case handling. The formula creates an appropriately small "micro-bias" as described.
247-297: LGTM!Clean implementation that properly converts a Qiskit circuit to DAG, tracks qubit activity, and delegates gate processing. The layer-by-layer iteration maintains correct depth ordering.
342-373: LGTM!Clear logic for applying code constraints based on gate membership in source/sink gate sets. The handling of one-way gates and multi-qubit constraints is correct.
425-433: LGTM!Clean static method with proper error handling and use of the pre-compiled regex pattern.
| def _edge_capacity_with_idle_bonus( | ||
| self, depths: list[int], total_edges: int, base_capacity: float | None = None | ||
| ) -> float: | ||
| """Compute the effective temporal edge capacity. | ||
|
|
||
| Optionally reduced by an idle bonus if the qubit has been inactive for several layers. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| depths : list[int] | ||
| The ordered list of depth indices for a given qubit's gates. | ||
| base_capacity : float, optional | ||
| The default temporal edge capacity. | ||
|
|
||
| Returns: | ||
| ------- | ||
| float | ||
| The adjusted edge capacity. | ||
| """ | ||
| if base_capacity is None: | ||
| base_capacity = self.config.default_temporal_edge_capacity | ||
| if len(depths) < 2: | ||
| return base_capacity | ||
|
|
||
| prev_depth, curr_depth = depths[-2], depths[-1] | ||
| bonus = self.compute_idle_bonus(prev_depth, curr_depth, total_edges) | ||
| return max(0.0, base_capacity - bonus) |
There was a problem hiding this comment.
Docstring is missing total_edges parameter description.
Apply this diff:
Parameters
----------
depths : list[int]
The ordered list of depth indices for a given qubit's gates.
+ total_edges : int
+ The total count of temporal edges in the circuit, used for normalization.
base_capacity : float, optional
The default temporal edge capacity.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _edge_capacity_with_idle_bonus( | |
| self, depths: list[int], total_edges: int, base_capacity: float | None = None | |
| ) -> float: | |
| """Compute the effective temporal edge capacity. | |
| Optionally reduced by an idle bonus if the qubit has been inactive for several layers. | |
| Parameters | |
| ---------- | |
| depths : list[int] | |
| The ordered list of depth indices for a given qubit's gates. | |
| base_capacity : float, optional | |
| The default temporal edge capacity. | |
| Returns: | |
| ------- | |
| float | |
| The adjusted edge capacity. | |
| """ | |
| if base_capacity is None: | |
| base_capacity = self.config.default_temporal_edge_capacity | |
| if len(depths) < 2: | |
| return base_capacity | |
| prev_depth, curr_depth = depths[-2], depths[-1] | |
| bonus = self.compute_idle_bonus(prev_depth, curr_depth, total_edges) | |
| return max(0.0, base_capacity - bonus) | |
| def _edge_capacity_with_idle_bonus( | |
| self, depths: list[int], total_edges: int, base_capacity: float | None = None | |
| ) -> float: | |
| """Compute the effective temporal edge capacity. | |
| Optionally reduced by an idle bonus if the qubit has been inactive for several layers. | |
| Parameters | |
| ---------- | |
| depths : list[int] | |
| The ordered list of depth indices for a given qubit's gates. | |
| total_edges : int | |
| The total count of temporal edges in the circuit, used for normalization. | |
| base_capacity : float, optional | |
| The default temporal edge capacity. | |
| Returns: | |
| ------- | |
| float | |
| The adjusted edge capacity. | |
| """ | |
| if base_capacity is None: | |
| base_capacity = self.config.default_temporal_edge_capacity | |
| if len(depths) < 2: | |
| return base_capacity | |
| prev_depth, curr_depth = depths[-2], depths[-1] | |
| bonus = self.compute_idle_bonus(prev_depth, curr_depth, total_edges) | |
| return max(0.0, base_capacity - bonus) |
🤖 Prompt for AI Agents
In src/mqt/qecc/code_switching/code_switching_compiler.py around lines 219 to
245, the method _edge_capacity_with_idle_bonus documents parameters but omits
the total_edges parameter; update the docstring to include total_edges (int)
describing it as the total number of temporal edges used for computing the idle
bonus and how it affects the bonus computation, and ensure the parameter section
follows the same style/format as the other parameters and the return description
remains unchanged.
| def _process_gate_operation( | ||
| self, | ||
| node: DAGOpNode, | ||
| depth: int, | ||
| circuit: QuantumCircuit, | ||
| qubit_activity: dict[int, list[int]], | ||
| qubit_last_node: list[str | None], | ||
| one_way_gates: set[str], | ||
| code_bias: bool, | ||
| idle_bonus: bool, | ||
| total_temporal_edges: int, | ||
| ) -> None: | ||
| """Handle node creation, temporal edges, and code constraints for a single gate. | ||
|
|
||
| Parameters. | ||
| ---------- | ||
| node : DAGOpNode | ||
| The gate operation node from the DAG. | ||
| depth : int | ||
| The depth (layer index) of the operation in the circuit. | ||
| """ | ||
| qubits_indices = [circuit.find_bit(q).index for q in node.qargs] | ||
| gate_type = node.name.upper() | ||
|
|
||
| current_step_nodes = [] | ||
|
|
||
| for q_idx in qubits_indices: | ||
| qubit_activity[q_idx].append(depth) | ||
|
|
||
| node_id = self._add_gate_node(gate_type, q_idx, depth) | ||
| current_step_nodes.append((q_idx, node_id)) | ||
|
|
||
| prev_node = qubit_last_node[q_idx] | ||
| if prev_node: | ||
| capacity = self.config.default_temporal_edge_capacity | ||
| if idle_bonus: | ||
| capacity = self._edge_capacity_with_idle_bonus(qubit_activity[q_idx], total_temporal_edges) | ||
| self._add_regular_edge(prev_node, node_id, capacity=capacity) | ||
|
|
||
| qubit_last_node[q_idx] = node_id | ||
|
|
||
| self._apply_code_constraints(gate_type, current_step_nodes, one_way_gates, code_bias) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Incomplete docstring for _process_gate_operation.
The Parameters section only documents node and depth, but the method has 10 parameters. Consider documenting all parameters or, since this is a private helper, removing the incomplete docstring stub.
def _process_gate_operation(
self,
node: DAGOpNode,
depth: int,
circuit: QuantumCircuit,
qubit_activity: dict[int, list[int]],
qubit_last_node: list[str | None],
one_way_gates: set[str],
code_bias: bool,
idle_bonus: bool,
total_temporal_edges: int,
) -> None:
- """Handle node creation, temporal edges, and code constraints for a single gate.
-
- Parameters.
- ----------
- node : DAGOpNode
- The gate operation node from the DAG.
- depth : int
- The depth (layer index) of the operation in the circuit.
- """
+ """Handle node creation, temporal edges, and code constraints for a single gate."""🧰 Tools
🪛 Ruff (0.14.7)
307-307: Boolean-typed positional argument in function definition
(FBT001)
308-308: Boolean-typed positional argument in function definition
(FBT001)
🤖 Prompt for AI Agents
In src/mqt/qecc/code_switching/code_switching_compiler.py around lines 299 to
340, the _process_gate_operation docstring only documents node and depth while
the method accepts many more parameters; update the docstring to either list and
briefly describe all parameters (circuit, qubit_activity, qubit_last_node,
one_way_gates, code_bias, idle_bonus, total_temporal_edges, and return/behavior)
or remove the incomplete docstring stub if you prefer to keep it private; ensure
parameter names, types/expected shapes, and short purpose notes are included and
keep the wording concise.
Definition should state which gates are one-way transversal and also in which direction this one-way-character holds.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (8)
tests/code_switching/test_code_switching_compilation.py (3)
45-45: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100) because the N806 rule is not enabled.- N = 100 # noqa: N806 + N = 100
65-67: Remove unusednoqadirective.Ruff flags
# noqa: SLF001as unused (RUF100) because the SLF001 rule is not enabled.- eff_cap = simple_graph._edge_capacity_with_idle_bonus( # noqa: SLF001 + eff_cap = simple_graph._edge_capacity_with_idle_bonus(
75-75: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100) because the N806 rule is not enabled.- huge_N = 1_000_000 # noqa: N806 + huge_N = 1_000_000src/mqt/qecc/code_switching/code_switching_compiler.py (5)
187-205: Fix incorrect parameter type in docstring.The docstring states
biased_code : floatbut the parameter isstr | None. Additionally, invalidbiased_codevalues are silently ignored.def _add_bias_edges(self, node_id: str, biased_code: str | None = None) -> None: - """Add biased_code bias edges to the terminal nodes slightly preferring one code over the other. + """Add bias edges to terminal nodes, slightly preferring one code over the other. Parameters ---------- - biased_code : float - Capacity of the biased_code edges to be added. + biased_code : str | None, optional + Which code to bias towards ("SRC" or "SNK"). Defaults to config.biased_code. """
239-265: Docstring is missingtotal_edgesparameter description.Parameters ---------- depths : list[int] The ordered list of depth indices for a given qubit's gates. + total_edges : int + Total count of temporal edges in the circuit, used for normalization. base_capacity : float, optional The default temporal edge capacity.
319-339: Incomplete docstring for_process_gate_operation.The Parameters section only documents
nodeanddepth, but the method has 10 parameters. Since this is a private helper, consider either documenting all parameters or removing the incomplete stub.def _process_gate_operation( self, node: DAGOpNode, depth: int, circuit: QuantumCircuit, qubit_activity: dict[int, list[int]], qubit_last_node: list[str | None], one_way_gates: dict[str, tuple[str, str]], code_bias: bool, idle_bonus: bool, total_temporal_edges: int, ) -> None: - """Handle node creation, temporal edges, and code constraints for a single gate. - - Parameters. - ---------- - node : DAGOpNode - The gate operation node from the DAG. - depth : int - The depth (layer index) of the operation in the circuit. - """ + """Handle node creation, temporal edges, and code constraints for a single gate."""
427-427: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100).- _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")
431-431: Remove unusednoqadirective.Ruff flags
# noqa: N803as unused (RUF100).- def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]: # noqa: N803 + def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/CodeSwitching.md(1 hunks)src/mqt/qecc/code_switching/code_switching_compiler.py(1 hunks)tests/code_switching/test_code_switching_compilation.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
📚 Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.
Applied to files:
tests/code_switching/test_code_switching_compilation.pysrc/mqt/qecc/code_switching/code_switching_compiler.py
📚 Learning: 2025-11-27T21:26:39.677Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 846
File: python/mqt/qmap/plugins/qiskit/sc/load_calibration.py:34-34
Timestamp: 2025-11-27T21:26:39.677Z
Learning: In the qmap project, the Ruff linter has the "PL" (pylint) rule category enabled, which includes PLC0415 (import-outside-top-level). Therefore, `# noqa: PLC0415` directives on lazy imports are appropriate and necessary, not unused.
Applied to files:
tests/code_switching/test_code_switching_compilation.pysrc/mqt/qecc/code_switching/code_switching_compiler.py
🪛 LanguageTool
docs/CodeSwitching.md
[grammar] ~106-~106: Please add a punctuation mark at the end of paragraph.
Context: ...de and the target qubit in the 2D color code To account for this, we can can pass a...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~108-~108: Ensure spelling is correct
Context: ...g gates that can be implemented one-way transverally together with their direction. To see h...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~153-~153: Ensure spelling is correct
Context: ...ch that switching operations are placed preferribly on idling qubits while keeping the tota...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.14.7)
tests/code_switching/test_code_switching_compilation.py
45-45: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
65-65: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
75-75: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
src/mqt/qecc/code_switching/code_switching_compiler.py
327-327: Boolean-typed positional argument in function definition
(FBT001)
328-328: Boolean-typed positional argument in function definition
(FBT001)
367-367: Boolean-typed positional argument in function definition
(FBT001)
427-427: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
431-431: Unused noqa directive (non-enabled: N803)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
🔇 Additional comments (8)
tests/code_switching/test_code_switching_compilation.py (3)
41-81: Test logic is sound; float comparisons are appropriately handled.The test correctly uses:
- Exact equality for
0.0comparisons (lines 48, 51) — safe because zero is exactly representable in IEEE 754pytest.approxfor non-zero float comparisons (lines 62, 70)The test coverage for idle bonus logic is thorough, including edge cases (short idle, active bonus, large graph scaling).
87-151: Integration tests provide solid coverage of min-cut behavior.The tests exercise:
- Forced switches (H→T pattern)
- No switches needed (same-code gates)
- One-way transversal gates with direction tuples
- Invariance of one-way transversal behavior across source/sink swaps
The assertions on
num_switchesandswitch_poscorrectly validate the expected min-cut results.
153-175: Code bias test validates configuration-driven behavior.The test demonstrates that
CompilerConfig(biased_code="SNK")correctly shifts switch positions compared to the default source bias. The assertionswitch_pos_sink_bias != switch_pos_source_biasalong with the specific position check ensures the bias mechanism works as intended.docs/CodeSwitching.md (1)
9-34: Documentation is well-structured and explains the min-cut model clearly.The introduction effectively motivates code switching and explains the graph-based formulation. The node/edge semantics are clearly defined.
src/mqt/qecc/code_switching/code_switching_compiler.py (4)
26-34: CompilerConfig dataclass is well-designed.The configuration parameters are sensibly typed with reasonable defaults. The dataclass approach provides clean initialization and immutability.
362-403: Code constraint logic is well-structured.The method correctly handles:
- Source-unique gates (infinite edge to source)
- Sink-unique gates (infinite edge to sink)
- Common gates with multi-qubit handling and optional one-way transversal direction
- Code bias edges for common gates
405-413: Good input validation in_parse_one_way_direction.The static method validates the direction tuple and raises a clear
ValueErrorfor invalid inputs, preventing silent failures.
69-91: Initialization and graph setup look good.The constructor properly initializes the graph with source/sink nodes and computes common gates. The
base_unary_capacitycalculation correctly combines the configuration parameters.
docs/CodeSwitching.md
Outdated
| Under specific conditions, CNOT operations can be implemented transversally even when the control and target qubits | ||
| are encoded in different codes. This property, however, is directional. In the 2D-3D color code scheme, it holds only when the | ||
| control qubit is encoded in the 3D color code and the target qubit in the 2D color code | ||
|
|
||
| To account for this, we can can pass a dictionary specifying gates that can be implemented one-way transverally together with their direction. |
There was a problem hiding this comment.
Fix typos and missing punctuation.
Two issues flagged by static analysis:
- Line 106: Missing period at end of paragraph
- Line 108: "transverally" → "transversally"
Under specific conditions, CNOT operations can be implemented transversally even when the control and target qubits
are encoded in different codes. This property, however, is directional. In the 2D-3D color code scheme, it holds only when the
-control qubit is encoded in the 3D color code and the target qubit in the 2D color code
+control qubit is encoded in the 3D color code and the target qubit in the 2D color code.
-To account for this, we can can pass a dictionary specifying gates that can be implemented one-way transverally together with their direction.
+To account for this, we can pass a dictionary specifying gates that can be implemented one-way transversally together with their direction.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Under specific conditions, CNOT operations can be implemented transversally even when the control and target qubits | |
| are encoded in different codes. This property, however, is directional. In the 2D-3D color code scheme, it holds only when the | |
| control qubit is encoded in the 3D color code and the target qubit in the 2D color code | |
| To account for this, we can can pass a dictionary specifying gates that can be implemented one-way transverally together with their direction. | |
| Under specific conditions, CNOT operations can be implemented transversally even when the control and target qubits | |
| are encoded in different codes. This property, however, is directional. In the 2D-3D color code scheme, it holds only when the | |
| control qubit is encoded in the 3D color code and the target qubit in the 2D color code. | |
| To account for this, we can pass a dictionary specifying gates that can be implemented one-way transversally together with their direction. |
🧰 Tools
🪛 LanguageTool
[grammar] ~106-~106: Please add a punctuation mark at the end of paragraph.
Context: ...de and the target qubit in the 2D color code To account for this, we can can pass a...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~108-~108: Ensure spelling is correct
Context: ...g gates that can be implemented one-way transverally together with their direction. To see h...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In docs/CodeSwitching.md around lines 104 to 108, fix two copy issues: add a
period at the end of the paragraph on line 106 and correct the misspelling
"transverally" to "transversally" on line 108; update the sentences accordingly
so punctuation and spelling are correct.
| Finding the minimum number of switches is a good starting point, but in practice, we might want to consider additional factors such as: | ||
|
|
||
| - **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferribly on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations. | ||
| - **Code Bias:** If one of the codes has a significantly higher overhead for switching operations, we might want to minimize switches into that code specifically. |
There was a problem hiding this comment.
Fix typo: "preferribly" → "preferably".
-- **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferribly on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations.
+- **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferably on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Finding the minimum number of switches is a good starting point, but in practice, we might want to consider additional factors such as: | |
| - **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferribly on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations. | |
| - **Code Bias:** If one of the codes has a significantly higher overhead for switching operations, we might want to minimize switches into that code specifically. | |
| Finding the minimum number of switches is a good starting point, but in practice, we might want to consider additional factors such as: | |
| - **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferably on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations. | |
| - **Code Bias:** If one of the codes has a significantly higher overhead for switching operations, we might want to minimize switches into that code specifically. |
🧰 Tools
🪛 LanguageTool
[grammar] ~153-~153: Ensure spelling is correct
Context: ...ch that switching operations are placed preferribly on idling qubits while keeping the tota...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In docs/CodeSwitching.md around lines 151 to 154, there's a typo: "preferribly"
should be corrected to "preferably"; update the word in the sentence "such that
switching operations are placed preferribly on idling qubits" to "preferably" to
fix the spelling.
| one_way_gates : set[str], optional | ||
| A set of multi-qubit gate names (e.g. {"CX"}) that are in `common_gates` | ||
| but allow for one-way transversality (mixed codes). | ||
| If a common multi-qubit gate is NOT in this set, it is assumed | ||
| both qubits must be in the same code (bidirectional infinite edges). | ||
| """ |
There was a problem hiding this comment.
Fix docstring type for one_way_gates parameter.
The docstring describes one_way_gates as set[str] but the actual type is dict[str, tuple[str, str]].
- one_way_gates : set[str], optional
- A set of multi-qubit gate names (e.g. {"CX"}) that are in `common_gates`
- but allow for one-way transversality (mixed codes).
- If a common multi-qubit gate is NOT in this set, it is assumed
- both qubits must be in the same code (bidirectional infinite edges).
+ one_way_gates : dict[str, tuple[str, str]], optional
+ A dict mapping multi-qubit gate names (e.g., "CX") to direction tuples
+ (e.g., ("SNK", "SRC")) specifying which code the control and target
+ qubits should be in for one-way transversal execution.
+ If a common multi-qubit gate is NOT in this dict, it is assumed
+ both qubits must be in the same code (bidirectional infinite edges).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| one_way_gates : set[str], optional | |
| A set of multi-qubit gate names (e.g. {"CX"}) that are in `common_gates` | |
| but allow for one-way transversality (mixed codes). | |
| If a common multi-qubit gate is NOT in this set, it is assumed | |
| both qubits must be in the same code (bidirectional infinite edges). | |
| """ | |
| one_way_gates : dict[str, tuple[str, str]], optional | |
| A dict mapping multi-qubit gate names (e.g., "CX") to direction tuples | |
| (e.g., ("SNK", "SRC")) specifying which code the control and target | |
| qubits should be in for one-way transversal execution. | |
| If a common multi-qubit gate is NOT in this dict, it is assumed | |
| both qubits must be in the same code (bidirectional infinite edges). | |
| """ |
🤖 Prompt for AI Agents
In src/mqt/qecc/code_switching/code_switching_compiler.py around lines 286 to
291, the docstring incorrectly types `one_way_gates` as `set[str]`; change it to
`dict[str, tuple[str, str]]` and update the description to state that it maps a
multi-qubit gate name (key) to a pair/tuple of code roles (value) indicating the
allowed one-way transversality direction (e.g. {"CX": ("control_code",
"target_code")}), and mark it as optional; ensure the wording matches the actual
parameter use and adjust any examples in the docstring accordingly.
| switch_positions = [] | ||
| seen = set() | ||
| for u, v, data in self.G.edges(data=True): | ||
| if data.get("edge_type") == "temporal": | ||
| key = tuple(sorted((u, v))) | ||
| if key in seen: | ||
| continue | ||
| seen.add(key) | ||
| if (u in S and v in T) or (v in S and u in T): | ||
| # We can take e.g. the 'earlier' node in time as the insertion point | ||
| qubit, depth = self.parse_node_id(u) | ||
| switch_positions.append((qubit, depth)) | ||
| return len(switch_positions), switch_positions |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && find . -name "*.py" -path "*/test*" | grep -i "code_switch" | head -20Repository: munich-quantum-toolkit/qecc
Length of output: 126
🏁 Script executed:
cd /repo && rg -n "parse_node_id" src/mqt/qecc/code_switching/ -A5Repository: munich-quantum-toolkit/qecc
Length of output: 126
🏁 Script executed:
cd /repo && rg -n "_extract_switch_locations" src/mqt/qecc/code_switching/ -B5 -A20Repository: munich-quantum-toolkit/qecc
Length of output: 126
🏁 Script executed:
cd /repo && rg -n "temporal" src/mqt/qecc/code_switching/code_switching_compiler.py -B2 -A2Repository: munich-quantum-toolkit/qecc
Length of output: 126
🏁 Script executed:
find . -name "*.py" -path "*/test*" | grep -i "code_switch" | head -20Repository: munich-quantum-toolkit/qecc
Length of output: 212
🏁 Script executed:
rg -n "parse_node_id" src/mqt/qecc/code_switching/ -A5Repository: munich-quantum-toolkit/qecc
Length of output: 1184
🏁 Script executed:
rg -n "_extract_switch_locations" src/mqt/qecc/code_switching/code_switching_compiler.py -B5 -A25Repository: munich-quantum-toolkit/qecc
Length of output: 1953
🏁 Script executed:
rg -n "temporal" src/mqt/qecc/code_switching/code_switching_compiler.py -B3 -A3Repository: munich-quantum-toolkit/qecc
Length of output: 6494
🏁 Script executed:
rg -n "switch_positions" tests/code_switching/test_code_switching_compilation.py -B5 -A5Repository: munich-quantum-toolkit/qecc
Length of output: 53
🏁 Script executed:
rg -n "_add_regular_edge" src/mqt/qecc/code_switching/code_switching_compiler.py -A10Repository: munich-quantum-toolkit/qecc
Length of output: 936
🏁 Script executed:
cat -n src/mqt/qecc/code_switching/code_switching_compiler.py | sed -n '350,360p'Repository: munich-quantum-toolkit/qecc
Length of output: 653
🏁 Script executed:
find tests/code_switching -name "*.py" -type f -exec cat {} \; | head -300Repository: munich-quantum-toolkit/qecc
Length of output: 9379
🏁 Script executed:
rg -n "switch" tests/code_switching/test_code_switching_compilation.py -i -B3 -A3Repository: munich-quantum-toolkit/qecc
Length of output: 4204
🏁 Script executed:
rg -n "bidirectional" src/mqt/qecc/code_switching/code_switching_compiler.py -B3 -A3Repository: munich-quantum-toolkit/qecc
Length of output: 3231
🏁 Script executed:
cat -n src/mqt/qecc/code_switching/code_switching_compiler.py | sed -n '451,465p'Repository: munich-quantum-toolkit/qecc
Length of output: 807
🏁 Script executed:
python3 << 'EOF'
# Simulate the edge iteration and seen set logic
G_edges = [
("H_q0_d0", "T_q0_d1"), # Earlier -> Later (forward)
("T_q0_d1", "H_q0_d0"), # Later -> Earlier (reverse, bidirectional)
]
seen = set()
positions = []
for u, v, data in [(u, v, {"edge_type": "temporal"}) for u, v in G_edges]:
if data.get("edge_type") == "temporal":
key = tuple(sorted((u, v)))
print(f"Processing edge ({u}, {v}), key: {key}")
if key in seen:
print(f" -> Already seen, skipping")
continue
seen.add(key)
# Simulate S and T sets: assume H is in S, T is in T
S = {"H_q0_d0"}
T = {"T_q0_d1"}
if (u in S and v in T) or (v in S and u in T):
print(f" -> Crosses cut! Taking u='{u}'")
# The code takes u
positions.append(u)
print(f"\nFinal positions: {positions}")
print(f"Issue: If we get (T_q0_d1, H_q0_d0) first, we take u=T_q0_d1 (the LATER node)")
EOFRepository: munich-quantum-toolkit/qecc
Length of output: 157
🏁 Script executed:
cat -n src/mqt/qecc/code_switching/code_switching_compiler.py | sed -n '465,475p'Repository: munich-quantum-toolkit/qecc
Length of output: 501
🏁 Script executed:
rg -n "def edges" --type py -A 3 | head -20Repository: munich-quantum-toolkit/qecc
Length of output: 53
🏁 Script executed:
python3 << 'EOF'
# Check if the logic with seen set really handles both directions correctly
import networkx as nx
G = nx.DiGraph()
# Simulate temporal edges: earlier -> later
G.add_edge("H_q0_d0", "T_q0_d1", edge_type="temporal")
G.add_edge("T_q0_d1", "H_q0_d0", edge_type="temporal")
S = {"H_q0_d0"}
T = {"T_q0_d1"}
print("Testing edge iteration and seen set logic:")
seen = set()
for u, v, data in G.edges(data=True):
if data.get("edge_type") == "temporal":
key = tuple(sorted((u, v)))
print(f" Iteration: u={u}, v={v}, key={key}, already_seen={key in seen}")
if key in seen:
print(f" -> Skipping (already processed)")
continue
seen.add(key)
if (u in S and v in T) or (v in S and u in T):
print(f" -> Crosses cut. Taking u='{u}' at position:")
# Code always takes u
print(f" ISSUE: If u=T_q0_d1, we get later node, not earlier!")
print(f"\nConclusion: Which edge we process first determines the result.")
print(f"If (T_q0_d1, H_q0_d0) comes first, we take T_q0_d1 (the LATER node).")
print(f"If (H_q0_d0, T_q0_d1) comes first, we take H_q0_d0 (the EARLIER node).")
EOFRepository: munich-quantum-toolkit/qecc
Length of output: 157
🏁 Script executed:
cat -n src/mqt/qecc/code_switching/code_switching_compiler.py | sed -n '466,475p'Repository: munich-quantum-toolkit/qecc
Length of output: 476
🏁 Script executed:
rg -n "pattern\s*=" src/mqt/qecc/code_switching/code_switching_compiler.py -B2 -A2Repository: munich-quantum-toolkit/qecc
Length of output: 174
🏁 Script executed:
cat -n src/mqt/qecc/code_switching/code_switching_compiler.py | head -50Repository: munich-quantum-toolkit/qecc
Length of output: 2138
🏁 Script executed:
python3 << 'PYEOF'
# Verify if the logical issue actually manifests
# Case: When we have prev_node="H_q0_d0" and node_id="T_q0_d1"
# Edges added: H_q0_d0 -> T_q0_d1 AND T_q0_d1 -> H_q0_d0
# Simulating the iteration and seen logic:
edges_bidirectional = [
("T_q0_d1", "H_q0_d0"), # Reverse edge (hypothetically encountered first)
("H_q0_d0", "T_q0_d1"), # Forward edge
]
S = {"H_q0_d0"} # Earlier node in source
T = {"T_q0_d1"} # Later node in sink
seen = set()
taken_node = None
for u, v in edges_bidirectional:
key = tuple(sorted((u, v)))
if key in seen:
print(f"Skip ({u}, {v}) - already in seen")
continue
seen.add(key)
# Check crossing condition
if (u in S and v in T) or (v in S and u in T):
print(f"Process ({u}, {v}) - crosses cut, taking u={u}")
taken_node = u
break
print(f"\nResult: Took node '{taken_node}'")
if taken_node == "T_q0_d1":
print("ERROR: Took the LATER node instead of EARLIER node!")
else:
print("OK: Took the EARLIER node")
PYEOFRepository: munich-quantum-toolkit/qecc
Length of output: 210
Always extract the earlier node when a temporal edge crosses the cut.
The code unconditionally takes node u (line 461), but when (v in S and u in T), u is the later node in time. This violates the documented intent to "take the earlier node in time as the insertion point" (line 460). Since bidirectional edges exist and iteration order is not guaranteed, the code can select the wrong node.
Extract depths from both endpoints and use the one with lower depth:
if (u in S and v in T) or (v in S and u in T):
- # We can take e.g. the 'earlier' node in time as the insertion point
- qubit, depth = self.parse_node_id(u)
- switch_positions.append((qubit, depth))
+ # Take the earlier node (lower depth) as the insertion point
+ q_u, d_u = self.parse_node_id(u)
+ q_v, d_v = self.parse_node_id(v)
+ if d_u <= d_v:
+ switch_positions.append((q_u, d_u))
+ else:
+ switch_positions.append((q_v, d_v))🤖 Prompt for AI Agents
In src/mqt/qecc/code_switching/code_switching_compiler.py around lines 451 to
463, the code always uses node `u` as the insertion point for temporal edges
crossing the cut which can pick the later-in-time node; instead parse both
endpoints' node ids (use self.parse_node_id on u and v), compare their depths
and append the node (qubit, depth) corresponding to the smaller depth (the
earlier-in-time node) to switch_positions; keep the existing seen dedup logic
and handle ties deterministically (e.g., choose the smaller node id) if depths
are equal.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
tests/code_switching/test_code_switching_compilation.py (3)
45-45: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100) because the N806 rule is not enabled.- N = 100 # noqa: N806 + N = 100
65-67: Remove unusednoqadirective.Ruff flags
# noqa: SLF001as unused (RUF100) because the SLF001 rule is not enabled.- eff_cap = simple_graph._edge_capacity_with_idle_bonus( # noqa: SLF001 + eff_cap = simple_graph._edge_capacity_with_idle_bonus( depths=[0, 10], total_edges=N, base_capacity=base_cap )
75-75: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100) because the N806 rule is not enabled.- huge_N = 1_000_000 # noqa: N806 + huge_N = 1_000_000src/mqt/qecc/code_switching/code_switching_compiler.py (5)
427-427: Remove unusednoqadirective.Ruff flags
# noqa: N806as unused (RUF100).- _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity") # noqa: N806 + _, (S, T) = nx.minimum_cut(self.G, self.source, self.sink, capacity="capacity")
431-431: Remove unusednoqadirective.Ruff flags
# noqa: N803as unused (RUF100).- def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]: # noqa: N803 + def _extract_switch_locations(self, S: set[str], T: set[str]) -> tuple[int, list[tuple[int, int]]]:
239-265: Docstring missingtotal_edgesparameter.The
total_edgesparameter is not documented in the docstring.Parameters ---------- depths : list[int] The ordered list of depth indices for a given qubit's gates. + total_edges : int + The total count of temporal edges in the circuit, used for normalization. base_capacity : float, optional The default temporal edge capacity.
319-339: Incomplete docstring for private helper.The docstring only documents
nodeanddepth, but the method has 10 parameters. Since this is a private helper, consider either completing the documentation or simplifying to a one-line summary.def _process_gate_operation( ... ) -> None: - """Handle node creation, temporal edges, and code constraints for a single gate. - - Parameters. - ---------- - node : DAGOpNode - The gate operation node from the DAG. - depth : int - The depth (layer index) of the operation in the circuit. - """ + """Handle node creation, temporal edges, and code constraints for a single gate."""
187-205: Docstring has incorrect parameter type.The docstring states
biased_code : floatbut the parameter isstr | None. Additionally, invalid values are silently ignored.def _add_bias_edges(self, node_id: str, biased_code: str | None = None) -> None: - """Add biased_code bias edges to the terminal nodes slightly preferring one code over the other. + """Add bias edges to the terminal nodes slightly preferring one code over the other. Parameters ---------- - biased_code : float - Capacity of the biased_code edges to be added. + biased_code : str, optional + Which code to bias towards ("SRC" or "SNK"). Defaults to config.biased_code. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/CodeSwitching.md(1 hunks)src/mqt/qecc/code_switching/code_switching_compiler.py(1 hunks)tests/code_switching/test_code_switching_compilation.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.
Applied to files:
tests/code_switching/test_code_switching_compilation.pysrc/mqt/qecc/code_switching/code_switching_compiler.py
📚 Learning: 2025-11-27T21:26:39.677Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 846
File: python/mqt/qmap/plugins/qiskit/sc/load_calibration.py:34-34
Timestamp: 2025-11-27T21:26:39.677Z
Learning: In the qmap project, the Ruff linter has the "PL" (pylint) rule category enabled, which includes PLC0415 (import-outside-top-level). Therefore, `# noqa: PLC0415` directives on lazy imports are appropriate and necessary, not unused.
Applied to files:
tests/code_switching/test_code_switching_compilation.pysrc/mqt/qecc/code_switching/code_switching_compiler.py
🧬 Code graph analysis (2)
tests/code_switching/test_code_switching_compilation.py (1)
src/mqt/qecc/code_switching/code_switching_compiler.py (6)
CompilerConfig(27-33)parse_node_id(466-473)compute_idle_bonus(206-237)_edge_capacity_with_idle_bonus(239-265)build_from_qiskit(267-317)compute_min_cut(415-429)
src/mqt/qecc/code_switching/code_switching_compiler.py (1)
src/mqt/qecc/circuit_synthesis/circuits.py (2)
depth(333-344)num_qubits(250-257)
🪛 LanguageTool
docs/CodeSwitching.md
[grammar] ~106-~106: Please add a punctuation mark at the end of paragraph.
Context: ...de and the target qubit in the 2D color code To account for this, we can can pass a...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~108-~108: Ensure spelling is correct
Context: ...g gates that can be implemented one-way transverally together with their direction. To see h...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~153-~153: Ensure spelling is correct
Context: ...ch that switching operations are placed preferribly on idling qubits while keeping the tota...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.14.7)
tests/code_switching/test_code_switching_compilation.py
45-45: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
65-65: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
75-75: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
src/mqt/qecc/code_switching/code_switching_compiler.py
327-327: Boolean-typed positional argument in function definition
(FBT001)
328-328: Boolean-typed positional argument in function definition
(FBT001)
367-367: Boolean-typed positional argument in function definition
(FBT001)
427-427: Unused noqa directive (non-enabled: N806)
Remove unused noqa directive
(RUF100)
431-431: Unused noqa directive (non-enabled: N803)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
🔇 Additional comments (10)
tests/code_switching/test_code_switching_compilation.py (7)
1-16: Imports and license header look correct.The imports properly reference the public API surface (
MinimalCodeSwitchingCompilerfrommqt.qecc.code_switching) andCompilerConfigfrom the internal module.
19-24: Fixture is well-designed.The
simple_graphfixture provides a reusable compiler instance with gate sets matching the documented 2D/3D color code example.
31-38: LGTM!Good coverage testing both valid parsing and the error case for invalid node IDs.
87-103: Good integration test for mandatory switching.Tests the fundamental case where H (source-only) followed by T (sink-only) requires exactly one switch.
105-118: LGTM!Correctly verifies that circuits staying within a single code (H, CX) require no switches.
121-150: Comprehensive one-way transversal testing.Good coverage of:
- Default behavior requiring a switch
- One-way gate enabling zero switches
- Invariance when swapping source/sink definitions
153-175: Code bias test is well-structured.Verifies that changing
biased_codefrom "SRC" to "SNK" shifts the switch position as expected.src/mqt/qecc/code_switching/code_switching_compiler.py (3)
1-23: Imports and module setup look good.Clean separation of runtime imports and TYPE_CHECKING-only imports. The regex pattern is appropriately compiled at module level for performance.
69-86: Constructor is well-structured.Proper initialization of the graph with source/sink nodes and computation of base capacity from configuration.
36-67: **Class docstring and structure are well-designed.**The comprehensive docstring explains the graph model well. "For each edge (u, v) in R, R[u][v]['capacity'] is equal to the capacity of (u, v) in G if it exists in G or zero otherwise." The implementation correctly ensures non-negative capacities viamax(0.0, ...)in_edge_capacity_with_idle_bonus(line 265), which aligns with NetworkX's expectations.
docs/CodeSwitching.md
Outdated
|
|
||
| Finding the minimum number of switches is a good starting point, but in practice, we might want to consider additional factors such as: | ||
|
|
||
| - **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferribly on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations. |
There was a problem hiding this comment.
Fix spelling: "preferribly" → "preferably".
-- **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferribly on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations.
+- **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferably on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferribly on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations. | |
| - **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferably on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations. |
🧰 Tools
🪛 LanguageTool
[grammar] ~153-~153: Ensure spelling is correct
Context: ...ch that switching operations are placed preferribly on idling qubits while keeping the tota...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In docs/CodeSwitching.md around line 153, fix the spelling typo in the sentence
by replacing "preferribly" with "preferably" so the text reads "...such that
switching operations are placed preferably on idling qubits..." (ensure only the
word is corrected and surrounding punctuation/spacing unchanged).
| @dataclass | ||
| class CompilerConfig: | ||
| """Holds all configuration parameters for the CodeSwitchGraph.""" | ||
|
|
||
| edge_capacity_ratio: float = 0.001 | ||
| default_temporal_edge_capacity: float = 1.0 | ||
| switching_time: int = 2 | ||
| biased_code: str = "SRC" | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding validation for biased_code.
The biased_code field accepts any string, but only "SRC" or "SNK" are valid. Invalid values are silently ignored in _add_bias_edges.
@dataclass
class CompilerConfig:
"""Holds all configuration parameters for the CodeSwitchGraph."""
edge_capacity_ratio: float = 0.001
default_temporal_edge_capacity: float = 1.0
switching_time: int = 2
biased_code: str = "SRC"
+
+ def __post_init__(self) -> None:
+ """Validate configuration parameters."""
+ if self.biased_code not in ("SRC", "SNK"):
+ msg = f"Invalid biased_code: {self.biased_code}. Must be 'SRC' or 'SNK'."
+ raise ValueError(msg)🤖 Prompt for AI Agents
In src/mqt/qecc/code_switching/code_switching_compiler.py around lines 26 to 34,
the CompilerConfig.dataclass allows any string for biased_code but only "SRC" or
"SNK" are valid; add validation in a __post_init__ (or change the type to a
small Enum/Literal) that checks biased_code is either "SRC" or "SNK" and raises
a clear ValueError if not, so invalid values are caught early and not silently
ignored by _add_bias_edges.
| one_way_gates : set[str], optional | ||
| A set of multi-qubit gate names (e.g. {"CX"}) that are in `common_gates` | ||
| but allow for one-way transversality (mixed codes). | ||
| If a common multi-qubit gate is NOT in this set, it is assumed | ||
| both qubits must be in the same code (bidirectional infinite edges). |
There was a problem hiding this comment.
Docstring type mismatch for one_way_gates.
The docstring says one_way_gates : set[str] but the actual type is dict[str, tuple[str, str]].
- one_way_gates : set[str], optional
- A set of multi-qubit gate names (e.g. {"CX"}) that are in `common_gates`
+ one_way_gates : dict[str, tuple[str, str]], optional
+ A dict mapping multi-qubit gate names (e.g. {"CX": ("SNK", "SRC")}) that are in `common_gates`
but allow for one-way transversality (mixed codes).
If a common multi-qubit gate is NOT in this set, it is assumed
both qubits must be in the same code (bidirectional infinite edges).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| one_way_gates : set[str], optional | |
| A set of multi-qubit gate names (e.g. {"CX"}) that are in `common_gates` | |
| but allow for one-way transversality (mixed codes). | |
| If a common multi-qubit gate is NOT in this set, it is assumed | |
| both qubits must be in the same code (bidirectional infinite edges). | |
| one_way_gates : dict[str, tuple[str, str]], optional | |
| A dict mapping multi-qubit gate names (e.g. {"CX": ("SNK", "SRC")}) that are in `common_gates` | |
| but allow for one-way transversality (mixed codes). | |
| If a common multi-qubit gate is NOT in this set, it is assumed | |
| both qubits must be in the same code (bidirectional infinite edges). |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
docs/CodeSwitching.md (7)
104-108: Fix grammar, punctuation, and spelling issues.Three issues flagged by static analysis:
- Line 106: Missing period at end of paragraph
- Line 108: Duplicated word "can can"
- Line 108: Spelling error "transverally" → "transversally"
Under specific conditions, CNOT operations can be implemented transversally even when the control and target qubits are encoded in different codes. This property, however, is directional. In the 2D-3D color code scheme, it holds only when the -control qubit is encoded in the 3D color code and the target qubit in the 2D color code +control qubit is encoded in the 3D color code and the target qubit in the 2D color code. -To account for this, we can can pass a dictionary specifying gates that can be implemented one-way transverally together with their direction. +To account for this, we can pass a dictionary specifying gates that can be implemented one-way transversally together with their direction.
137-145: Fix undefined variable in loop (second of 6 occurrences).Same issue as lines 125-133: iterate over
switch_posinstead ofpositions.mcsc = MinimalCodeSwitchingCompiler({"H", "CX"}, {"T", "CX"}) mcsc.build_from_qiskit(qc, one_way_gates={"CX": ("SNK", "SRC")}) num_switches, switch_pos, _, _ = mcsc.compute_min_cut() print(f"Total switches required (without one-way CNOT): {num_switches}") print("Switch locations (qubit, depth):") -for pos in positions: +for pos in switch_pos: print(f" - Qubit {pos[0]} after operation depth {pos[1]}")
151-154: Fix spelling error "preferribly" → "preferably".-- **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferribly on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations. +- **Depth Optimization:** Choosing the positions of the switching positions such that switching operations are placed preferably on idling qubits while keeping the total number of switches minimal. This has the potential to reduce the overall circuit depth increase caused by the switching operations.
176-184: Fix undefined variable in loop (third of 6 occurrences).Same issue: iterate over
switch_posinstead ofpositions.mcsc = MinimalCodeSwitchingCompiler({"H", "CX"}, {"T", "CX"}) mcsc.build_from_qiskit(qc, one_way_gates={"CX": ("SNK", "SRC")}) num_switches, switch_pos, _, _ = mcsc.compute_min_cut() print(f"Total switches required (without one-way CNOT): {num_switches}") print("Switch locations (qubit, depth):") -for pos in positions: +for pos in switch_pos: print(f" - Qubit {pos[0]} after operation depth {pos[1]}")
188-196: Fix undefined variable in loop (fourth of 6 occurrences).Same issue: iterate over
switch_posinstead ofpositions.mcsc = MinimalCodeSwitchingCompiler({"H", "CX"}, {"T", "CX"}) mcsc.build_from_qiskit(qc, one_way_gates={"CX": ("SNK", "SRC")}, idle_bonus=True) num_switches, switch_pos, _, _ = mcsc.compute_min_cut() print(f"Total switches required (without one-way CNOT): {num_switches}") print("Switch locations (qubit, depth):") -for pos in positions: +for pos in switch_pos: print(f" - Qubit {pos[0]} after operation depth {pos[1]}")
216-224: Fix undefined variable in loop (fifth of 6 occurrences).Same issue: iterate over
switch_posinstead ofpositions.mcsc = MinimalCodeSwitchingCompiler({"H", "CX"}, {"T", "CX"}) mcsc.build_from_qiskit(qc, one_way_gates={"CX": ("SNK", "SRC")}) num_switches, switch_pos, _, _ = mcsc.compute_min_cut() print(f"Total switches required (without one-way CNOT): {num_switches}") print("Switch locations (qubit, depth):") -for pos in positions: +for pos in switch_pos: print(f" - Qubit {pos[0]} after operation depth {pos[1]}")
228-237: Fix undefined variable in loop (sixth of 6 occurrences).Same issue: iterate over
switch_posinstead ofpositions.config = CompilerConfig(biased_code="SNK") mcsc = MinimalCodeSwitchingCompiler({"H", "CX"}, {"T", "CX"}, config=config) mcsc.build_from_qiskit(qc, one_way_gates={"CX": ("SNK", "SRC")}, code_bias=True) num_switches, switch_pos, _, _ = mcsc.compute_min_cut() print(f"Total switches required (without one-way CNOT): {num_switches}") print("Switch locations (qubit, depth):") -for pos in positions: +for pos in switch_pos: print(f" - Qubit {pos[0]} after operation depth {pos[1]}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/CodeSwitching.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-02T07:37:46.860Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Applied to files:
docs/CodeSwitching.md
🪛 LanguageTool
docs/CodeSwitching.md
[grammar] ~106-~106: Please add a punctuation mark at the end of paragraph.
Context: ...de and the target qubit in the 2D color code To account for this, we can can pass a...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~108-~108: Ensure spelling is correct
Context: ...g gates that can be implemented one-way transverally together with their direction. To see h...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~153-~153: Ensure spelling is correct
Context: ...ch that switching operations are placed preferribly on idling qubits while keeping the tota...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
🔇 Additional comments (1)
docs/CodeSwitching.md (1)
125-133: Fix undefined variable in loop (first of 6 occurrences).The code unpacks the min-cut result into
switch_pos(line 128) but the loop iterates overpositions(undefined variable), which will cause aNameErrorat runtime. This same bug appears 5 more times in this file (lines 143, 182, 194, 222, 235).mcsc = MinimalCodeSwitchingCompiler({"H", "CX"}, {"T", "CX"}) mcsc.build_from_qiskit(qc) num_switches, switch_pos, _, _ = mcsc.compute_min_cut() print(f"Total switches required (without one-way CNOT): {num_switches}") print("Switch locations (qubit, depth):") -for pos in positions: +for pos in switch_pos: print(f" - Qubit {pos[0]} after operation depth {pos[1]}")Likely an incorrect or invalid review comment.
Description
Please include a summary of the change and, if applicable, which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes #(issue)
Checklist: