Refined MDP for RL #449
Refined MDP for RL #449
Conversation
Update action space and feature space Update actions Update action space
Fix: resolve pre-commit issues and add missing annotations Fix: resolve pre-commit issues and add missing annotations Remove example_test.py Remove example_test.py
Fix: resolve pre-commit issues and add missing annotations Fix: resolve pre-commit issues and add missing annotations Fix: resolve pre-commit issues and add missing annotations
Signed-off-by: Shaobo-Zhou <[email protected]>
Fix windows runtime warning problem Fix windows runtime warning issue
Signed-off-by: Patrick Hopf <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/mqt/predictor/rl/predictorenv.py (4)
241-246: Remove unusednoqaand consider decomposing bothunitaryandcliffordindependently.Two minor points:
- Line 246 uses
# noqa: SLF001, but static analysis indicates this rule is not enabled in your configuration, making the directive unnecessary noise.- The
elifat line 243 means if a circuit contains bothunitaryandcliffordgates, onlyunitarywill be decomposed. If both types should be decomposed when present, use two independentifstatements.Apply this diff:
- if self.state.count_ops().get("unitary"): + if self.state.count_ops().get("unitary"): self.state = self.state.decompose(gates_to_decompose="unitary") - elif self.state.count_ops().get("clifford"): + if self.state.count_ops().get("clifford"): self.state = self.state.decompose(gates_to_decompose="clifford") - self.state._layout = self.layout # noqa: SLF001 + self.state._layout = self.layout ```<!-- review_comment_end --> <!-- [scratchpad_start] [task review_file_1/1 segment_4/9] Review calculate_reward updates lines 250-261. [observations] - Line 250: Added optional qc parameter - Line 251: Updated docstring - Line 252: circuit = self.state if qc is None else qc - Lines 254-260: Return statements for each reward type - Line 261: assert_never(circuit) [analysis] - Past comment suggested docstring update and assert_never(self.reward_function) instead of circuit - Marked as "Minor" in past comments - The assert_never on circuit is weird, should be on reward_function [actions] Generate review comment (duplicate of past). [scratchpad_end] --> --- `250-261`: **Update docstring and fix `assert_never` argument.** Two minor points: 1. Line 251: The docstring still says "for either the current state or a quantum circuit (if provided)" which is accurate, but could be more precise about the `qc` parameter's role. 2. Line 261: Using `assert_never(circuit)` is incorrect. The exhaustiveness check should be on the discriminant (`self.reward_function`), not on the circuit variable. Apply this diff: ```diff def calculate_reward(self, qc: QuantumCircuit | None = None) -> float: - """Calculates and returns the reward for either the current state or a quantum circuit (if provided).""" + """Calculate the reward for the current state or an optionally provided circuit. + + Args: + qc: Optional circuit to compute reward for. Defaults to self.state if None. + + Returns: + The computed reward value. + """ circuit = self.state if qc is None else qc if self.reward_function == "expected_fidelity": return expected_fidelity(circuit, self.device) if self.reward_function == "estimated_success_probability": return estimated_success_probability(circuit, self.device) if self.reward_function == "estimated_hellinger_distance": return estimated_hellinger_distance(circuit, self.device, self.hellinger_model) if self.reward_function == "critical_depth": return crit_depth(circuit) - assert_never(circuit) + assert_never(self.reward_function)
426-462: Fix type inconsistency and guardfinal_layoutkey access.Three issues:
Line 427: Type annotation declares
PropertySet | Nonebut initializes to{}. The actual values assigned (lines 446, 453) aredictobjects converted frompm.property_set, notPropertySetinstances.Lines 446, 453: Assignments use
dict(pm.property_set) if hasattr(...) else {}, which returnsdictor empty dict, contradicting the| Noneannotation.Line 460: Accessing
pm_property_set["final_layout"]will raiseKeyErrorif routing passes don't populate this key. The checkpm_property_set is not Noneonly guards againstNone, not missing keys.Apply this diff:
def _apply_qiskit_action(self, action: Action, action_index: int) -> QuantumCircuit: - pm_property_set: PropertySet | None = {} + pm_property_set: dict[str, Any] | None = None if getattr(action, "stochastic", False): # Wrap stochastic action to optimize for the used figure of merit altered_qc, pm_property_set = self.fom_aware_compile( action, self.device, self.state, max_iteration=self.max_iter, ) else: if action.name in ["QiskitO3", "Opt2qBlocks_preserve"] and isinstance(action, DeviceDependentAction): passes = action.transpile_pass( self.device.operation_names, CouplingMap(self.device.build_coupling_map()) if self.layout else None, ) if action.name == "QiskitO3": pm = PassManager([DoWhileController(passes, do_while=action.do_while)]) else: pm = PassManager(passes) altered_qc = pm.run(self.state) - pm_property_set = dict(pm.property_set) if hasattr(pm, "property_set") else {} + pm_property_set = dict(pm.property_set) if hasattr(pm, "property_set") else None else: transpile_pass = ( action.transpile_pass(self.device) if callable(action.transpile_pass) else action.transpile_pass ) pm = PassManager(transpile_pass) altered_qc = pm.run(self.state) - pm_property_set = dict(pm.property_set) if hasattr(pm, "property_set") else {} + pm_property_set = dict(pm.property_set) if hasattr(pm, "property_set") else None if action_index in ( self.actions_layout_indices + self.actions_mapping_indices + self.actions_final_optimization_indices ): altered_qc = self._handle_qiskit_layout_postprocessing(action, pm_property_set, altered_qc) - elif action_index in self.actions_routing_indices and self.layout and pm_property_set is not None: - self.layout.final_layout = pm_property_set["final_layout"] + elif ( + action_index in self.actions_routing_indices + and self.layout is not None + and pm_property_set is not None + and "final_layout" in pm_property_set + ): + self.layout.final_layout = pm_property_set["final_layout"] return altered_qcAlso update the parameter type in
_handle_qiskit_layout_postprocessing(line 467) todict[str, Any] | Nonefor consistency.
499-540: Fragile dict key ordering assumption in TKET placement mapping.Two issues:
Lines 520-522: The dict comprehension assumes
placement.keys()iteration order aligns withqc_tmp.qubits[i]indexing:qiskit_mapping = { qc_tmp.qubits[i]: placement[list(placement.keys())[i]].index[0] for i in range(len(placement)) }This is fragile. If
get_placement_mapchanges iteration order, the layout will be incorrect. According to pytket documentation,get_placement_map()returnsdict[Qubit, Node]where both types have.indexattributes.Line 514: Catching broad
Exceptionis flagged by static analysis. If possible, catch the specific TKET exception raised byget_placement_map.Apply this diff to use explicit iteration:
try: placement = transpile_pass[0].get_placement_map(tket_qc) except Exception as e: - logger.warning("Placement failed (%s): %s. Falling back to original circuit.", action.name, e) + logger.warning( + "Placement failed (%s): %s. Falling back to original circuit.", + action.name, + e, + ) return tk_to_qiskit(tket_qc, replace_implicit_swaps=True) else: qc_tmp = tk_to_qiskit(tket_qc, replace_implicit_swaps=True) - qiskit_mapping = { - qc_tmp.qubits[i]: placement[list(placement.keys())[i]].index[0] for i in range(len(placement)) - } + qiskit_mapping = {} + for tket_qubit, node in placement.items(): + # Map TKET qubit index to corresponding Qiskit qubit + qiskit_q = qc_tmp.qubits[tket_qubit.index[0]] + qiskit_mapping[qiskit_q] = node.index[0] layout = Layout(qiskit_mapping)This removes the ordering assumption and correctly uses the pytket
UnitID.indexattributes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/mqt/predictor/rl/predictorenv.py(12 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-04T06:59:40.314Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.314Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Applied to files:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
src/mqt/predictor/rl/predictorenv.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/predictor/rl/predictorenv.py
🧬 Code graph analysis (1)
src/mqt/predictor/rl/predictorenv.py (1)
src/mqt/predictor/utils.py (1)
get_rl_openqasm_gates(148-185)
🪛 Ruff (0.14.7)
src/mqt/predictor/rl/predictorenv.py
246-246: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
409-409: Do not catch blind exception: Exception
(BLE001)
410-410: Logging statement uses f-string
(G004)
418-418: Logging statement uses f-string
(G004)
514-514: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (1)
src/mqt/predictor/rl/predictorenv.py (1)
592-625: Excellent refactoring of action selection logic.The refactored
determine_valid_actions_for_statemethod provides clear, well-structured logic for determining valid actions based on the circuit's state (native gates, mapping status, and layout presence). The multi-branch approach correctly handles:
- Non-native circuits requiring synthesis or optimization
- Mapped circuits needing only mapping-preserving actions
- Circuits with layouts ready for fine-tuning or termination
- Unmapped circuits needing routing or layout exploration
The comments make the logic easy to follow. Nice work!
| def fom_aware_compile( | ||
| self, action: Action, device: Target, qc: QuantumCircuit, max_iteration: int = 20 | ||
| ) -> tuple[QuantumCircuit, PropertySet | None]: | ||
| """Run a stochastic pass multiple times optimizing for the given figure of merit. | ||
| Args: | ||
| action: The action containing the transpile pass. | ||
| device: The compilation target device. | ||
| qc: The input quantum circuit. | ||
| max_iteration: Number of iterations to run the pass. | ||
| Returns: | ||
| A tuple of the best circuit found and its property set (if available). | ||
| """ | ||
| best_result = None | ||
| best_property_set = None | ||
| best_fom = -1.0 | ||
| best_swap_count = float("inf") # for fallback | ||
|
|
||
| assert callable(action.transpile_pass), "Mapping action should be callable" | ||
| for i in range(max_iteration): | ||
| pm = PassManager(action.transpile_pass(device)) | ||
| try: | ||
| out_circ = pm.run(qc) | ||
| prop_set = dict(pm.property_set) | ||
|
|
||
| try: | ||
| # For fidelity-based metrics, do a cheap "lookahead" synthesis step: | ||
| # routing may have introduced non-native SWAPs, so we translate the | ||
| # circuit into the device's native basis before evaluating the metric. | ||
| # | ||
| # Note: | ||
| # - BasisTranslator *only* performs basis conversion; it does not optimize. | ||
| # - This isolates the effect of mapping (inserted SWAPs) on fidelity | ||
| # without conflating it with further optimizations. | ||
|
|
||
| synth_pass = PassManager([ | ||
| BasisTranslator(StandardEquivalenceLibrary, target_basis=device.operation_names) | ||
| ]) | ||
| synth_circ = synth_pass.run(out_circ.copy()) | ||
| fom = self.calculate_reward(synth_circ) | ||
|
|
||
| if fom > best_fom: | ||
| best_fom = fom | ||
| best_result = out_circ | ||
| best_property_set = prop_set | ||
|
|
||
| except Exception as e: | ||
| logger.warning(f"[Fallback to SWAP counts] Synthesis or fidelity computation failed: {e}") | ||
| swap_count = out_circ.count_ops().get("swap", 0) | ||
| if best_result is None or swap_count < best_swap_count: | ||
| best_swap_count = swap_count | ||
| best_result = out_circ | ||
| best_property_set = prop_set | ||
|
|
||
| except Exception: | ||
| logger.exception(f"[Error] Pass failed at iteration {i + 1}") | ||
| continue | ||
|
|
||
| if best_result is not None: | ||
| return best_result, best_property_set | ||
| logger.error("All attempts failed.") | ||
| return qc, None |
There was a problem hiding this comment.
Critical: fom_aware_compile only optimizes fidelity-based metrics; critical_depth is ignored.
The current implementation at lines 398-407 only handles synthesis + fidelity evaluation in a single code path, with no alternative branch for non-fidelity metrics like critical_depth. This means:
- When
reward_function == "critical_depth", the code never computes or compares FOM values. best_resultremainsNonethroughout all iterations.- The method always falls through to line 424 returning
(qc, None)with "All attempts failed" logged, even though all passes may have succeeded.
This breaks the intended multi-trial optimization for stochastic actions when using critical_depth as the reward function.
Additionally, address the static analysis hints:
- Lines 409, 417: Catching broad
Exceptionis flagged. If feasible, catch specific Qiskit exceptions, or document why blanket catches are required for RL robustness. - Lines 410, 418: Replace f-string logging with parameterized logging to avoid eager string formatting.
Apply this diff:
def fom_aware_compile(
self, action: Action, device: Target, qc: QuantumCircuit, max_iteration: int = 20
) -> tuple[QuantumCircuit, PropertySet | None]:
"""Run a stochastic pass multiple times optimizing for the given figure of merit.
Args:
action: The action containing the transpile pass.
device: The compilation target device.
qc: The input quantum circuit.
max_iteration: Number of iterations to run the pass.
Returns:
A tuple of the best circuit found and its property set (if available).
"""
best_result = None
best_property_set = None
best_fom = -1.0
best_swap_count = float("inf") # for fallback
assert callable(action.transpile_pass), "Mapping action should be callable"
for i in range(max_iteration):
pm = PassManager(action.transpile_pass(device))
try:
out_circ = pm.run(qc)
prop_set = dict(pm.property_set)
try:
- # For fidelity-based metrics, do a cheap "lookahead" synthesis step:
- # routing may have introduced non-native SWAPs, so we translate the
- # circuit into the device's native basis before evaluating the metric.
- #
- # Note:
- # - BasisTranslator *only* performs basis conversion; it does not optimize.
- # - This isolates the effect of mapping (inserted SWAPs) on fidelity
- # without conflating it with further optimizations.
-
+ # Synthesize for lookahead evaluation (routing may insert non-native SWAPs)
synth_pass = PassManager([
BasisTranslator(StandardEquivalenceLibrary, target_basis=device.operation_names)
])
synth_circ = synth_pass.run(out_circ.copy())
fom = self.calculate_reward(synth_circ)
if fom > best_fom:
best_fom = fom
best_result = out_circ
best_property_set = prop_set
except Exception as e:
- logger.warning(f"[Fallback to SWAP counts] Synthesis or fidelity computation failed: {e}")
+ logger.warning(
+ "[Fallback to SWAP counts] Synthesis or fidelity computation failed: %s",
+ e,
+ )
swap_count = out_circ.count_ops().get("swap", 0)
if best_result is None or swap_count < best_swap_count:
best_swap_count = swap_count
best_result = out_circ
best_property_set = prop_set
except Exception:
- logger.exception(f"[Error] Pass failed at iteration {i + 1}")
+ logger.exception("[Error] Pass failed at iteration %d", i + 1)
continue
if best_result is not None:
return best_result, best_property_set
logger.error("All attempts failed.")
return qc, NoneNote: The current design assumes all reward functions should be maximized (higher is better). If you need to support minimization metrics in the future, initialize best_fom = float("-inf") and keep the > comparator universally, ensuring all reward functions return values where higher = better.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.7)
409-409: Do not catch blind exception: Exception
(BLE001)
410-410: Logging statement uses f-string
(G004)
418-418: Logging statement uses f-string
(G004)
src/mqt/predictor/rl/actions.py
Outdated
| optimization_level=1 if os.getenv("GITHUB_ACTIONS") == "true" else 2, | ||
| synthesis_epsilon=1e-1 if os.getenv("GITHUB_ACTIONS") == "true" else 1e-8, | ||
| max_synthesis_size=2 if os.getenv("GITHUB_ACTIONS") == "true" else 3, | ||
| max_synthesis_size=3, |
There was a problem hiding this comment.
flowerthrowerPending
Is there a specific reason for removing this (and not the others above)?
There was a problem hiding this comment.
Not really, I think this is outdated
src/mqt/predictor/rl/predictorenv.py
Outdated
| if not mapped: | ||
| # Non-native + unmapped → synthesis or optimization | ||
| return self.actions_synthesis_indices + self.actions_opt_indices | ||
| # Non-native + mapped → synthesis or mapping-preserving |
There was a problem hiding this comment.
I thought all post-synthesis passes are native-gate prerserving (e.g. by doing re-synthesis after routing) in order to keep the flow pictured in the MDP of your thesis?
There was a problem hiding this comment.
Indeed after mapping the passes are native-gate preserving as well. I think the comments are outdated and I will update them.
src/mqt/predictor/utils.py
Outdated
| ) | ||
|
|
||
|
|
||
| def get_rl_openqasm_gates() -> list[str]: |
There was a problem hiding this comment.
I think we should not duplicate get_openqasm_gates here, but instead move the original function into the shared utils.py folder and only explicitly remove unnecessary gates here. Naming here could then be get_openqasm_gates_without_u.
CHANGELOG.md
Outdated
|
|
||
| ### Changed | ||
|
|
||
| - ✨ Improved the MDP for RL ([#449]) ([**@Shaobo-Zhou**]) |
There was a problem hiding this comment.
Very humble of you to list only these things, but you also added:
Expanded Action Space
Added AIRouting
Added optimization actions KAKDecomposition and ElidePermutations.
Support for Stochastic Passes
Wrapped stochastic actions
Expanded state observation vector
Regarding the versions, |
Signed-off-by: Shaobo-Zhou <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
src/mqt/predictor/rl/actions.py (3)
84-86: Remove the Python 3.13 version gate for AIRouting.A previous review confirmed that qiskit-ibm-transpiler is compatible with Python 3.13+ (published with universal py3 wheels). This version check artificially excludes AIRouting from Python 3.13+ users.
qiskit-ibm-transpiler Python 3.13 compatibility December 2025
140-141: Simplify type annotations forstochasticandpreserve_layout.The type
bool | None = Falseis semantically confusing—ifFalseis a valid default,Nonelikely has no meaningful distinction. Consider simplifying tobool = Falseunless there's a specific use case forNone.- stochastic: bool | None = False - preserve_layout: bool | None = False + stochastic: bool = False + preserve_layout: bool = False
723-734: Consider defensive handling instead of assertions forfinal_layout.The assertions in
SafeAIRouting.run()will crash in production iffinal_layoutis unexpectedlyNoneor malformed. Since this wraps an external pass (AIRouting), consider logging warnings and returning gracefully instead of asserting.final_layout = getattr(self, "property_set", {}).get("final_layout", None) - assert final_layout is not None, "final_layout is None — cannot map virtual qubits" + if final_layout is None: + logger.warning("final_layout is None; returning routed circuit without measurement restoration") + return dag_routed qubit_map = {} for virt in qc_orig.qubits: - assert virt in final_layout, f"Virtual qubit {virt} not found in final layout!" + if virt not in final_layout: + logger.warning("Virtual qubit %s not found in final layout; skipping", virt) + continuesrc/mqt/predictor/rl/predictorenv.py (7)
240-244: Handle bothunitaryandcliffordgates if both are present.The
elifbranch means onlyunitaryis decomposed if both gate types exist in the circuit. If both should be decomposed when present, use two independentifstatements.- if self.state.count_ops().get("unitary"): - self.state = self.state.decompose(gates_to_decompose="unitary") - elif self.state.count_ops().get("clifford"): + if self.state.count_ops().get("unitary"): + self.state = self.state.decompose(gates_to_decompose="unitary") + if self.state.count_ops().get("clifford"): self.state = self.state.decompose(gates_to_decompose="clifford")
246-246: Remove unusednoqadirective.Static analysis indicates
SLF001is not enabled, so the directive is unnecessary.- self.state._layout = self.layout # noqa: SLF001 + self.state._layout = self.layout
259-261: Useassert_neveron the discriminant, not the circuit.For exhaustiveness checking,
assert_never(self.reward_function)is more idiomatic thanassert_never(circuit)since the unreachable branch is determined by the reward function value.if self.reward_function == "critical_depth": return crit_depth(circuit) - assert_never(circuit) + assert_never(self.reward_function)
459-460: Guard against missingfinal_layoutkey.Accessing
pm_property_set["final_layout"]without checking key existence risksKeyErrorif routing passes don't populate it.- elif action_index in self.actions_routing_indices and self.layout and pm_property_set is not None: - self.layout.final_layout = pm_property_set["final_layout"] + elif ( + action_index in self.actions_routing_indices + and self.layout is not None + and pm_property_set is not None + and "final_layout" in pm_property_set + ): + self.layout.final_layout = pm_property_set["final_layout"]
514-516: Improve exception handling for TKET placement.The broad
Exceptioncatch hides potential programming errors. If feasible, catch the specific TKET exception type raised byget_placement_map.
520-522: Dict key ordering assumption is fragile.The comprehension assumes
placement.keys()iteration order matchesqc_tmp.qubits[i]indexing, which is fragile ifget_placement_mapchanges iteration order.Use explicit iteration over
placement.items()with the TKET qubit's index:- qiskit_mapping = { - qc_tmp.qubits[i]: placement[list(placement.keys())[i]].index[0] for i in range(len(placement)) - } + qiskit_mapping = {} + for tket_qubit, node in placement.items(): + qiskit_q = qc_tmp.qubits[tket_qubit.index[0]] + qiskit_mapping[qiskit_q] = node.index[0]This correctly uses the pytket
UnitID.indexattributes and eliminates the ordering assumption.
409-419: Use parameterized logging instead of f-strings.Replace f-string logging with parameterized logging for consistency with Python logging best practices and to satisfy Ruff G004.
except Exception as e: - logger.warning(f"[Fallback to SWAP counts] Synthesis or fidelity computation failed: {e}") + logger.warning("[Fallback to SWAP counts] Synthesis or fidelity computation failed: %s", e) swap_count = out_circ.count_ops().get("swap", 0) if best_result is None or swap_count < best_swap_count: best_swap_count = swap_count best_result = out_circ best_property_set = prop_set except Exception: - logger.exception(f"[Error] Pass failed at iteration {i + 1}") + logger.exception("[Error] Pass failed at iteration %d", i + 1) continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(2 hunks)src/mqt/predictor/ml/helper.py(1 hunks)src/mqt/predictor/reward.py(1 hunks)src/mqt/predictor/rl/actions.py(20 hunks)src/mqt/predictor/rl/helper.py(3 hunks)src/mqt/predictor/rl/predictorenv.py(12 hunks)src/mqt/predictor/utils.py(2 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-10-14T14:37:38.047Z
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.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-01T15:57:31.153Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1283
File: src/qir/runtime/QIR.cpp:196-201
Timestamp: 2025-11-01T15:57:31.153Z
Learning: In the QIR runtime (src/qir/runtime/QIR.cpp), the PRX gate (__quantum__qis__prx__body) is an alias for the R gate (Phased X-Rotation) and should call runtime.apply<qc::R>(theta, phi, qubit), not runtime.apply<qc::RX>() which is a single-parameter rotation gate.
Applied to files:
src/mqt/predictor/utils.py
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/test/Dialect/MQTOpt/Transforms/lift-measurements.mlir:269-288
Timestamp: 2025-10-09T13:20:11.483Z
Learning: In the MQT MLIR dialect, the `rz` gate should not be included in the `DIAGONAL_GATES` set for the `ReplaceBasisStateControlsWithIfPattern` because its operator matrix does not have the required shape | 1 0 | / | 0 x | for the targets-as-controls optimization. It is only included in `LiftMeasurementsAboveGatesPatterns` where the matrix structure requirement differs.
Applied to files:
src/mqt/predictor/utils.py
📚 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:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-04T06:59:40.314Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.314Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Applied to files:
src/mqt/predictor/rl/predictorenv.pysrc/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
src/mqt/predictor/rl/predictorenv.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/predictor/rl/predictorenv.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:
src/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
src/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
src/mqt/predictor/rl/actions.py
🧬 Code graph analysis (3)
src/mqt/predictor/rl/helper.py (2)
src/mqt/predictor/utils.py (2)
calc_supermarq_features(93-145)get_openqasm_gates_without_u(197-233)src/mqt/predictor/ml/helper.py (1)
dict_to_featurevector(53-60)
src/mqt/predictor/ml/helper.py (1)
src/mqt/predictor/utils.py (2)
calc_supermarq_features(93-145)get_openqasm_gates(148-194)
src/mqt/predictor/rl/predictorenv.py (3)
src/mqt/predictor/rl/actions.py (4)
CompilationOrigin(103-109)DeviceDependentAction(150-161)PassType(112-121)Action(125-141)src/mqt/predictor/rl/parsing.py (1)
prepare_noise_data(253-278)src/mqt/predictor/utils.py (1)
get_openqasm_gates_without_u(197-233)
🪛 Ruff (0.14.7)
src/mqt/predictor/rl/predictorenv.py
246-246: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
409-409: Do not catch blind exception: Exception
(BLE001)
410-410: Logging statement uses f-string
(G004)
418-418: Logging statement uses f-string
(G004)
514-514: Do not catch blind exception: Exception
(BLE001)
⏰ 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-14) / 🐍 macos-14
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
🔇 Additional comments (12)
src/mqt/predictor/reward.py (1)
214-214: Verify that this formatting change alone is the "correction" referenced in the PR.The PR summary mentions "corrected estimated success probability calculation in reward.py," but the provided diff shows only a blank line added at line 214 for readability between the single-qubit and multi-qubit branches. No functional logic changes are visible in this file snippet.
Please confirm:
- Is this blank line the only change to the
estimated_success_probabilityfunction, or are there other functional modifications not shown in the diff?- If this is purely a formatting change, the PR summary should clarify that the functional correction was made elsewhere or describe the specific logic that was corrected.
CHANGELOG.md (1)
51-51: Changelog references are properly formatted.The PR link reference and contributor link have been correctly added and formatted in the reference sections.
Also applies to: 70-70
src/mqt/predictor/rl/helper.py (2)
72-81: LGTM!The normalized feature vector implementation is correct. The exclusion of "barrier" from the total and the division-by-zero guard are properly handled. The return type
dict[str, float]correctly reflects the normalized values.
93-99: LGTM!The integration with
dict_to_featurevectorand separate handling of the "measure" feature is correct, since "measure" is not part ofget_openqasm_gates_without_u()but still needs to be tracked as a normalized feature.src/mqt/predictor/utils.py (2)
148-194: LGTM!The centralized
get_openqasm_gates()function properly consolidates the gate list that was previously duplicated across modules. The documentation correctly references the qelib1.inc source.
197-233: LGTM!The RL-specific gate list correctly excludes u-family gates and multi-qubit gates with >2 qubits as documented. The
"r"gate inclusion is valid as it's a parametric rotation gate supported by Qiskit.src/mqt/predictor/ml/helper.py (1)
18-18: LGTM!Clean refactor that centralizes the gate list definition in
utils.pywhile preserving the existing behavior of the ML helper.src/mqt/predictor/rl/actions.py (4)
276-282: LGTM!Good fix:
CollectCliffords()now runs beforeOptimizeCliffords(), which is required for the optimization pass to work correctly as noted in the PR objectives.
326-333: LGTM!The
KAKDecompositionaction withallow_swaps=Falseis correctly configured for the optimization workflow.
441-468: LGTM!The TKET placement actions (
GraphPlacementandNoiseAwarePlacement) are correctly configured with appropriate timeouts and match limits. The use of keyword arguments for noise parameters (node_errors,link_errors,readout_errors) addresses the previous review concern about positional argument ordering.
608-636: LGTM!The helper functions correctly preserve register and qubit identity by reusing original objects instead of cloning, addressing the issues from previous reviews. The type annotations are now correct with
list[Clbit]for the measurements tuple.src/mqt/predictor/rl/predictorenv.py (1)
592-627: LGTM!The refactored
determine_valid_actions_for_statelogic is much clearer with explicit comments explaining each state transition. The use ofactions_structure_preserving_indicescorrectly restricts post-mapping actions to those that preserve the layout and native gates.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Shaobo-Zhou <[email protected]>
Signed-off-by: Shaobo-Zhou <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
src/mqt/predictor/rl/actions.py (2)
137-138: Simplify type annotation forstochasticandpreserve_layout.The
bool | None = Falsetype is confusing—ifFalseis a valid default,Noneis likely never used. This was flagged in a previous review but not yet addressed.- stochastic: bool | None = False - preserve_layout: bool | None = False + stochastic: bool = False + preserve_layout: bool = False
697-733: Consider defensive handling instead of assertions forfinal_layout.The assertions on lines 719, 722, 725, 728 will crash in production if
final_layoutis unexpectedlyNoneor malformed. Since this wraps an external pass (AIRouting), consider logging warnings and falling back gracefully instead of asserting.This was flagged in a previous review and should be addressed for robustness against unexpected AIRouting behavior.
Example approach:
final_layout = getattr(self, "property_set", {}).get("final_layout", None) - assert final_layout is not None, "final_layout is None — cannot map virtual qubits" + if final_layout is None: + logger.warning("final_layout is None; returning routed circuit without measurement restoration") + return dag_routed qubit_map = {} for virt in qc_orig.qubits: - assert virt in final_layout, f"Virtual qubit {virt} not found in final layout!" + if virt not in final_layout: + logger.warning("Virtual qubit %s not found in final layout; skipping", virt) + continuesrc/mqt/predictor/rl/predictorenv.py (4)
240-248: Remove unusednoqadirective.Static analysis indicates
SLF001is not enabled, so thenoqa: SLF001directive is unnecessary.- self.state._layout = self.layout # noqa: SLF001 + self.state._layout = self.layout
250-261: Minor: Update docstring andassert_nevertarget.The docstring now reflects the optional
qcparameter. However,assert_never(circuit)is slightly misleading—it should beassert_never(self.reward_function)since that's the discriminant being exhaustively checked.- assert_never(circuit) + assert_never(self.reward_function)
362-424: Critical:fom_aware_compileassumes all reward functions should be maximized.The implementation uses
if fom > best_fomuniformly, which works for fidelity-based metrics but may not be appropriate ifcritical_depth(where lower is better in some interpretations) is used. The initializationbest_fom = -1.0and the>comparator assume maximization.Per the PR objectives and previous review discussions, all reward functions are now normalized to return higher values for better results (e.g.,
crit_depthreturns1 - supermarq_features.critical_depth). Verify this assumption holds for all current and future reward functions.Additionally, address the static analysis hints:
Lines 409, 417: Broad
except Exceptioncatches are flagged. While appropriate for robustness against arbitrary pass failures, consider documenting the rationale.Lines 410, 418: Replace f-string logging with parameterized logging:
- logger.warning(f"[Fallback to SWAP counts] Synthesis or fidelity computation failed: {e}") + logger.warning("[Fallback to SWAP counts] Synthesis or fidelity computation failed: %s", e) ... - logger.exception(f"[Error] Pass failed at iteration {i + 1}") + logger.exception("[Error] Pass failed at iteration %d", i + 1)
497-540: Dict key ordering assumption is fragile.The current code at lines 520-522 assumes
placement.keys()iteration order matchesqc_tmp.qubits[i]indexing:qiskit_mapping = { qc_tmp.qubits[i]: placement[list(placement.keys())[i]].index[0] for i in range(len(placement)) }This is fragile. Since
get_placement_map()returnsdict[Qubit, Node]with.indexproperties on both types, iterate explicitly:- qiskit_mapping = { - qc_tmp.qubits[i]: placement[list(placement.keys())[i]].index[0] for i in range(len(placement)) - } + qiskit_mapping = {} + for tket_qubit, node in placement.items(): + qiskit_q = qc_tmp.qubits[tket_qubit.index[0]] + qiskit_mapping[qiskit_q] = node.index[0]Also, the broad
except Exceptionat line 514 and f-string logging should be addressed per static analysis hints:- except Exception as e: - logger.warning("Placement failed (%s): %s. Falling back to original circuit.", action.name, e) + except Exception as e: # Broad catch intentional: TKET placement may fail in various ways + logger.warning("Placement failed (%s): %s. Falling back to original circuit.", action.name, e)pyproject.toml (1)
269-272: Critical: Add Python version constraint to networkx override to prevent Python 3.13 installation failures.The
networkx==2.8.5pin will break on Python 3.13 because NetworkX 2.8.5 only supports Python 3.8–3.10. The project declares Python 3.13 support (line 68), so installations on Python 3.13 will fail during dependency resolution. This issue was flagged in previous reviews but remains unaddressed.Apply this diff to add the required Python version constraint:
[tool.uv] override-dependencies = [ - "networkx==2.8.5", # Required by `qiskit-ibm-transpiler` + "networkx==2.8.5; python_version < \"3.13\"", # Required by `qiskit-ibm-transpiler` ]Additionally, consider verifying whether the exact pin
==2.8.5is necessary or if a lower-bound constraint (e.g.,>=2.8.5,<3) would provide better compatibility. Run the following to check:#!/bin/bash # Verify qiskit-ibm-transpiler's actual networkx requirements echo "=== Check qiskit-ibm-transpiler 0.15.0 networkx dependency ===" curl -s https://pypi.org/pypi/qiskit-ibm-transpiler/0.15.0/json | \ jq -r '.info.requires_dist[]? | select(. | contains("networkx"))' echo -e "\n=== Check if there are known compatibility issues ===" rg -nP --type=py 'import networkx|from networkx' -A 2 -B 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
pyproject.toml(6 hunks)src/mqt/predictor/rl/actions.py(18 hunks)src/mqt/predictor/rl/predictorenv.py(12 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
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.
📚 Learning: 2025-12-13T20:08:45.549Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 862
File: pyproject.toml:65-66
Timestamp: 2025-12-13T20:08:45.549Z
Learning: In the qmap project (pyproject.toml), maintain broad compatibility with dependencies across supported Python versions. Avoid artificially raising minimum version requirements unless there's a specific need (e.g., to guarantee binary wheel availability for certain Python versions, or to access required features). The goal is to keep the software as broadly compatible as possible with the rest of the ecosystem.
Applied to files:
pyproject.toml
📚 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.tomlsrc/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
pyproject.tomlsrc/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
pyproject.tomlsrc/mqt/predictor/rl/actions.py
📚 Learning: 2025-11-28T14:33:15.199Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1337
File: pyproject.toml:351-351
Timestamp: 2025-11-28T14:33:15.199Z
Learning: In the munich-quantum-toolkit/core repository, the dev dependency "ty==0.0.1a27" is intentionally pinned to an exact pre-release version. This dependency is used by the `ty-check` pre-commit hook (via `uv run --only-dev ty check`), and the exact pin ensures all developers run the same version, preventing unexpected issues from updates since ty is in alpha and changing rapidly.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-13T00:03:08.078Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:252-256
Timestamp: 2025-10-13T00:03:08.078Z
Learning: In uv's `override-dependencies`, multiple entries for the same package with different environment markers (or one without a marker and one with a specific marker) can coexist. uv correctly resolves these by applying the appropriate constraint based on the active Python version. For example, `["symengine>=0.11,<0.14", "symengine>=0.14.1; python_version >= '3.14'"]` is valid and will apply the second constraint on Python 3.14+.
Applied to files:
pyproject.toml
📚 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:
pyproject.toml
📚 Learning: 2025-12-15T01:59:17.023Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Applied to files:
pyproject.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:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-08T11:32:57.308Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1375
File: python/mqt/core/plugins/qiskit/converters.py:71-78
Timestamp: 2025-12-08T11:32:57.308Z
Learning: In the munich-quantum-toolkit/core repository, helper functions like `_raise_error` are used to satisfy Ruff's TRY301 rule when raising exceptions in nested contexts (e.g., within try/except blocks). This pattern is necessary given the project's strict linting configuration.
Applied to files:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-04T06:59:40.314Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.314Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Applied to files:
src/mqt/predictor/rl/predictorenv.pysrc/mqt/predictor/rl/actions.py
📚 Learning: 2025-12-14T15:23:54.712Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 23
File: docs/conf.py:110-130
Timestamp: 2025-12-14T15:23:54.712Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the Ruff configuration has 'ALL' rules enabled with only specific rules disabled. PLR6301 (no-self-use) is active, so `# noqa: PLR6301` directives are necessary for methods that don't use self, even if Ruff reports RUF100 warnings suggesting the directive is unused.
Applied to files:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
src/mqt/predictor/rl/predictorenv.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/predictor/rl/predictorenv.py
📚 Learning: 2025-11-26T10:29:36.435Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/fomac/test_fomac.py:54-66
Timestamp: 2025-11-26T10:29:36.435Z
Learning: In test/python/fomac/test_fomac.py, the `ddsim_device` fixture returns `Device | None` even though it only returns a `Device` or calls `pytest.skip()`. The `| None` annotation is required for the type checker to pass, as type checkers may not recognize `pytest.skip()` as a function that never returns.
Applied to files:
src/mqt/predictor/rl/predictorenv.py
🧬 Code graph analysis (1)
src/mqt/predictor/rl/predictorenv.py (4)
src/mqt/predictor/rl/actions.py (3)
CompilationOrigin(100-106)PassType(109-118)run(705-733)src/mqt/predictor/rl/parsing.py (5)
final_layout_bqskit_to_qiskit(186-226)final_layout_pytket_to_qiskit(167-183)postprocess_vf2postlayout(229-250)prepare_noise_data(253-278)apply(89-92)src/mqt/predictor/utils.py (1)
get_openqasm_gates_without_u(197-233)src/mqt/predictor/reward.py (2)
expected_fidelity(46-74)estimated_success_probability(102-226)
🪛 Ruff (0.14.8)
src/mqt/predictor/rl/predictorenv.py
246-246: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
409-409: Do not catch blind exception: Exception
(BLE001)
410-410: Logging statement uses f-string
(G004)
418-418: Logging statement uses f-string
(G004)
514-514: Do not catch blind exception: Exception
(BLE001)
⏰ 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-14) / 🐍 macos-14
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
🔇 Additional comments (30)
src/mqt/predictor/rl/actions.py (16)
25-30: LGTM!New imports for TKET passes (
KAKDecomposition,GraphPlacement,NoiseAwarePlacement) and Qiskit routing (SabreSwap) are correctly added.
50-57: LGTM!Circuit/DAG conversion utilities and
CouplingMapimports are properly added to support the new routing and layout actions.
95-96: LGTM!Type hints for
ClbitandDAGCircuitcorrectly added to TYPE_CHECKING block.
150-157: LGTM!
DeviceDependentAction.transpile_passtype correctly updated to includeCallable[..., list[Any]]for flexibility.
199-207: LGTM!New
Optimize1qGatesDecomposition_preserveaction correctly uses device basis gates and is marked withpreserve_layout=True.
239-246: LGTM!
ElidePermutationsaction added for optimization. Note it doesn't havepreserve_layout=True, which seems intentional since it may modify the circuit structure.
273-280: LGTM!
OptimizeCliffordsnow correctly preceded byCollectCliffordsas per PR objectives.
291-303: LGTM!
Opt2qBlocks_preserveaction correctly configured with device-aware basis gates and coupling map.
323-330: LGTM!
KAKDecompositionaction added withallow_swaps=Falseto prevent layout disruption.
438-465: LGTM!
GraphPlacementandNoiseAwarePlacementactions correctly registered.NoiseAwarePlacementnow uses keyword arguments (node_errors=,link_errors=,readout_errors=) as suggested in previous review.
479-506: LGTM!
SabreSwapandAIRoutingactions correctly marked asstochastic=Trueand use the newSafeAIRoutingwrapper.
508-523: LGTM!
AIRouting_optaction correctly combinesSabreLayoutfor initial layout withSafeAIRoutingin "optimize" mode.
525-535: LGTM!
SabreMappingupdated withstochastic=Trueandmax_iterations=1for controlled stochastic behavior.
605-633: LGTM!
extract_cregs_and_measurementshelper correctly reuses original register objects instead of cloning, addressing the previous review concern about bit identity.
636-659: LGTM!
remove_cregscorrectly reuses originalQuantumRegisterobjects to preserve qubit identity.
662-694: LGTM!
add_cregs_and_measurementscorrectly usesif qubit_map is not Noneinstead of truthiness check, as suggested in previous review.src/mqt/predictor/rl/predictorenv.py (7)
27-32: LGTM!New type imports for
Node,PropertySet, andTargetcorrectly added.
46-57: LGTM!New imports for basis translation, layout passes, and mapping checks are properly organized.
68-85: LGTM!Imports from
actions.py,helper.py, andparsing.pyupdated to include new exports (prepare_noise_data,get_openqasm_gates_without_u).
120-142: LGTM!
actions_structure_preserving_indicescorrectly populated from actions withpreserve_layout=True. The attribute check usesgetattr(elem, "preserve_layout", False)as previously suggested.
181-199: LGTM!Observation space extended with normalized gate counts for OpenQASM 2.0 gates. New attributes
max_iter,node_err,edge_err,readout_errinitialized for stochastic compilation and noise-aware placement support.
464-495: LGTM!
_handle_qiskit_layout_postprocessingcorrectly updated to acceptpm_property_setas a dict parameter, with proper null checks and layout construction.
592-627: LGTM!
determine_valid_actions_for_stateis well-structured with clear comments explaining each branch:
- Non-native + unmapped → synthesis or optimization
- Non-native + mapped → synthesis or structure-preserving
- Native + mapped with layout → terminate or fine-tune
- Native + mapped without layout → explore layout/mapping improvements
- Layout chosen but not mapped → routing only
- No layout, not mapped → full exploration
The logic correctly uses
actions_structure_preserving_indicesto prevent invalidating mappings after routing.pyproject.toml (7)
40-40: LGTM: stable-baselines3 dependency added for RL functionality.The addition of stable-baselines3>=2.7.0 aligns with the PR's RL predictor refactoring objectives and supports the expanded action space features.
122-123: LGTM: Filter warnings appropriately suppress third-party library issues.The new filter warnings for
qiskit_ibm_transpilersyntax warnings are reasonable. These suppress known issues in the external library that are outside the project's control.
134-135: LGTM: Windows timeout warning suppression is justified.This filter warning addresses a platform-specific limitation where timeout warnings on Windows Python 3.13 are treated as errors. The corresponding code change in
utils.py(from explicit warn to logger.info) prevents unnecessary test failures while maintaining visibility of the limitation.
174-174: LGTM: mypy ignore entry for qiskit_ibm_transpiler is appropriate.Adding
qiskit_ibm_transpilerto the mypy ignore list is correct since the library lacks type stubs. This is consistent with the project's handling of other untyped dependencies.
36-38: Optional: Enhance TODO comment with full issue URL.The TODO comment references issue #471 but lacks the full URL. Consider updating it to include the complete GitHub issue URL for easier tracking and navigation.
Apply this diff to improve clarity:
- # TODO(denialhaag): Remove once pytket_qiskit is updated - # https://github.com/munich-quantum-toolkit/predictor/issues/471 - "qiskit-ibm-runtime>=0.30.0,<0.42.0", + # TODO(denialhaag): Remove upper bound once issue #471 is resolved + # https://github.com/munich-quantum-toolkit/predictor/issues/471 + "qiskit-ibm-runtime>=0.30.0,<0.42.0", # TODO tracked in #471⛔ Skipped due to learnings
Learnt from: burgholzer Repo: munich-quantum-toolkit/qmap PR: 862 File: pyproject.toml:65-66 Timestamp: 2025-12-13T20:08:45.549Z Learning: In the qmap project (pyproject.toml), maintain broad compatibility with dependencies across supported Python versions. Avoid artificially raising minimum version requirements unless there's a specific need (e.g., to guarantee binary wheel availability for certain Python versions, or to access required features). The goal is to keep the software as broadly compatible as possible with the rest of the ecosystem.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.
51-52: The exact version pins for these IBM transpiler packages are justified. Since qiskit-ibm-transpiler is currently in beta release, using>=constraints would expose the project to unexpected breaking changes as the package evolves. Additionally, the AI local mode dependencies are now included by default as part of the main transpiler package, making coordinated exact pinning the appropriate approach to ensure consistent behavior across development environments.
267-267: Clarify why the GH200 check is being ignored.The change adds
ignore = ["GH200"]to the repo-review configuration. While the commit message indicates this relates to syntax warnings, the specific purpose and appropriateness of ignoring this check should be documented in a comment or commit message for future maintainers. Consider adding a brief explanation inpyproject.toml(e.g.,# Ignoring GH200: <reason>) to clarify the decision.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
pyproject.toml (1)
269-272: CRITICAL: Missing Python version constraint on networkx override will break Python 3.13 installations.The
networkx==2.8.5override lacks a Python version constraint, but NetworkX 2.8.5 only supports Python 3.8–3.10. Since this PR adds Python 3.13 support (line 68), installations on Python 3.13 will fail with dependency resolution errors.This issue was extensively discussed in previous reviews, with web searches confirming NetworkX 2.8.5 incompatibility with Python 3.13. The fix is to add a Python version marker.
Additionally, past reviews found that
qiskit-ibm-transpilerdoesn't actually pin networkx to a specific version. With the update to version 0.15.0, please verify whether this override is still necessary or if it can be relaxed tonetworkx>=2.8or removed entirely.🔎 Apply this fix to add Python version constraint:
[tool.uv] override-dependencies = [ - "networkx==2.8.5", # Required by `qiskit-ibm-transpiler` + "networkx==2.8.5; python_version < '3.13'", # Required by `qiskit-ibm-transpiler` ]Please also verify whether this networkx override is still needed with
qiskit-ibm-transpiler==0.15.0:#!/bin/bash # Check if qiskit-ibm-transpiler 0.15.0 actually requires networkx==2.8.5 echo "=== Checking qiskit-ibm-transpiler 0.15.0 dependencies ===" curl -s https://pypi.org/pypi/qiskit-ibm-transpiler/0.15.0/json | \ jq -r '.info.requires_dist[] | select(contains("networkx"))' echo -e "\n=== Latest networkx versions ===" curl -s https://pypi.org/pypi/networkx/json | \ jq -r '.releases | keys[] | select(startswith("2.8") or startswith("3."))' | \ sort -V | tail -10Based on past review discussions and learnings about maintaining broad dependency compatibility.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(6 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
📚 Learning: 2025-12-13T20:08:45.549Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 862
File: pyproject.toml:65-66
Timestamp: 2025-12-13T20:08:45.549Z
Learning: In the qmap project (pyproject.toml), maintain broad compatibility with dependencies across supported Python versions. Avoid artificially raising minimum version requirements unless there's a specific need (e.g., to guarantee binary wheel availability for certain Python versions, or to access required features). The goal is to keep the software as broadly compatible as possible with the rest of the ecosystem.
Applied to files:
pyproject.toml
📚 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
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-28T14:33:15.199Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1337
File: pyproject.toml:351-351
Timestamp: 2025-11-28T14:33:15.199Z
Learning: In the munich-quantum-toolkit/core repository, the dev dependency "ty==0.0.1a27" is intentionally pinned to an exact pre-release version. This dependency is used by the `ty-check` pre-commit hook (via `uv run --only-dev ty check`), and the exact pin ensures all developers run the same version, preventing unexpected issues from updates since ty is in alpha and changing rapidly.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-13T00:03:08.078Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:252-256
Timestamp: 2025-10-13T00:03:08.078Z
Learning: In uv's `override-dependencies`, multiple entries for the same package with different environment markers (or one without a marker and one with a specific marker) can coexist. uv correctly resolves these by applying the appropriate constraint based on the active Python version. For example, `["symengine>=0.11,<0.14", "symengine>=0.14.1; python_version >= '3.14'"]` is valid and will apply the second constraint on Python 3.14+.
Applied to files:
pyproject.toml
📚 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:
pyproject.toml
📚 Learning: 2025-12-15T01:59:17.023Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Applied to files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (macos-14) / 🐍 macos-14
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
🔇 Additional comments (3)
pyproject.toml (3)
35-38: TODO tracking is in place for the qiskit-ibm-runtime constraint.The upper bound
<0.42.0onqiskit-ibm-runtimeis properly documented with a TODO referencing issue #471. Ensure this constraint is revisited when that issue is resolved.
174-174: LGTM - mypy override for untyped dependency.Adding
qiskit_ibm_transpiler.*to theignore_missing_importslist is standard practice for dependencies without type stubs.
51-52: Remove redundant dependency and verify version pins for transpiler packages.The
qiskit-ibm-ai-local-transpileris now included by default inqiskit-ibm-transpiler; the[ai-local-mode]extra is maintained for backward compatibility but is no longer required. Remove the separateqiskit-ibm-ai-local-transpiler==0.5.5dependency since it's already bundled.Additionally, verify the
qiskit-ibm-transpiler==0.15.0version exists. Current stable releases show version 0.13.1 (released Jul 21, 2025), not 0.15.0. Update to the latest stable version or use a minimum version constraint (e.g.,>=0.13.1) to allow patch updates and maintain compatibility across the ecosystem.⛔ Skipped due to learnings
Learnt from: burgholzer Repo: munich-quantum-toolkit/core PR: 1337 File: pyproject.toml:351-351 Timestamp: 2025-11-28T14:33:15.199Z Learning: In the munich-quantum-toolkit/core repository, the dev dependency "ty==0.0.1a27" is intentionally pinned to an exact pre-release version. This dependency is used by the `ty-check` pre-commit hook (via `uv run --only-dev ty check`), and the exact pin ensures all developers run the same version, preventing unexpected issues from updates since ty is in alpha and changing rapidly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
pyproject.toml (1)
273-276: Add Python version constraint to networkx override for Python 3.13 compatibility.NetworkX 2.8.5 supports only Python 3.8–3.10 and is incompatible with Python 3.13, yet this project declares Python 3.13 support (line 68). The unconditional
networkx==2.8.5override will cause dependency resolution failures on Python 3.13.Additionally, verify whether
qiskit-ibm-transpiler==0.15.0actually requires exactly networkx 2.8.5, or if it can work with newer networkx versions on Python 3.13.🔎 Verification steps
Step 1: Check qiskit-ibm-transpiler 0.15.0 networkx requirements and Python 3.13 support:
#!/bin/bash # Check qiskit-ibm-transpiler 0.15.0 dependency requirements echo "=== qiskit-ibm-transpiler 0.15.0 requirements ===" curl -s https://pypi.org/pypi/qiskit-ibm-transpiler/0.15.0/json | \ jq -r '.info.requires_dist[]? | select(. | contains("networkx"))' echo -e "\n=== Python versions supported by qiskit-ibm-transpiler 0.15.0 ===" curl -s https://pypi.org/pypi/qiskit-ibm-transpiler/0.15.0/json | \ jq -r '.info.requires_python // "Not specified"' echo -e "\n=== Check if networkx 2.8.5 exists and its Python support ===" curl -s https://pypi.org/pypi/networkx/2.8.5/json | \ jq -r '"\nnetworkx 2.8.5 requires Python: " + (.info.requires_python // "Not specified")' echo -e "\n=== Latest networkx 3.x that supports Python 3.13 ===" curl -s https://pypi.org/pypi/networkx/json | \ jq -r '.releases | keys[] | select(startswith("3."))' | sort -V | tail -5Step 2: If qiskit-ibm-transpiler 0.15.0 does not strictly require networkx==2.8.5, remove or relax the override. If it does require 2.8.5 for Python <3.13 but can use newer networkx on 3.13+, apply this fix:
[tool.uv] override-dependencies = [ - "networkx==2.8.5", # Required by `qiskit-ibm-transpiler` + "networkx==2.8.5; python_version < \"3.13\"", # Required by `qiskit-ibm-transpiler` for Python <3.13 ]src/mqt/predictor/rl/predictorenv.py (5)
246-246: Remove unusednoqadirective.The
# noqa: SLF001directive is unnecessary because SLF001 is not enabled in your Ruff configuration. This has been flagged multiple times in previous reviews.- self.state._layout = self.layout # noqa: SLF001 + self.state._layout = self.layout
409-419: Use parameterized logging and consider narrowing exception handling.Two style issues flagged by static analysis:
- F-string logging (lines 410, 418): Use parameterized logging instead of f-strings to defer string formatting and align with Python logging best practices:
- logger.warning(f"[Fallback to SWAP counts] Synthesis or fidelity computation failed: {e}") + logger.warning( + "[Fallback to SWAP counts] Synthesis or fidelity computation failed: %s", + e, + )- logger.exception(f"[Error] Pass failed at iteration {i + 1}") + logger.exception("[Error] Pass failed at iteration %d", i + 1)
- Broad exception handling (lines 409, 417): The blanket
except Exceptioncatches are appropriate for robustness in the RL loop where arbitrary transpiler passes may fail unpredictably. However, if specific Qiskit exception types are expected (e.g.,TranspilerError), narrowing the catch would make failures easier to diagnose.
519-521: Use parameterized logging and avoid broad exception handling.Two issues flagged by static analysis and previous reviews:
Broad exception catch (line 519): If possible, catch the specific TKET exception raised by
get_placement_map(e.g., frompytket.placementorpytket.routing) instead of all exceptions to avoid masking unrelated errors.F-string logging (line 520): Use parameterized logging:
try: placement = transpile_pass[0].get_placement_map(tket_qc) except Exception as e: - logger.warning("Placement failed (%s): %s. Falling back to original circuit.", action.name, e) + logger.warning( + "Placement failed (%s): %s. Falling back to original circuit.", + action.name, + e, + ) return tk_to_qiskit(tket_qc, replace_implicit_swaps=True)
525-527: CRITICAL: Dict key ordering assumption is fragile and incorrect.The current mapping assumes
placement.keys()iteration order corresponds toqc_tmp.qubits[i]indices, which is unsafe.get_placement_mapreturnsdict[Qubit, Node]where both types have.indexattributes, and the dictionary order is not guaranteed to match the qubit order. This was flagged in multiple previous reviews.Use explicit iteration over
placement.items()to map each TKET qubit to its corresponding Qiskit qubit:- qiskit_mapping = { - qc_tmp.qubits[i]: placement[list(placement.keys())[i]].index[0] for i in range(len(placement)) - } + qiskit_mapping = {} + for tket_qubit, node in placement.items(): + qiskit_q = qc_tmp.qubits[tket_qubit.index[0]] + qiskit_mapping[qiskit_q] = node.index[0]This eliminates the fragile ordering assumption and correctly uses the pytket
UnitID.indexattributes.
250-262: Useassert_neveron the discriminant, not the data.Line 261 uses
assert_never(circuit)when it should useassert_never(self.reward_function). Theassert_neverhelper is designed to assert on the discriminant value (the thing being switched on) to prove exhaustiveness, not on unrelated data.if self.reward_function == "critical_depth": return crit_depth(circuit) - assert_never(circuit) + assert_never(self.reward_function)
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
pyproject.toml(6 hunks)src/mqt/predictor/rl/predictorenv.py(12 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
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.
📚 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:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-08T11:32:57.308Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1375
File: python/mqt/core/plugins/qiskit/converters.py:71-78
Timestamp: 2025-12-08T11:32:57.308Z
Learning: In the munich-quantum-toolkit/core repository, helper functions like `_raise_error` are used to satisfy Ruff's TRY301 rule when raising exceptions in nested contexts (e.g., within try/except blocks). This pattern is necessary given the project's strict linting configuration.
Applied to files:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-04T06:59:40.314Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.314Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Applied to files:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-14T15:23:54.712Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 23
File: docs/conf.py:110-130
Timestamp: 2025-12-14T15:23:54.712Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the Ruff configuration has 'ALL' rules enabled with only specific rules disabled. PLR6301 (no-self-use) is active, so `# noqa: PLR6301` directives are necessary for methods that don't use self, even if Ruff reports RUF100 warnings suggesting the directive is unused.
Applied to files:
src/mqt/predictor/rl/predictorenv.pypyproject.toml
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
src/mqt/predictor/rl/predictorenv.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/predictor/rl/predictorenv.py
📚 Learning: 2025-11-26T10:29:36.435Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/fomac/test_fomac.py:54-66
Timestamp: 2025-11-26T10:29:36.435Z
Learning: In test/python/fomac/test_fomac.py, the `ddsim_device` fixture returns `Device | None` even though it only returns a `Device` or calls `pytest.skip()`. The `| None` annotation is required for the type checker to pass, as type checkers may not recognize `pytest.skip()` as a function that never returns.
Applied to files:
src/mqt/predictor/rl/predictorenv.py
📚 Learning: 2025-12-13T20:08:45.549Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 862
File: pyproject.toml:65-66
Timestamp: 2025-12-13T20:08:45.549Z
Learning: In the qmap project (pyproject.toml), maintain broad compatibility with dependencies across supported Python versions. Avoid artificially raising minimum version requirements unless there's a specific need (e.g., to guarantee binary wheel availability for certain Python versions, or to access required features). The goal is to keep the software as broadly compatible as possible with the rest of the ecosystem.
Applied to files:
pyproject.toml
📚 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
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-28T14:33:15.199Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1337
File: pyproject.toml:351-351
Timestamp: 2025-11-28T14:33:15.199Z
Learning: In the munich-quantum-toolkit/core repository, the dev dependency "ty==0.0.1a27" is intentionally pinned to an exact pre-release version. This dependency is used by the `ty-check` pre-commit hook (via `uv run --only-dev ty check`), and the exact pin ensures all developers run the same version, preventing unexpected issues from updates since ty is in alpha and changing rapidly.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-13T00:03:08.078Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:252-256
Timestamp: 2025-10-13T00:03:08.078Z
Learning: In uv's `override-dependencies`, multiple entries for the same package with different environment markers (or one without a marker and one with a specific marker) can coexist. uv correctly resolves these by applying the appropriate constraint based on the active Python version. For example, `["symengine>=0.11,<0.14", "symengine>=0.14.1; python_version >= '3.14'"]` is valid and will apply the second constraint on Python 3.14+.
Applied to files:
pyproject.toml
📚 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:
pyproject.toml
📚 Learning: 2025-12-15T01:59:17.023Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Applied to files:
pyproject.toml
🧬 Code graph analysis (1)
src/mqt/predictor/rl/predictorenv.py (4)
src/mqt/predictor/rl/actions.py (4)
CompilationOrigin(100-106)DeviceDependentAction(147-158)PassType(109-118)Action(122-138)src/mqt/predictor/rl/helper.py (2)
create_feature_dict(84-110)get_path_training_circuits(123-125)src/mqt/predictor/rl/parsing.py (2)
final_layout_bqskit_to_qiskit(186-226)prepare_noise_data(253-278)src/mqt/predictor/utils.py (1)
get_openqasm_gates_without_u(197-233)
🪛 Ruff (0.14.8)
src/mqt/predictor/rl/predictorenv.py
246-246: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
409-409: Do not catch blind exception: Exception
(BLE001)
410-410: Logging statement uses f-string
(G004)
418-418: Logging statement uses f-string
(G004)
519-519: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. 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-14) / 🐍 macos-14
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
🔇 Additional comments (1)
src/mqt/predictor/rl/predictorenv.py (1)
241-244: Consider whetherunitaryandcliffordgates can coexist.The current
if/elifstructure only decomposesunitarygates when both are present. If circuits can contain both gate types simultaneously and both require decomposition beforeBasisTranslator, use two independentifstatements:if self.state.count_ops().get("unitary"): self.state = self.state.decompose(gates_to_decompose="unitary") - elif self.state.count_ops().get("clifford"): + if self.state.count_ops().get("clifford"): self.state = self.state.decompose(gates_to_decompose="clifford")However, if these gate types are mutually exclusive or decomposing
unitaryalready handlesclifford, the current code is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/mqt/predictor/rl/actions.py (2)
142-143: Simplify type annotation forstochasticandpreserve_layout.As noted in a previous review, the type
bool | None = Falseis confusing—ifFalseis a valid default,Noneisn't a meaningful value. Simplify tobool = False.- stochastic: bool | None = False - preserve_layout: bool | None = False + stochastic: bool = False + preserve_layout: bool = False
726-737: Consider defensive handling instead of assertions for production resilience.As noted in a prior review, these assertions will crash in production if
AIRoutingreturns unexpected results. For a wrapper around an external pass, graceful degradation (logging warnings, returning the unmodified routed circuit) may be more robust than hard failures.Additionally, Line 737 uses
qc_routed.qubits.index(phys)which is O(n) per lookup. For large circuits, consider building a reverse lookup dict.pyproject.toml (1)
273-276: CRITICAL: networkx override lacks Python version constraint and breaks Python 3.13 compatibility.The
networkx==2.8.5override has no Python version constraint, but networkx 2.8.5 only supports Python 3.8–3.10 and is incompatible with Python 3.13. Since:
- This project declares Python 3.13 support (line 68)
qiskit-ibm-transpileris installed on Linux/macOS Python 3.13 (line 51 constraint excludes only Windows+3.13)- uv's
override-dependenciesapplies globally during resolutionThis will cause installation failures on Python 3.13 (non-Windows platforms).
🔎 Apply this diff to add the required Python version constraint:
[tool.uv] override-dependencies = [ - "networkx==2.8.5", # Required by `qiskit-ibm-transpiler` + "networkx==2.8.5; python_version < \"3.13\"", # Required by `qiskit-ibm-transpiler` ]Based on past review comments, this fix was identified and discussed but not yet applied. The constraint ensures networkx 2.8.5 is only used on Python versions it supports.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
pyproject.toml(6 hunks)src/mqt/predictor/rl/actions.py(19 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
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.
📚 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:
src/mqt/predictor/rl/actions.pypyproject.toml
📚 Learning: 2025-12-04T06:59:40.314Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.314Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Applied to files:
src/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
src/mqt/predictor/rl/actions.pypyproject.toml
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
src/mqt/predictor/rl/actions.pypyproject.toml
📚 Learning: 2025-12-13T20:08:45.549Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 862
File: pyproject.toml:65-66
Timestamp: 2025-12-13T20:08:45.549Z
Learning: In the qmap project (pyproject.toml), maintain broad compatibility with dependencies across supported Python versions. Avoid artificially raising minimum version requirements unless there's a specific need (e.g., to guarantee binary wheel availability for certain Python versions, or to access required features). The goal is to keep the software as broadly compatible as possible with the rest of the ecosystem.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-28T14:33:15.199Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1337
File: pyproject.toml:351-351
Timestamp: 2025-11-28T14:33:15.199Z
Learning: In the munich-quantum-toolkit/core repository, the dev dependency "ty==0.0.1a27" is intentionally pinned to an exact pre-release version. This dependency is used by the `ty-check` pre-commit hook (via `uv run --only-dev ty check`), and the exact pin ensures all developers run the same version, preventing unexpected issues from updates since ty is in alpha and changing rapidly.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-13T00:03:08.078Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:252-256
Timestamp: 2025-10-13T00:03:08.078Z
Learning: In uv's `override-dependencies`, multiple entries for the same package with different environment markers (or one without a marker and one with a specific marker) can coexist. uv correctly resolves these by applying the appropriate constraint based on the active Python version. For example, `["symengine>=0.11,<0.14", "symengine>=0.14.1; python_version >= '3.14'"]` is valid and will apply the second constraint on Python 3.14+.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-14T15:23:54.712Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 23
File: docs/conf.py:110-130
Timestamp: 2025-12-14T15:23:54.712Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the Ruff configuration has 'ALL' rules enabled with only specific rules disabled. PLR6301 (no-self-use) is active, so `# noqa: PLR6301` directives are necessary for methods that don't use self, even if Ruff reports RUF100 warnings suggesting the directive is unused.
Applied to files:
pyproject.toml
📚 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:
pyproject.toml
📚 Learning: 2025-12-15T01:59:17.023Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Applied to files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🐍 Test (macos-14) / 🐍 macos-14
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
🔇 Additional comments (9)
src/mqt/predictor/rl/actions.py (6)
89-92: Clarify scope of Windows Python 3.13 restriction.The condition
sys.version_info[:2] == (3, 13)only matches Python 3.13 exactly. If the underlying issue (e.g., threading/multiprocessing on Windows) persists in Python 3.14+, this check will silently break. Consider:
- Using
>= (3, 13)if the issue affects all future versions on Windows- Adding a comment explaining why only 3.13 is affected if this is intentional
-IS_WIN_PY313 = sys.platform == "win32" and sys.version_info[:2] == (3, 13) +# AIRouting has threading issues on Windows with Python 3.13+ (single-threaded fallback) +IS_WIN_PY313 = sys.platform == "win32" and sys.version_info >= (3, 13)
204-212: LGTM!The
Optimize1qGatesDecomposition_preserveaction correctly takes the device's operation names and setspreserve_layout=Trueto maintain the existing qubit mapping after circuit optimization.
496-529: LGTM!The AIRouting actions are properly gated behind the Windows Python 3.13 check and correctly use the
SafeAIRoutingwrapper to handle classical registers. TheAIRouting_optvariant appropriately combinesSabreLayoutfor initial layout withSafeAIRoutingin "optimize" mode.
531-541: Verifymax_iterations=1aligns with RL training strategy.Setting
max_iterations=1makes eachSabreMappingcall deterministic (no internal random restarts). This is reasonable if the RL environment handles multiple trials externally for stochastic actions, but could reduce solution quality if only called once.
657-664: Verify intentional removal of barriers inremove_cregs.The function removes both
measureandbarrieroperations. While measurements need to be stripped for AIRouting, barriers might carry scheduling semantics that should be preserved. If barriers should be kept, remove"barrier"from the exclusion check.- if instr.name in ("measure", "barrier"): + if instr.name == "measure":
278-285: LGTM!Correctly prepending
CollectCliffords()beforeOptimizeCliffords()ensures Clifford blocks are collected before optimization, fixing the previous issue whereOptimizeCliffordshad no blocks to optimize.pyproject.toml (3)
40-40: LGTM: stable-baselines3 dependency added with flexible version constraint.The addition of
stable-baselines3>=2.7.0aligns with the RL predictor enhancements mentioned in the PR objectives, and the flexible>=constraint maintains broad ecosystem compatibility.
178-178: LGTM: qiskit_ibm_transpiler added to mypy ignore list.Adding
qiskit_ibm_transpiler.*to the mypy ignore list is appropriate, as this package lacks type stubs. This follows the established pattern for other untyped dependencies in the project.
51-52: Clarify Python 3.13 support for qiskit-ibm-transpiler 0.15.0.The constraint excludes qiskit-ibm-transpiler only on Windows+Python 3.13 but allows it on Linux/macOS with Python 3.13. Since qiskit-ibm-transpiler 0.15.0 has not yet been released, confirm that this version will support Python 3.13 on non-Windows platforms before merging. If 0.15.0 uses ABI3 wheels like Qiskit, Python 3.13 compatibility should be fine; if not, the constraint may need adjustment or version bump deferral until 0.15.0 officially declares 3.13 support.
| transpile_pass: ( | ||
| list[qiskit_BasePass | tket_BasePass] | ||
| | Callable[..., list[Any]] | ||
| | Callable[..., list[qiskit_BasePass | tket_BasePass]] | ||
| | Callable[ | ||
| ..., | ||
| Callable[..., tuple[Any, ...] | Circuit], | ||
| ] | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Broad type annotation reduces type safety.
The addition of Callable[..., list[Any]] (line 135) is very permissive and undermines the type system's ability to catch misuse. Consider keeping the more specific union without list[Any], or documenting which action types require this flexibility.
🤖 Prompt for AI Agents
In src/mqt/predictor/rl/actions.py around lines 133 to 141, the union type for
transpile_pass currently includes the overly-broad Callable[..., list[Any]]
which weakens type safety; remove that permissive member and instead restrict
callables to return the concrete pass types (e.g. Callable[...,
list[qiskit_BasePass | tket_BasePass]] or Callable[..., tuple[Any, ...] |
Circuit] as already present), and if dynamic/heterogeneous returns are required,
add a specific typed alias or a clear docstring explaining the allowed runtime
shapes so callers and linters can rely on precise types.
Signed-off-by: Shaobo-Zhou <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @noxfile.py:
- Around line 90-106: De-indent the @nox.session decorators and the definitions
of tests and minimums so they are module-level functions (same top-level
indentation as lint) instead of nested inside lint; specifically move the
decorators and the def tests(session: nox.Session) and def minimums(session:
nox.Session) left to top-level indentation so Nox can register them, while
leaving the existing _run_tests helper nested inside lint as intended.
In @pyproject.toml:
- Around line 274-277: The pinned dependency "networkx==2.8.5" under [tool.uv]
override-dependencies is overly restrictive; remove the exact pin or loosen it
(e.g., allow a modern, compatible range) and update or remove the comment about
qiskit-ibm-transpiler—qiskit-ibm-transpiler does not enforce a strict networkx
version. Locate the override-dependencies list and either delete the
"networkx==2.8.5" entry or replace it with a range (for example a >= constraint
or a broad <4 upper bound) so newer compatible networkx releases can be used.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
noxfile.pypyproject.toml
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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.
📚 Learning: 2025-12-13T20:08:45.549Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 862
File: pyproject.toml:65-66
Timestamp: 2025-12-13T20:08:45.549Z
Learning: In the qmap project (pyproject.toml), maintain broad compatibility with dependencies across supported Python versions. Avoid artificially raising minimum version requirements unless there's a specific need (e.g., to guarantee binary wheel availability for certain Python versions, or to access required features). The goal is to keep the software as broadly compatible as possible with the rest of the ecosystem.
Applied to files:
pyproject.toml
📚 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
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-28T14:33:15.199Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1337
File: pyproject.toml:351-351
Timestamp: 2025-11-28T14:33:15.199Z
Learning: In the munich-quantum-toolkit/core repository, the dev dependency "ty==0.0.1a27" is intentionally pinned to an exact pre-release version. This dependency is used by the `ty-check` pre-commit hook (via `uv run --only-dev ty check`), and the exact pin ensures all developers run the same version, preventing unexpected issues from updates since ty is in alpha and changing rapidly.
Applied to files:
pyproject.tomlnoxfile.py
📚 Learning: 2025-10-13T00:03:08.078Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:252-256
Timestamp: 2025-10-13T00:03:08.078Z
Learning: In uv's `override-dependencies`, multiple entries for the same package with different environment markers (or one without a marker and one with a specific marker) can coexist. uv correctly resolves these by applying the appropriate constraint based on the active Python version. For example, `["symengine>=0.11,<0.14", "symengine>=0.14.1; python_version >= '3.14'"]` is valid and will apply the second constraint on Python 3.14+.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-14T15:23:54.712Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 23
File: docs/conf.py:110-130
Timestamp: 2025-12-14T15:23:54.712Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the Ruff configuration has 'ALL' rules enabled with only specific rules disabled. PLR6301 (no-self-use) is active, so `# noqa: PLR6301` directives are necessary for methods that don't use self, even if Ruff reports RUF100 warnings suggesting the directive is unused.
Applied to files:
pyproject.toml
📚 Learning: 2026-01-08T10:07:23.393Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 20
File: python/mqt/core/plugins/catalyst/device.py:69-69
Timestamp: 2026-01-08T10:07:23.393Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, Ruff is configured with select = ["ALL"] and only specific rules are disabled. SLF001 (flake8-self - private member access) is enabled, so `# noqa: SLF001` directives are necessary when accessing private attributes like `device._to_matrix_ops` in python/mqt/core/plugins/catalyst/device.py, even if Ruff reports RUF100 warnings suggesting the directive is unused.
Applied to files:
pyproject.toml
📚 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:
pyproject.toml
📚 Learning: 2025-11-24T10:18:23.706Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: pyproject.toml:148-154
Timestamp: 2025-11-24T10:18:23.706Z
Learning: In pytest 9.0+, the `strict = true` configuration option is a global setting that automatically enables strict_config, strict_markers, strict_parametrization_ids, and strict_xfail. This is different from earlier pytest versions where these options had to be enabled separately. When `strict = true` is set in pytest 9.0+, custom markers do not need separate registration in many cases, and the strict marker checking is already enabled.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-15T01:59:17.023Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-27T16:58:08.564Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/ddsim PR: 720
File: noxfile.py:57-60
Timestamp: 2025-11-27T16:58:08.564Z
Learning: In Nox sessions, when using `session.install()` to install a tool and then `session.run(..., external=True)` to execute it, this pattern is valid because the session's virtual environment is activated and its bin directory is on PATH. The `external=True` flag allows calling executables on PATH, which includes the activated venv's bin directory, so it will find tools installed via `session.install()`.
Applied to files:
noxfile.py
🔇 Additional comments (7)
pyproject.toml (5)
35-40: LGTM! Dependency updates align with PR scope.The pytket_qiskit bump to >=0.71.0 supports the new layout actions (GraphPlacement, NoiseAwarePlacement) and optimizations added in this PR. The stable-baselines3>=2.7.0 addition is appropriate for the expanded RL functionality.
Note: The qiskit-ibm-runtime constraint includes a TODO at line 36 to remove the <0.42.0 upper bound once pytket_qiskit is updated.
112-112: LGTM! Correct pytest configuration format.The rename from
[tool.pytest]to[tool.pytest.ini_options]follows the standard pytest configuration format for pyproject.toml.
122-140: LGTM! Appropriate workarounds for upstream issues.The filterwarnings additions properly suppress known issues:
- Lines 122-128: Temporary ignores for qiskit-ibm-transpiler SyntaxWarnings until upstream fixes
- Lines 138-140: Platform-specific ignore for Windows timeout limitations
The comments clearly document the temporary nature of these suppressions. Consider tracking the upstream issues to remove these ignores once fixed.
179-179: LGTM! Mypy override for new dependency.Added
qiskit_ibm_transpiler.*to the ignore_missing_imports override, consistent with other untyped dependencies.
51-52: Dependency versions are current and appropriately support Python 3.13.Version 0.15.0 of
qiskit-ibm-transpilerand 0.5.5 ofqiskit-ibm-ai-local-transpilerare both the latest releases (Dec 2025) and explicitly support Python 3.13. The platform constraint on line 51 correctly excludesqiskit-ibm-transpileronly on Windows Python 3.13, aligning with the conditionalSafeAIRoutingimplementation.noxfile.py (2)
62-88: LGTM! Test execution logic is well-structured (pending indentation fix).The
_run_testshelper properly encapsulates test execution with good features:
- Flexible parameters for install args, extra commands, and pytest args
- Proper environment variable setup for uv
- Smart optimization: enables
sysmoncoverage core for Python 3.12+ when--covis present (Line 73-75)- Clean parameter forwarding to pytest
Note: This approval assumes the function placement/indentation is corrected per the critical issue flagged above.
95-106: LGTM! Minimum dependency version testing is properly implemented (pending indentation fix).The
minimumssession correctly:
- Preserves and restores the lockfile via context manager (Line 98)
- Tests with
--resolution=lowest-directto verify minimum versions (Line 101)- Enables all warnings with
-Wdefault(Line 102)- Verifies dependency tree and refreshes lock after testing (Lines 105-106)
This provides good coverage for ensuring the declared minimum dependency versions actually work.
Note: This approval assumes the function placement/indentation is corrected per the critical issue flagged above.
| [tool.uv] | ||
| override-dependencies = [ | ||
| "networkx==2.8.5", # Required by `qiskit-ibm-transpiler` | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
networkx 2.8.5 security vulnerabilities CVE 2024 2025
💡 Result:
Short answer: There are no NVD/CVE entries that specifically name NetworkX 2.8.5 for 2024–2025. However, two relevant security notes you should know:
- A YAML deserialization (unsafe yaml.load) issue (arbitrary code execution) was reported for older NetworkX releases and Snyk advises upgrading to ≥2.6 to avoid it; that advisory affects versions before 2.6—not 2.8.5. [1][2]
- In June 2025 there were reports of malicious/typo-squatting packages named "networkx" on package registries (scanner/Snyk/Vuln feeds); those are supply‑chain incidents in package registries rather than CVEs assigned to the official NetworkX project. No CVE was listed for that incident in the sources I found. [3][4]
Sources:
- Snyk advisory — insecure YAML deserialization / read_yaml (upgrade to ≥2.6). [Snyk]. [2]
- NetworkX PyPI page for 2.8.5. [PyPI]. [5]
- Snyk (malicious package / registry incident, June 10, 2025). [3]
- Vuln aggregator report on malicious networkx package. [4]
If you want, I can (a) search NVD again for any CVE that may have been published after these sources, or (b) check your installed environment for any transitive dependency or registry‑typo packages that might be malicious.
🌐 Web query:
qiskit-ibm-transpiler 0.15.0 networkx version compatibility requirements
💡 Result:
qiskit-ibm-transpiler v0.15.0 lists networkx as a dependency but does not specify a version constraint (i.e., simply "networkx"). Sources: PyPI package metadata and piwheels listing show networkx in the requirements, and the release notes state networkx was added to the requirements. [1][2][3]
Sources
- PyPI qiskit-ibm-transpiler (0.15.0) release/files. [1]
- piwheels package page (qiskit-ibm-transpiler dependencies include networkx). [2]
- qiskit-ibm-transpiler release notes (notes about adding networkx to requirements). [3]
🌐 Web query:
networkx latest version 2025
💡 Result:
-
Latest release during 2025: networkx 3.5 — released May 29, 2025. [1]
-
Current latest (as of Jan 8, 2026): networkx 3.6 — released Nov 24, 2025. [1][2]
Sources:
[1] PyPI networkx release details (networkx‑3.5).
[2] networkx GitHub Releases (networkx 3.6).
Remove or loosen the networkx==2.8.5 pin; qiskit-ibm-transpiler does not enforce a specific version.
The comment states this pin is "Required by qiskit-ibm-transpiler", but qiskit-ibm-transpiler 0.15.0 does not specify a version constraint on networkx—it simply lists networkx as a dependency. The exact pin to 2.8.5 is unnecessarily restrictive, as it is ~2 years outdated (current is 3.6, released Nov 2025). If this pin was added to address a specific compatibility issue that has since been resolved, consider removing it or updating the comment to reflect the actual constraint.
🤖 Prompt for AI Agents
In @pyproject.toml around lines 274 - 277, The pinned dependency
"networkx==2.8.5" under [tool.uv] override-dependencies is overly restrictive;
remove the exact pin or loosen it (e.g., allow a modern, compatible range) and
update or remove the comment about qiskit-ibm-transpiler—qiskit-ibm-transpiler
does not enforce a strict networkx version. Locate the override-dependencies
list and either delete the "networkx==2.8.5" entry or replace it with a range
(for example a >= constraint or a broad <4 upper bound) so newer compatible
networkx releases can be used.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/mqt/predictor/rl/actions.py:
- Around line 497-530: The comment preceding the SabreLayout call uses a
triple-hash (###) instead of the project's standard single-hash comment prefix;
change the comment to start with a single '#' to match style conventions near
the DeviceDependentAction registration (refer to the SabreLayout,
FullAncillaAllocation, EnlargeWithAncilla, ApplyLayout, and SafeAIRouting block)
so the inline note about initial layouts reads with a single '#' prefix for
consistency.
- Around line 718-746: The run method in actions.py uses assert statements for
critical checks (final_layout presence and qubit mapping) which can be skipped
with -O; replace each assert in run (the checks for final_layout is not None,
virt in final_layout, physical index bounds when phys is int, and phys
membership when phys is a qubit object) with explicit exception handling: raise
a clear exception type (e.g., ValueError or RuntimeError) with the same
diagnostic message, validate types where needed before indexing into
qc_routed.qubits, and ensure qubit_map is only populated after successful checks
so add_guard clauses around accesses in run to prevent silent failures.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/mqt/predictor/rl/actions.py
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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:
src/mqt/predictor/rl/actions.py
📚 Learning: 2025-12-25T13:28:19.850Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/predictor PR: 526
File: src/mqt/predictor/rl/predictorenv.py:271-271
Timestamp: 2025-12-25T13:28:19.850Z
Learning: In the munich-quantum-toolkit/predictor repository, Ruff has a broad rule set including FLake8-SLF (slf) in extend-select. For private member access (e.g., self.state._layout = self.layout) in src/mqt/predictor/rl/predictorenv.py, include a per-file noqa directive: # noqa: SLF001. This is appropriate even if Ruff reports RUF100 as unused, to acknowledge intentional private attribute access and to avoid false positives in this specific code path. Apply this directive only to the files where private-member access is intentionally used and where SLF001 is the correct rationale.
Applied to files:
src/mqt/predictor/rl/actions.py
📚 Learning: 2025-12-04T06:59:40.314Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.314Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Applied to files:
src/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
src/mqt/predictor/rl/actions.py
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
src/mqt/predictor/rl/actions.py
🔇 Additional comments (11)
src/mqt/predictor/rl/actions.py (11)
14-14: LGTM!The conditional import guard for AIRouting on Windows + Python 3.13 is a reasonable workaround for platform-specific compatibility issues.
Also applies to: 89-92
26-26: LGTM!The new imports support the expanded action space (GraphPlacement, NoiseAwarePlacement, KAKDecomposition, ElidePermutations, SabreSwap) and helper utilities for classical register management.
Also applies to: 31-34, 51-54, 58-58, 64-64, 75-75
100-101: LGTM!The dataclass extensions (stochastic and preserve_layout flags) and type annotation widening support the expanded action space and layout-preservation semantics described in the PR objectives.
Also applies to: 133-143, 155-162
283-283: LGTM!Adding CollectCliffords() before OptimizeCliffords() ensures the correct pass ordering, as mentioned in the PR objectives.
204-212: LGTM!The preserve_layout flags correctly identify optimization passes that maintain the existing qubit layout, and the new device-aware optimization action (Optimize1qGatesDecomposition_preserve) appropriately takes the device basis as a parameter.
Also applies to: 220-220, 230-230, 240-242, 244-251, 274-274
296-308: LGTM!The new layout-preserving optimization actions (Opt2qBlocks_preserve with device parameters, KAKDecomposition with allow_swaps=False, QiskitO3 with preserve_layout) correctly support the PR's goal of maintaining layouts during optimization.
Also applies to: 328-335, 380-380
444-471: LGTM!The new placement actions (GraphPlacement and NoiseAwarePlacement) support fidelity-aware compilation as described in the PR objectives. The timeout and maximum_matches parameters provide reasonable resource bounds.
485-495: LGTM!The SabreSwap action is correctly marked as stochastic and uses the "decay" heuristic for routing.
537-540: Verify that max_iterations=1 provides acceptable layout quality.The max_iterations parameter was set to 1, which significantly limits the search space for SabreLayout. While the PR objectives indicate this is intentional (to optimize for a figure of merit with multi-trial evaluation), ensure this doesn't excessively degrade layout quality in practice.
Based on PR objectives, this is part of the stochastic pass strategy where the RL environment performs multi-trial evaluation.
616-706: LGTM!The helper functions correctly manage classical registers and measurements around routing operations. The approach of preserving register identity (not cloning) is important for maintaining circuit structure in Qiskit.
405-405: The time_limit=100 setting is appropriate and aligns with industry practice.Qiskit's own preset passmanagers use time limits of 10 seconds (level 2) and 60 seconds (level 3). The 100-second limit chosen here is more generous, which is reasonable for a final optimization pass. The existing test in
tests/compilation/test_helper_rl.pyconfirms that VF2PostLayout completes successfully on typical benchmark circuits (3-qubit GHZ on IBM Falcon 27) with this limit, finding a solution without hitting the timeout. No changes needed.
| if not IS_WIN_PY313: | ||
| register_action( | ||
| DeviceDependentAction( | ||
| "AIRouting", | ||
| CompilationOrigin.QISKIT, | ||
| PassType.ROUTING, | ||
| stochastic=True, | ||
| transpile_pass=lambda device: [ | ||
| SafeAIRouting( | ||
| coupling_map=device.build_coupling_map(), | ||
| optimization_level=3, | ||
| layout_mode="improve", | ||
| local_mode=True, | ||
| ) | ||
| ], | ||
| ) | ||
| ) | ||
|
|
||
| register_action( | ||
| DeviceDependentAction( | ||
| "AIRouting_opt", | ||
| CompilationOrigin.QISKIT, | ||
| PassType.MAPPING, | ||
| stochastic=True, | ||
| transpile_pass=lambda device: [ | ||
| ### Requires an initial layout, but "optimize" mode overwrites it | ||
| SabreLayout(coupling_map=CouplingMap(device.build_coupling_map()), skip_routing=True, max_iterations=1), | ||
| FullAncillaAllocation(coupling_map=CouplingMap(device.build_coupling_map())), | ||
| EnlargeWithAncilla(), | ||
| ApplyLayout(), | ||
| SafeAIRouting(coupling_map=device.build_coupling_map(), optimization_level=3, layout_mode="optimize"), | ||
| ], | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor style issue: standardize comment prefix.
The comment on line 522 uses ### instead of the standard #. Consider using a single # for consistency.
♻️ Proposed fix
- ### Requires an initial layout, but "optimize" mode overwrites it
+ # Requires an initial layout, but "optimize" mode overwrites itOtherwise, the AIRouting actions are correctly configured with appropriate layout modes and the stochastic flag.
📝 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.
| if not IS_WIN_PY313: | |
| register_action( | |
| DeviceDependentAction( | |
| "AIRouting", | |
| CompilationOrigin.QISKIT, | |
| PassType.ROUTING, | |
| stochastic=True, | |
| transpile_pass=lambda device: [ | |
| SafeAIRouting( | |
| coupling_map=device.build_coupling_map(), | |
| optimization_level=3, | |
| layout_mode="improve", | |
| local_mode=True, | |
| ) | |
| ], | |
| ) | |
| ) | |
| register_action( | |
| DeviceDependentAction( | |
| "AIRouting_opt", | |
| CompilationOrigin.QISKIT, | |
| PassType.MAPPING, | |
| stochastic=True, | |
| transpile_pass=lambda device: [ | |
| ### Requires an initial layout, but "optimize" mode overwrites it | |
| SabreLayout(coupling_map=CouplingMap(device.build_coupling_map()), skip_routing=True, max_iterations=1), | |
| FullAncillaAllocation(coupling_map=CouplingMap(device.build_coupling_map())), | |
| EnlargeWithAncilla(), | |
| ApplyLayout(), | |
| SafeAIRouting(coupling_map=device.build_coupling_map(), optimization_level=3, layout_mode="optimize"), | |
| ], | |
| ) | |
| ) | |
| if not IS_WIN_PY313: | |
| register_action( | |
| DeviceDependentAction( | |
| "AIRouting", | |
| CompilationOrigin.QISKIT, | |
| PassType.ROUTING, | |
| stochastic=True, | |
| transpile_pass=lambda device: [ | |
| SafeAIRouting( | |
| coupling_map=device.build_coupling_map(), | |
| optimization_level=3, | |
| layout_mode="improve", | |
| local_mode=True, | |
| ) | |
| ], | |
| ) | |
| ) | |
| register_action( | |
| DeviceDependentAction( | |
| "AIRouting_opt", | |
| CompilationOrigin.QISKIT, | |
| PassType.MAPPING, | |
| stochastic=True, | |
| transpile_pass=lambda device: [ | |
| # Requires an initial layout, but "optimize" mode overwrites it | |
| SabreLayout(coupling_map=CouplingMap(device.build_coupling_map()), skip_routing=True, max_iterations=1), | |
| FullAncillaAllocation(coupling_map=CouplingMap(device.build_coupling_map())), | |
| EnlargeWithAncilla(), | |
| ApplyLayout(), | |
| SafeAIRouting(coupling_map=device.build_coupling_map(), optimization_level=3, layout_mode="optimize"), | |
| ], | |
| ) | |
| ) |
🤖 Prompt for AI Agents
In @src/mqt/predictor/rl/actions.py around lines 497 - 530, The comment
preceding the SabreLayout call uses a triple-hash (###) instead of the project's
standard single-hash comment prefix; change the comment to start with a single
'#' to match style conventions near the DeviceDependentAction registration
(refer to the SabreLayout, FullAncillaAllocation, EnlargeWithAncilla,
ApplyLayout, and SafeAIRouting block) so the inline note about initial layouts
reads with a single '#' prefix for consistency.
| def run(self, dag: DAGCircuit) -> DAGCircuit: | ||
| """Run the routing pass on a DAGCircuit.""" | ||
| qc_orig = dag_to_circuit(dag) | ||
| # Extract classical registers and measurement instructions | ||
| cregs, measurements = extract_cregs_and_measurements(qc_orig) | ||
| # Remove cregs and measurements | ||
| qc_noclassical = remove_cregs(qc_orig) | ||
| # Convert back to dag and run routing (AIRouting) | ||
| dag_noclassical = circuit_to_dag(qc_noclassical) | ||
| dag_routed = super().run(dag_noclassical) | ||
| # Convert routed dag to circuit for restoration | ||
| qc_routed = dag_to_circuit(dag_routed) | ||
| # Build mapping from original qubits to qubits in routed circuit | ||
| final_layout = getattr(self, "property_set", {}).get("final_layout", None) | ||
| assert final_layout is not None, "final_layout is None — cannot map virtual qubits" | ||
| qubit_map = {} | ||
| for virt in qc_orig.qubits: | ||
| assert virt in final_layout, f"Virtual qubit {virt} not found in final layout!" | ||
| phys = final_layout[virt] | ||
| if isinstance(phys, int): | ||
| assert 0 <= phys < len(qc_routed.qubits), f"Physical index {phys} out of range in routed circuit!" | ||
| qubit_map[virt] = qc_routed.qubits[phys] | ||
| else: | ||
| assert phys in qc_routed.qubits, f"Physical qubit {phys} not found in output circuit!" | ||
| qubit_map[virt] = qc_routed.qubits[qc_routed.qubits.index(phys)] | ||
| # Restore classical registers and measurement instructions | ||
| qc_final = add_cregs_and_measurements(qc_routed, cregs, measurements, qubit_map) | ||
| # Return as dag | ||
| return circuit_to_dag(qc_final) |
There was a problem hiding this comment.
Replace assertions with explicit error handling.
The run method uses multiple assert statements (lines 732, 735, 738, 741) that can be disabled with Python's -O flag. In production code, these critical checks should raise explicit exceptions.
🔒 Proposed fix to replace assertions with explicit checks
final_layout = getattr(self, "property_set", {}).get("final_layout", None)
- assert final_layout is not None, "final_layout is None — cannot map virtual qubits"
+ if final_layout is None:
+ msg = "final_layout is None — cannot map virtual qubits"
+ raise RuntimeError(msg)
qubit_map = {}
for virt in qc_orig.qubits:
- assert virt in final_layout, f"Virtual qubit {virt} not found in final layout!"
+ if virt not in final_layout:
+ msg = f"Virtual qubit {virt} not found in final layout!"
+ raise RuntimeError(msg)
phys = final_layout[virt]
if isinstance(phys, int):
- assert 0 <= phys < len(qc_routed.qubits), f"Physical index {phys} out of range in routed circuit!"
+ if not 0 <= phys < len(qc_routed.qubits):
+ msg = f"Physical index {phys} out of range in routed circuit!"
+ raise ValueError(msg)
qubit_map[virt] = qc_routed.qubits[phys]
else:
- assert phys in qc_routed.qubits, f"Physical qubit {phys} not found in output circuit!"
+ if phys not in qc_routed.qubits:
+ msg = f"Physical qubit {phys} not found in output circuit!"
+ raise ValueError(msg)
qubit_map[virt] = qc_routed.qubits[qc_routed.qubits.index(phys)]🤖 Prompt for AI Agents
In @src/mqt/predictor/rl/actions.py around lines 718 - 746, The run method in
actions.py uses assert statements for critical checks (final_layout presence and
qubit mapping) which can be skipped with -O; replace each assert in run (the
checks for final_layout is not None, virt in final_layout, physical index bounds
when phys is int, and phys membership when phys is a qubit object) with explicit
exception handling: raise a clear exception type (e.g., ValueError or
RuntimeError) with the same diagnostic message, validate types where needed
before indexing into qc_routed.qubits, and ensure qubit_map is only populated
after successful checks so add_guard clauses around accesses in run to prevent
silent failures.
4 participants
Description
This PR refactors the underlying MDP used by the RL-based predictor. It expands the action space, refines state transitions and observations, and adds support for stochastic transpiler passes.
🚀 Major Changes
Expanded Action Space
AIRoutingas a routing/mapping option and wrapped inSafeAIRoutingfor stable integration into the RL pipeline.GraphPlacementandNoiseAwarePlacement(fidelity-aware).KAKDecompositionandElidePermutations.Support for Stochastic Passes
AIRouting,SabreLayout) in a multi-trial evaluation loop, optimizing for the figure of merit instead of the default gate count internally in Qiskit passes.max_iterationsas parameters to control trial counts, enabling improved predictor performance and stability.Changes in
determine_valid_actions_for_stateExpanded state observation vector
Fixes and Enhancements
OptimizeCliffordsby ensuringCollectCliffordsruns beforehand.reward.pyGatesInBasisinrl/predictorenv.pyINDEPintest_predictor_rl.py, since the current action space does not guarantee support for high-level gates.Dependency Update
qiskit-ibm-ai-local-transpilerto the dependenciesnetworkx==2.8.5to ensure compatibility withqiskit-ibm-ai-local-transpilerpytket_qiskit>=0.71.0for compatibility with the new action spaceChecklist: