✨ [NA] Adding evaluation script for zoned neutral atom compiler #875
Conversation
Co-authored-by: Copilot <[email protected]> Signed-off-by: Yannick Stade <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.license-tools-config.json(1 hunks)eval/na/zoned/eval_ids_relaxed_routing.py(1 hunks)eval/na/zoned/mqt.nastyle(1 hunks)eval/na/zoned/square_architecture.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
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.
🪛 Ruff (0.14.8)
eval/na/zoned/eval_ids_relaxed_routing.py
51-51: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (6)
.license-tools-config.json (1)
17-18: LGTM!The addition of
.nastylesuffix mapping toSLASH_STYLEis consistent with the newmqt.nastylefile which uses//style comments for its license header.eval/na/zoned/mqt.nastyle (1)
1-193: LGTM!The visualization style configuration is well-structured with comprehensive styling for all visualization components. The regex patterns for zone name extraction and the color scheme are consistent throughout.
eval/na/zoned/square_architecture.json (1)
1-73: LGTM!The architecture configuration defines a comprehensive zoned square architecture with reasonable physical parameters for neutral atom systems. The zone definitions, SLM configurations, and range specifications are well-structured.
eval/na/zoned/eval_ids_relaxed_routing.py (3)
71-71: Fork context is not portable to Windows.The
"fork"context is Unix-only and will fail on Windows. If cross-platform support is needed, consider using"spawn"(which requires picklable arguments) or add a platform check.If this script is intended to run only on Unix systems, this is acceptable. Otherwise, consider:
- ctx = get_context("fork") # use fork so bound methods don't need to be pickled on macOS/Unix + import sys + ctx = get_context("fork" if sys.platform != "win32" else "spawn")
49-52: Blind exception catch is acceptable here.The static analysis flags this as BLE001, but catching
Exceptionin a multiprocess worker wrapper is a valid pattern—it ensures all exceptions are captured and communicated back through the queue rather than being lost in the subprocess.
101-122: LGTM!The transpilation logic correctly flattens the circuit, transpiles to the native gate set, and strips measurement/barrier operations. Using a fixed seed ensures reproducibility.
## Description ### Summary by CodeRabbit #### New Features - Introduced PlacementMethod configuration enum with astar and ids placement algorithm options - Added placement_method parameter to RoutingAwareCompiler for flexible algorithm selection during compilation #### Tests - Extended test coverage with new placement configuration scenarios and comprehensive test cases ### Walkthrough This PR refactors the quantum placement system by replacing AStarPlacer with HeuristicPlacer, introducing a configurable placement method enum (ASTAR and IDS) exposed to Python, modernizing method signatures to use shared_ptr-based nodes, and updating the placer configuration pipeline to accept placement method parameters in both C++ and Python interfaces. ## Checklist: <!--- This checklist serves as a reminder of a couple of things that ensure your pull request will be merged swiftly. --> - [x] The pull request only contains commits that are focused and relevant to this change. - [x] I have added appropriate tests that cover the new/changed functionality. - [x] I have updated the documentation to reflect these changes. - [x] I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals. - [ ] I have added migration instructions to the upgrade guide (if needed). - [x] The changes follow the project's style guidelines and introduce no new warnings. - [x] The changes are fully tested and pass the CI checks. - [x] I have reviewed my own code changes. --------- Signed-off-by: Yannick Stade <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Haag <[email protected]> # Conflicts: # include/na/zoned/layout_synthesizer/placer/HeuristicPlacer.hpp
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [astral-sh/ruff-pre-commit](https://redirect.github.com/astral-sh/ruff-pre-commit) | repository | patch | `v0.14.8` -> `v0.14.9` | | [astral-sh/uv-pre-commit](https://redirect.github.com/astral-sh/uv-pre-commit) | repository | patch | `0.9.16` -> `0.9.17` | Note: The `pre-commit` manager in Renovate is not supported by the `pre-commit` maintainers or community. Please do not report any problems there, instead [create a Discussion in the Renovate repository](https://redirect.github.com/renovatebot/renovate/discussions/new) if you have any questions. --- ### Release Notes <details> <summary>astral-sh/ruff-pre-commit (astral-sh/ruff-pre-commit)</summary> ### [`v0.14.9`](https://redirect.github.com/astral-sh/ruff-pre-commit/releases/tag/v0.14.9) [Compare Source](https://redirect.github.com/astral-sh/ruff-pre-commit/compare/v0.14.8...v0.14.9) See: <https://github.com/astral-sh/ruff/releases/tag/0.14.9> </details> <details> <summary>astral-sh/uv-pre-commit (astral-sh/uv-pre-commit)</summary> ### [`v0.9.17`](https://redirect.github.com/astral-sh/uv-pre-commit/releases/tag/0.9.17) [Compare Source](https://redirect.github.com/astral-sh/uv-pre-commit/compare/0.9.16...0.9.17) See: <https://github.com/astral-sh/uv/releases/tag/0.9.17> </details> --- ### Configuration 📅 **Schedule**: Branch creation - "every weekend" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://redirect.github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/munich-quantum-toolkit/qmap). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi40Mi4yIiwidXBkYXRlZEluVmVyIjoiNDIuNDIuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIiwicHJlLWNvbW1pdCJdfQ==--> Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.github/workflows/cd.yml (1)
51-55: Confirm Actions Runner version requirement for v7.0.0 upgrade.The v7.0.0 action maintains full compatibility with the
patternandmerge-multipleparameters. However, v7.0.0 requires Actions Runner version 2.327.1 or later as a breaking change. Verify that your runner infrastructure (including any self-hosted runners) meets this version requirement before merging this upgrade.include/na/zoned/layout_synthesizer/placer/HeuristicPlacer.hpp (4)
18-33: Add missing standard headers forstd::isnanand exceptions (don’t rely on transitive includes).
iterativeDivingSearchusesstd::isnanand throwsstd::invalid_argument/std::runtime_error, but this header doesn’t include<cmath>/<stdexcept>explicitly. This can break builds depending on include order.#include <array> #include <cassert> +#include <cmath> #include <cstddef> #include <cstdint> #include <deque> #include <functional> #include <map> #include <memory> #include <optional> #include <queue> #include <spdlog/spdlog.h> +#include <stdexcept> #include <unordered_map> #include <unordered_set> #include <utility> #include <vector>Also applies to: 595-635, 688-695
258-326: Shared-parent nodes: watch memory/atomic-refcount overhead at large search sizes.
Switching nodes tostd::shared_ptr<const Node>withparentpointers is safer, but it can amplify memory retention (every queued node keeps its whole ancestry alive) and adds atomic refcount churn in the hot loop. WithmaxNodesup to 10,000,000, this can get expensive quickly. Consider (if profiling shows it matters) an arena + raw/std::weak_ptrparent, or storing parent as an index/offset into an owned node pool.Also applies to: 736-804
697-735: Update docblocks: they still describe “references”, but APIs now useshared_ptr.
Multiple comments in the A* section andgetNeighborssections still say “references remain valid” / “return references”, but signatures now take/returnstd::shared_ptr<const Node>. This is confusing for users and future maintainers.- * @param getNeighbors is a function that returns the neighbors of a node as - * references + * @param getNeighbors is a function that returns the neighbors of a node as + * shared pointers ... - * @return a vector of node references representing the path from the start to - * a goal + * @return a shared pointer to a goal node (follow `parent` to reconstruct the path)Also applies to: 1020-1063
73-190: Verify JSON config roundtrip forConfig::Method+ factory defaults.The enum serialization is correctly defined via
NLOHMANN_JSON_SERIALIZE_ENUM(lines 1130–1135) mapping "astar"/"ids" strings, andcreateForMethod(Method)correctly sets the method field consistently through its factory methods. However, verify that unknown enum strings are handled safely: nlohmann/json throwsnlohmann::json::exceptionby default for unrecognized enum values, but there are no tests confirming this behavior. Consider adding an integration test that deserializes both valid and invalid enum strings to document the error handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/cd.yml(1 hunks).pre-commit-config.yaml(1 hunks)include/na/zoned/layout_synthesizer/placer/HeuristicPlacer.hpp(18 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
📚 Learning: 2025-12-13T20:08:32.333Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 862
File: pyproject.toml:65-66
Timestamp: 2025-12-13T20:08:32.333Z
Learning: In pyproject.toml, maintain broad compatibility by avoiding unnecessary increases to the minimum Python version. Only raise minimum version when there is a concrete need (e.g., to guarantee binary wheel availability or access to required features). This helps keep the project compatible with the ecosystem and other dependencies across supported Python versions.
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-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-02T10:08:36.022Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: .github/workflows/cd.yml:40-48
Timestamp: 2025-12-02T10:08:36.022Z
Learning: In munich-quantum-toolkit workflows, the reusable-python-packaging-sdist.yml workflow uploads the sdist artifact with the name `cibw-sdist` (or `dev-cibw-sdist` for pull requests), which means it is covered by the `cibw-*` glob pattern when downloading artifacts for deployment.
Applied to files:
.github/workflows/cd.yml
📚 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:
.pre-commit-config.yaml
📚 Learning: 2025-11-20T18:19:31.857Z
Learnt from: lsschmid
Repo: munich-quantum-toolkit/qmap PR: 832
File: src/hybridmap/HybridNeutralAtomMapper.cpp:1679-1715
Timestamp: 2025-11-20T18:19:31.857Z
Learning: In src/hybridmap/HybridNeutralAtomMapper.cpp, the compareSwapAndBridge and compareShuttlingAndFlyingAncilla methods intentionally use division by zero to produce infinity when distance reductions are zero or negative. This infinity value marks that option as highly unfavorable in cost comparisons, which is the intended behavior rather than a bug.
Applied to files:
include/na/zoned/layout_synthesizer/placer/HeuristicPlacer.hpp
🧬 Code graph analysis (1)
include/na/zoned/layout_synthesizer/placer/HeuristicPlacer.hpp (1)
src/na/zoned/layout_synthesizer/placer/HeuristicPlacer.cpp (12)
getNeighbors(1215-1249)getNeighbors(1215-1217)getNeighbors(1251-1294)getNeighbors(1251-1253)getCost(1084-1090)getCost(1084-1084)getCost(1092-1098)getCost(1092-1092)getHeuristic(1127-1170)getHeuristic(1127-1131)getHeuristic(1172-1213)getHeuristic(1172-1176)
⏰ 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). (11)
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🐍 Test (macos-14) / 🐍 macos-14
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🚀 CD (windows-2022) / 🎡 windows-2022
- GitHub Check: 🚀 CD (windows-11-arm) / 🎡 windows-11-arm
- GitHub Check: 🚀 CD (ubuntu-24.04) / 🎡 ubuntu-24.04
- GitHub Check: 🚀 CD (ubuntu-24.04-arm) / 🎡 ubuntu-24.04-arm
- GitHub Check: 🚀 CD (macos-15-intel) / 🎡 macos-15-intel
- GitHub Check: 🇨 Test 🏁 (windows-2022, msvc, Debug) / 🏁 windows-2022 msvc Debug
🔇 Additional comments (2)
.pre-commit-config.yaml (1)
69-69: Clarify if version bumps are necessary for this PR.The pre-commit dependency updates (uv-pre-commit 0.9.16 → 0.9.17 and ruff-pre-commit v0.14.8 → v0.14.9) appear unrelated to the PR's main objective of adding an evaluation script for zoned neutral atom compiler. Patch-level version bumps are generally safe, but please clarify:
- Are these updates necessary for the functional changes introduced by this PR?
- If these are incidental maintenance, consider including them in a separate maintenance PR to keep the commit history focused on the PR's primary objective.
Also applies to: 75-75
include/na/zoned/layout_synthesizer/placer/HeuristicPlacer.hpp (1)
595-695: IDSItemComparesemantics: double-check “smaller priority is better” matches the bounded-queue max-discard logic.
YourBoundedPriorityQueuekeeps the min element attop()and discards the max element on overflow; withItemComparedefined asa.priority < b.priority, that implies lowercost+heuristicsurvives, which seems intended—please sanity-check with a small unit test (overflow + pop/popAndShrink).
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
eval/na/zoned/eval_ids_relaxed_routing.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
eval/na/zoned/eval_ids_relaxed_routing.py (1)
python/mqt/qmap/na/zoned.pyi (10)
PlacementMethod(57-67)RoutingAwareCompiler(146-239)RoutingMethod(69-81)ZonedNeutralAtomArchitecture(15-55)compile(130-138)compile(225-233)stats(139-144)stats(234-239)from_json_file(19-30)to_namachine_file(44-49)
🪛 Ruff (0.14.8)
eval/na/zoned/eval_ids_relaxed_routing.py
56-56: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (8)
eval/na/zoned/eval_ids_relaxed_routing.py (8)
30-38: LGTM!The TYPE_CHECKING block is well-structured with proper use of
TypeVarfor the return typeRandParamSpecfor parameter specificationP.
54-57: Catching bareExceptionis appropriate here.The broad exception catch is intentional for this multiprocessing pattern—any exception raised in the child process needs to be captured and transmitted back to the parent via the queue. The static analysis warning (BLE001) can be safely ignored in this context.
106-127: LGTM!The transpilation logic correctly flattens the circuit, transpiles to the native gate set (
cz,id,u2,u1,u3), and strips measurement/barrier operations. The fixed seed ensures reproducibility.
227-257: LGTM!The initialization correctly delegates to
reset()to avoid code duplication, addressing the previous review feedback.
491-540: LGTM!The
evaluatemethod correctly parses the naviz format, extracts atom locations, and dispatches operations to their respective handlers. The use ofitertools.chainfor iterator restoration andValueErrorfor unrecognized operations addresses previous review feedback.
542-559: LGTM!The CSV header and data output are properly aligned with 14 columns. The format is consistent across all output methods.
658-673: LGTM!The benchmark loop correctly creates the
in/directory, iterates through benchmark configurations, and processes each with all three compilers.
692-694: LGTM!Changing the working directory to the script's location ensures relative file paths work correctly regardless of invocation directory.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
eval/na/zoned/eval_ids_relaxed_routing.py (1)
575-607: Timeout/memout/error CSV rows likely have fewer columns than the header.The header defines 14 columns, but the
print_timeout/print_memout/print_errorlines build status rows with fewer empty fields, so these rows end up with 13 columns. That can break downstream CSV consumers and reintroduces the earlier format mismatch problem.To guarantee alignment and avoid manual comma counting, consider constructing rows as lists:
def print_timeout(self, circuit_name: str, qc: QuantumComputation, setting: str) -> None: @@ - with pathlib.Path(self.filename).open("a", encoding="utf-8") as csv: - csv.write(f"{circuit_name},{qc.num_qubits},{setting},timeout,,,,,,,,,\n") + with pathlib.Path(self.filename).open("a", encoding="utf-8") as csv: + row = [ + circuit_name, + str(qc.num_qubits), + setting, + "timeout", + "", # two_qubit_gates + "", # scheduling_time + "", # reuse_analysis_time + "", # placement_time + "", # routing_time + "", # code_generation_time + "", # total_time + "", # two_qubit_gate_layer + "", # max_two_qubit_gates + "", # rearrangement_duration + ] + csv.write(",".join(row) + "\n") @@ def print_memout(self, circuit_name: str, qc: QuantumComputation, setting: str) -> None: @@ - with pathlib.Path(self.filename).open("a", encoding="utf-8") as csv: - csv.write(f"{circuit_name},{qc.num_qubits},{setting},memout,,,,,,,,,\n") + with pathlib.Path(self.filename).open("a", encoding="utf-8") as csv: + row = [ + circuit_name, + str(qc.num_qubits), + setting, + "memout", + "", "", "", "", "", "", "", "", "", "", + ] + csv.write(",".join(row) + "\n") @@ def print_error(self, circuit_name: str, qc: QuantumComputation, setting: str) -> None: @@ - with pathlib.Path(self.filename).open("a", encoding="utf-8") as csv: - csv.write(f"{circuit_name},{qc.num_qubits},{setting},error,,,,,,,,,\n") + with pathlib.Path(self.filename).open("a", encoding="utf-8") as csv: + row = [ + circuit_name, + str(qc.num_qubits), + setting, + "error", + "", "", "", "", "", "", "", "", "", "", + ] + csv.write(",".join(row) + "\n")This keeps the status rows structurally identical to the data rows and avoids off‑by‑one comma issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
eval/na/zoned/README.md(1 hunks)eval/na/zoned/eval_ids_relaxed_routing.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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:
eval/na/zoned/eval_ids_relaxed_routing.py
🧬 Code graph analysis (1)
eval/na/zoned/eval_ids_relaxed_routing.py (1)
python/mqt/qmap/na/zoned.pyi (10)
PlacementMethod(57-67)RoutingAwareCompiler(146-239)RoutingMethod(69-81)ZonedNeutralAtomArchitecture(15-55)compile(130-138)compile(225-233)stats(139-144)stats(234-239)from_json_file(19-30)to_namachine_file(44-49)
🪛 Ruff (0.14.8)
eval/na/zoned/eval_ids_relaxed_routing.py
65-65: Do not catch blind exception: Exception
(BLE001)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
eval/na/zoned/eval_ids_relaxed_routing.py (5)
68-71: BareExceptioncatch intentionally forwards errors but trips BLE001.This is flagged by static analysis. The catch-all is intentional for forwarding any function error to the parent process via the queue.
Add a noqa comment to document the intent:
try: q.put(("ok", func(*args, **kwargs))) - except Exception as e: + except Exception as e: # noqa: BLE001 - forward all function errors to parent q.put(("err", e))
324-336: Inconsistent use ofassertvsValueErrorfor atom validation.
_process_loadcorrectly usesValueErrorfor validation, but_process_moveusesassertstatements which can be stripped with-O. For consistency and robustness, use explicit exceptions here as well.move_match = re.match(r"\((-?\d+\.\d+), (-?\d+\.\d+)\) (\w+)", next_line_stripped) if move_match: x, y, atom = move_match.groups() - assert atom in self.atom_locations, f"Atom {atom} not found in atom locations" + if atom not in self.atom_locations: + raise ValueError(f"Atom {atom} not found in atom locations") moves.append((atom, (int(float(x)), int(float(y))))) else: # Single atom move match = re.match(r"@\+ move \((-?\d+\.\d+), (-?\d+\.\d+)\) (\w+)", line) if match: x, y, atom = match.groups() - assert atom in self.atom_locations, f"Atom {atom} not found in atom locations" + if atom not in self.atom_locations: + raise ValueError(f"Atom {atom} not found in atom locations") moves.append((atom, (int(float(x)), int(float(y)))))
354-364: Sameassertinconsistency in_process_store.Apply the same pattern as
_process_loadfor consistency.- assert next_line_stripped in self.atom_locations, ( - f"Atom {next_line_stripped} not found in atom locations" - ) + if next_line_stripped not in self.atom_locations: + raise ValueError(f"Atom {next_line_stripped} not found in atom locations") atoms.append(next_line_stripped) else: # Single atom store match = re.match(r"@\+ store (\w+)", line) if match: - assert match.group(1) in self.atom_locations, f"Atom {match.group(1)} not found in atom locations" + if match.group(1) not in self.atom_locations: + raise ValueError(f"Atom {match.group(1)} not found in atom locations") atoms.append(match.group(1))
366-432: Remainingassertstatements in gate processing methods.Lines 373, 392-394, 422-424, and 430 also use
assertfor validation. Consider converting these to explicitValueErrorraises for consistency with_process_load.
615-616: Docstring mentions only "fast relaxed compiler" but script evaluates three compilers.The main function evaluates
astar,ids, andrelaxedcompilers, but the docstring only mentions "fast relaxed compiler."def main() -> None: - """Main function for evaluating the fast relaxed compiler.""" + """Main function for evaluating astar, IDS, and relaxed routing compilers."""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
eval/na/zoned/README.md(1 hunks)eval/na/zoned/eval_ids_relaxed_routing.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
📚 Learning: 2025-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:
eval/na/zoned/eval_ids_relaxed_routing.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:
eval/na/zoned/eval_ids_relaxed_routing.py
🧬 Code graph analysis (1)
eval/na/zoned/eval_ids_relaxed_routing.py (6)
noxfile.py (1)
qiskit(137-145)python/mqt/qmap/na/zoned.pyi (8)
PlacementMethod(57-67)RoutingAwareCompiler(146-239)RoutingMethod(69-81)ZonedNeutralAtomArchitecture(15-55)compile(130-138)compile(225-233)stats(139-144)stats(234-239)include/logicblocks/Z3Logic.hpp (1)
ctx(47-47)include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (1)
start(239-241)include/hybridmap/MoveToAodConverter.hpp (1)
get(320-320)python/mqt/qmap/clifford_synthesis.pyi (1)
two_qubit_gates(72-72)
🪛 Ruff (0.14.8)
eval/na/zoned/eval_ids_relaxed_routing.py
70-70: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (10)
eval/na/zoned/eval_ids_relaxed_routing.py (9)
1-17: LGTM on script metadata and dependency pinning.The inline script dependencies with version pinning and
exclude-newerconstraint ensure reproducible evaluation runs.
74-118: LGTM on timeout handling with process isolation.The graceful termination with fallback to kill ensures no zombie processes. The memory exhaustion detection via string matching is pragmatic for the internal compiler error.
121-155: LGTM on benchmark transpilation pipeline.The circuit flattening, transpilation to native gate set, and stripping of measurement/barriers is appropriate for the evaluation workload.
171-220: LGTM on benchmark processing with comprehensive error handling.Exception handling correctly logs timeout, memory, and runtime errors to CSV. Output directory creation with
parents=Truehandles nested paths correctly.
245-276: LGTM on Evaluator initialization.The
reset()call from__init__eliminates duplication, and initializingmax_two_qubit_gatesto0ensures clean CSV output.
442-470: LGTM on movement timing model.The physics constants are well-documented with units (µm, µs), and the cubic/linear timing profile for atom movement is clearly implemented.
511-560: LGTM on evaluate method.The iterator reconstruction with
chainis efficient, and unrecognized operations now raise descriptiveValueErrorexceptions.
562-612: LGTM on CSV output methods.The column order and count are now consistent across
print_data,print_timeout,print_memout, andprint_errormethods.
667-696: LGTM on benchmark configuration and execution.The benchmark selection covers a good range of circuit types and qubit counts. The final summary message clearly documents output locations.
eval/na/zoned/README.md (1)
20-24: LGTM on output documentation.The output section accurately describes the three output locations:
results.csv,in/directory, andout/directory structure.
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Nice. Two minor suggestions. Then let's merge this.
Co-authored-by: Lukas Burgholzer <[email protected]> Signed-off-by: Yannick Stade <[email protected]>
Description
The changes introduce a comprehensive evaluation framework for zoned neutral atom compiler routing performance. A new Python evaluation script implements benchmarking across three routing compilers with timeout. Configuration files establish visualization styling and a zoned square architecture specification. The license configuration is updated to recognize the new style format.
New Features
Chores
Checklist: