✨ [NA] Add New Relaxed Router #859
Conversation
Co-authored-by: Copilot <[email protected]> Signed-off-by: Yannick Stade <[email protected]>
ystade
added
c++
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp (1)
409-410: Misleading variable name: "AscByDist" but uses descending order.The variable
atomsToMoveOrderedAscByDistusesstd::greater<>which produces descending order (highest distance first), but the name suggests ascending order. The comment at lines 425–426 correctly states "ordered decreasingly," confirming the intent.Consider renaming for consistency with the actual ordering:
- std::set<std::pair<double, qc::Qubit>, std::greater<>> - atomsToMoveOrderedAscByDist; + std::set<std::pair<double, qc::Qubit>, std::greater<>> + atomsToMoveOrderedDescByDist;Also applies to: 435-436
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (2)
39-67: DefaultConfig::method = RELAXEDplus_WITH_DEFAULTJSON may silently change behaviorWith
Method method = Method::RELAXED;andNLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(Config, method, preferSplit), any default-constructedConfigor JSON that omitsmethodwill now use relaxed routing. If the pre-PR behavior ofIndependentSetRouterwas effectively “strict routing”, this is a silent behavior change for existing call sites/configs that do not explicitly setmethod.If strict routing is meant to remain the default, consider:
- Method method = Method::RELAXED; + // Default to strict routing for backward compatibility; RELAXED must be + // opted into explicitly. + Method method = Method::STRICT;If RELAXED-by-default is intentional, I’d suggest documenting that explicitly in the
Configcomment (and in any user-facing JSON examples) so users understand that omittingmethodenables relaxed routing.Also applies to: 328-333
252-326: TightenMovementCompatibility/isRelaxedCompatibleMovementdocs and avoid C++20-only designated initializersTwo small points here:
- The doc for
isRelaxedCompatibleMovementstill starts with “Check whether two movements are incompatible…”, but the function returns a fullMovementCompatibilitystatus (StrictlyCompatible,RelaxedCompatible,Incompatible). This can be misleading. Consider rewording to describe checking compatibility under relaxed constraints and returning one of the three statuses:- /** - * Check whether two movements are incompatible with respect to the relaxed - * routing constraints, i.e., moved atoms remain not on the same row (column). - * This is, however, independent of their topological order (i.e., relaxed). + /** + * Check the compatibility of two movements under the relaxed routing + * constraints. The movements may change their topological order via offsets, + * but must still satisfy the relaxed row/column conditions. @@ - * @returns a @ref MovementCompatibility object indicating whether they are - * strictly compatible, relaxed compatible (with merge cost), or incompatible. + * @returns a @ref MovementCompatibility indicating whether they are strictly + * compatible, relaxed compatible (with merge cost), or incompatible. */This is essentially revisiting the earlier doc-mismatch feedback in light of the current wording.
- The factory methods use C++20 designated initializers:
return {.status = Status::StrictlyCompatible, .mergeCost = std::nullopt};If the project is still built with C++17, this will not compile. A more portable implementation is trivial and avoids any dependency on designated initializers:
- [[nodiscard]] static auto strictlyCompatible() -> MovementCompatibility { - return {.status = Status::StrictlyCompatible, .mergeCost = std::nullopt}; - } + [[nodiscard]] static auto strictlyCompatible() -> MovementCompatibility { + MovementCompatibility mc{}; + mc.status = Status::StrictlyCompatible; + mc.mergeCost = std::nullopt; + return mc; + } @@ - [[nodiscard]] static auto incompatible() -> MovementCompatibility { - return {.status = Status::Incompatible, .mergeCost = std::nullopt}; - } + [[nodiscard]] static auto incompatible() -> MovementCompatibility { + MovementCompatibility mc{}; + mc.status = Status::Incompatible; + mc.mergeCost = std::nullopt; + return mc; + } @@ - [[nodiscard]] static auto relaxedCompatible(const double cost) - -> MovementCompatibility { - return {.status = Status::RelaxedCompatible, .mergeCost = cost}; - } + [[nodiscard]] static auto relaxedCompatible(const double cost) + -> MovementCompatibility { + MovementCompatibility mc{}; + mc.status = Status::RelaxedCompatible; + mc.mergeCost = cost; + return mc; + }Optionally, you could also
assert(cost >= 0.0);insiderelaxedCompatibleto enforce the non-negativity invariant described in the comment.#!/bin/bash # Verify which C++ standard the project is configured to use. set -euo pipefail # Search all CMakeLists.txt for CMAKE_CXX_STANDARD settings. fd -a CMakeLists.txt | xargs -r rg -n "CMAKE_CXX_STANDARD" -S || true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp(4 hunks)src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-27T08:52:06.428Z
Learnt from: lsschmid
Repo: munich-quantum-toolkit/qmap PR: 832
File: src/hybridmap/HybridNeutralAtomMapper.cpp:1379-1383
Timestamp: 2025-11-27T08:52:06.428Z
Learning: In src/hybridmap/HybridNeutralAtomMapper.cpp, the getBestMovePos function throws std::runtime_error when no move position is found (finalBestPos.coords is empty). This behavior is acceptable for current use cases since tight architectures are not a concern. A future improvement would be to fall back to SWAP-based strategies instead of throwing.
Applied to files:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hppsrc/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hppsrc/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
🔇 Additional comments (10)
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp (6)
119-131: LGTM! Mathematical helper functions are correct.The cube root identities are correctly implemented:
(x+y)³ = a + b + 3xy(x+y)wherex=∛a, y=∛b(x-y)³ = a - b + 3xy(y-x)wherex=∛a, y=∛b
std::cbrtcorrectly handles negative inputs, which may occur in intermediate calculations.
142-145: LGTM! Safe unsigned difference computation.The
distDoublelambda correctly computes absolute difference without signed overflow by usinga > b ? a - b : b - ain the unsigned domain before converting todouble. This addresses the previous review concern about potential integer overflow.
161-171: LGTM! Clean dispatch pattern.The routing method selection based on
config_.methodprovides clear separation between strict and relaxed routing paths.
208-218: LGTM! Helper extraction improves clarity.The
mergeConflictCosthelper cleanly encapsulates the conflict cost merging logic as suggested in past reviews. The semantics are clear: take max when both have values, propagate nullopt (strict conflict) otherwise.
369-378: LGTM! Well-documented reverse iterator erasure.The comment clearly explains the subtle reverse iterator manipulation for in-place erasure. The pattern is correct: incrementing the reverse iterator moves it towards the list front,
.base()yields the forward iterator to the original element, andmake_reverse_iterator(b)properly re-establishes iteration position after erasure.
384-405: LGTM! Clean relaxed routing implementation.The function properly composes the helper methods: single-pass conflict graph creation, initial group formation, and cost-based merging. The
reservecall at line 399 is a good optimization before populating the routing vector.include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (4)
19-27: Header is now self-contained with explicit standard-library includesThe additions of
<cstdint>,<list>,<nlohmann/json.hpp>,<optional>, and<utility>correctly coveruint8_t,std::list, JSON macros,std::optional, andstd::pair, so this header no longer relies on transitive includes. Looks good.
31-34: Class-level description clearly states independent-set routingThe updated docstring concisely explains that routing is based on forming movement groups via a maximal independent set, which matches the class name and intent. No changes needed.
108-178: Routing helper API (GroupInfo, strict/relaxed flows) is well factoredThe decomposition into
GroupInfo,makeStrictRoutingForRelaxedRouting,mergeGroups,routeStrict,routeRelaxed, and thegetAtomsToMove*helpers yields a clear separation between strict routing, relaxed merging, and data extraction. Parameter constness and ownership (e.g., passingatomsToMoveby value where reordering is likely) look appropriate.
239-250:isCompatibleMovementdocumentation now aligns with theboolreturnThe function is documented as a strict-compatibility check and the comment now explicitly states the
true/falsemeaning, which matches the signature. No further changes needed.
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
This looks extremely clean.
I only have a typo, a rewording suggestion, and a potential small code duplication and that's it. 😌
Feel free to directly merge once resolved.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/mqt/qmap/na/zoned.pyi (1)
74-85: API changes need clarification and documentation refinement; parking_offset claim cannot be verifiedThe
.pyistub signatures forRoutingAgnosticCompiler.__init__andRoutingAwareCompiler.__init__correctly match the pybind11 bindings inzoned.cpp—parameter names, order, types, and defaults are consistent. Both now includerouting_method: RoutingMethodandprefer_split: floatparameters.However, the original review comment claims that these parameters replace a removed
parking_offsetparameter. No evidence ofparking_offsetexists in the current codebase, and the repository history does not extend back far enough to verify this claim. Without access to prior versions, this specific API break assertion cannot be confirmed.On documentation: the docstrings for both compilers mention
routing_methodandprefer_splitbut lack crucial implementation details:
routing_method: Does not specify which value is the default (strict or relaxed).prefer_split: Does not indicate the expected numeric range (e.g., 0.0–1.0) or semantics (e.g., "larger values favor splitting more aggressively").Adding these specifics would align the stubs with best practices for typed Python APIs and help users understand the intended behavior without consulting C++ source code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
python/mqt/qmap/na/zoned.pyi(7 hunks)
⏰ 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). (5)
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🇨 Test 🏁 (windows-2022, msvc, Debug) / 🏁 windows-2022 msvc Debug
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (1)
39-67: Clarify default routing method in docs and reconfirm RELAXED‑by‑default behavior
Config::Methodcurrently defaults toMethod::RELAXED, and JSON deserialization viaNLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULTwill also treat a missingmethodfield as RELAXED. If older code assumed strict routing as the implicit default, this is a behavioral change.Two suggestions:
- Explicitly state in the
Configdocs that the default routing method is RELAXED (including for JSON configs).- Reconfirm that all call sites and configs expecting “old” strict behavior now set
method = Method::STRICTexplicitly.A minimal doc tweak could be:
struct Config { /// The routing method. @@ - Method method = Method::RELAXED; + /// Defaults to `Method::RELAXED` (also when omitted in JSON + /// configuration). + Method method = Method::RELAXED;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
Learning: In the munich-quantum-toolkit/core repository, duplication of QDMI-related macros (such as IS_INVALID_ARGUMENT) across device implementations (e.g., in src/qdmi/sc/Device.cpp and src/qdmi/dd/Device.cpp) is acceptable as a temporary measure. The preferred long-term solution is to upstream these macros to the QDMI repository rather than creating local shared headers, so they can be reused across all dependent projects.
Applied to files:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
📚 Learning: 2025-11-27T08:52:06.428Z
Learnt from: lsschmid
Repo: munich-quantum-toolkit/qmap PR: 832
File: src/hybridmap/HybridNeutralAtomMapper.cpp:1379-1383
Timestamp: 2025-11-27T08:52:06.428Z
Learning: In src/hybridmap/HybridNeutralAtomMapper.cpp, the getBestMovePos function throws std::runtime_error when no move position is found (finalBestPos.coords is empty). This behavior is acceptable for current use cases since tight architectures are not a concern. A future improvement would be to fall back to SWAP-based strategies instead of throwing.
Applied to files:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
📚 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:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (1)
197-225: Document both elements of the returned pair increateRelaxedAndStrictConflictGraph.The function returns a
std::paircontaining two conflict graphs, but the@returndocumentation (lines 213-215) only describes the relaxed conflict graph (the second element). The first element—the strict conflict graph—is not documented in the return section.Update the documentation to describe both elements:
- /** - * Creates the relaxed and strict conflict graph. + /** + * Creates both the strict and relaxed conflict graphs. * @details Atom/qubit indices are the nodes. Two nodes are connected if their * corresponding move with respect to the given @p start- and @p * targetPlacement stands in conflict with each other based on the strict * routing constraints. The graph is represented as adjacency lists. * @par * * In contrast to the strict conflict graph, all edges that do not represent * a conflict with respect to the relaxed routing constraints carry a weight * representing the cost for merging the two adjacent movements. These edges * correspond to the value `RelaxedCompatible` returned by @ref * isRelaxedCompatibleMovement. The weight of other edges, i.e., * `Incompatible`, is `std::nullopt`. * @param atomsToMove are all atoms corresponding to nodes in the graph. * @param startPlacement is the start placement of all atoms as a mapping from * atoms to their sites. * @param targetPlacement is the target placement of the atoms. - * @return the relaxed conflict graph as an unordered_map where the keys are - * the nodes and the values are vectors of (neighbor, optional merge cost) - * pairs. + * @return a pair consisting of: + * 1. the strict conflict graph as an unordered_map where the keys are nodes + * and the values are vectors of their neighbors, and + * 2. the relaxed conflict graph as an unordered_map where the keys are nodes + * and the values are vectors of (neighbor, optional merge cost) pairs. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
Learning: In the munich-quantum-toolkit/core repository, duplication of QDMI-related macros (such as IS_INVALID_ARGUMENT) across device implementations (e.g., in src/qdmi/sc/Device.cpp and src/qdmi/dd/Device.cpp) is acceptable as a temporary measure. The preferred long-term solution is to upstream these macros to the QDMI repository rather than creating local shared headers, so they can be reused across all dependent projects.
Applied to files:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
📚 Learning: 2025-11-27T08:52:06.428Z
Learnt from: lsschmid
Repo: munich-quantum-toolkit/qmap PR: 832
File: src/hybridmap/HybridNeutralAtomMapper.cpp:1379-1383
Timestamp: 2025-11-27T08:52:06.428Z
Learning: In src/hybridmap/HybridNeutralAtomMapper.cpp, the getBestMovePos function throws std::runtime_error when no move position is found (finalBestPos.coords is empty). This behavior is acceptable for current use cases since tight architectures are not a concern. A future improvement would be to fall back to SWAP-based strategies instead of throwing.
Applied to files:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
📚 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:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
🧬 Code graph analysis (1)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (1)
include/na/zoned/layout_synthesizer/router/RouterBase.hpp (1)
placement(33-34)
ystade
force-pushed
the
ystade/relaxed-router
branch
from
1d06611 to
0b02d53
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (1)
318-327: Documentation wording conflicts with function name.The docstring says "Check whether two movements are incompatible..." but the function is named
isRelaxedCompatibleMovement, which suggests it checks compatibility. As suggested by @burgholzer in a previous comment, consider updating to align with the function name:- * Check whether two movements are incompatible with respect to the relaxed - * routing constraints, i.e., moved atoms remain not on the same row (column). + * Check whether two movements are compatible with respect to the relaxed + * routing constraints, i.e., moved atoms remain on the same row (column).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(2 hunks)include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp(4 hunks)src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp(5 hunks)
🧰 Additional context used
🧠 Learnings (13)
📚 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-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
Learning: In the munich-quantum-toolkit/core repository, duplication of QDMI-related macros (such as IS_INVALID_ARGUMENT) across device implementations (e.g., in src/qdmi/sc/Device.cpp and src/qdmi/dd/Device.cpp) is acceptable as a temporary measure. The preferred long-term solution is to upstream these macros to the QDMI repository rather than creating local shared headers, so they can be reused across all dependent projects.
Applied to files:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp
📚 Learning: 2025-11-27T08:52:06.428Z
Learnt from: lsschmid
Repo: munich-quantum-toolkit/qmap PR: 832
File: src/hybridmap/HybridNeutralAtomMapper.cpp:1379-1383
Timestamp: 2025-11-27T08:52:06.428Z
Learning: In src/hybridmap/HybridNeutralAtomMapper.cpp, the getBestMovePos function throws std::runtime_error when no move position is found (finalBestPos.coords is empty). This behavior is acceptable for current use cases since tight architectures are not a concern. A future improvement would be to fall back to SWAP-based strategies instead of throwing.
Applied to files:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hppsrc/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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:
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hppsrc/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 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/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 Learning: 2025-10-07T15:58:19.247Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h:202-214
Timestamp: 2025-10-07T15:58:19.247Z
Learning: In the QMAP routing algorithm (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h), the heuristic function h() intentionally uses a non-admissible heuristic that sums per-gate shortest-path distances. This is a deliberate design choice from the QMAP paper, where admissibility is dropped because locally optimal solutions can negatively affect the overall routing quality. This approach prioritizes global solution quality over guaranteed local optimality.
Applied to files:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 Learning: 2025-11-20T16:57:24.305Z
Learnt from: lsschmid
Repo: munich-quantum-toolkit/qmap PR: 832
File: test/hybridmap/test_hybridmap.cpp:200-287
Timestamp: 2025-11-20T16:57:24.305Z
Learning: In test/hybridmap/test_hybridmap.cpp, the LongShuttling test is designed as a stress test to catch rare edge cases that only occur after extended shuttling sequences, which is why it maps a long circuit without explicit assertions beyond ensuring no exceptions are thrown.
Applied to files:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 Learning: 2025-10-09T04:30:29.071Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h:72-140
Timestamp: 2025-10-09T04:30:29.071Z
Learning: In the CrawlScheduler (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h), the persistent `layoutCopy` is updated across all lookahead layers via `remapQubitValue` calls. After each two-qubit gate is scheduled in a layer, its input qubits are remapped to output qubits, ensuring the next layer's crawl starts from those outputs. This progressive remapping through SSA values prevents the same operation from being scheduled in multiple lookahead layers.
Applied to files:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 Learning: 2025-12-10T02:20:01.189Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:20:01.189Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), the intentional design prevents duplicate measurements: the early return when `registerResultMap.find(key)` succeeds avoids generating multiple `mz` calls to the same classical bit index and ensures output recording contains only one entry per index. This implements an "override" semantic where repeated measurements to the same resultIndex reuse the cached pointer without additional side effects.
Applied to files:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 Learning: 2025-12-02T07:37:46.860Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Applied to files:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
📚 Learning: 2025-12-08T23:41:55.972Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:96-117
Timestamp: 2025-12-08T23:41:55.972Z
Learning: In the QIR (Quantum Intermediate Representation) Builder (mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp), the `ptrCache` is intentionally shared between qubit and result pointer creation (in `staticQubit()` and `measure()` methods) because QIR uses opaque pointers and `inttoptr` conversions for both qubits and results. For any given index N, the LLVM IR pointer representation is identical whether it represents a qubit or a result, so the pointer only needs to be created once and can be safely reused across both contexts.
Applied to files:
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
🧬 Code graph analysis (1)
include/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (1)
include/na/zoned/layout_synthesizer/router/RouterBase.hpp (1)
placement(33-34)
🪛 Cppcheck (2.18.0)
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[style] 70-70: The function 'isStorage' is never used.
(unusedFunction)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[style] 163-163: The function 'route' is never used.
(unusedFunction)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[style] 214-214: The function 'fromJSONFile' is never used.
(unusedFunction)
[style] 229-229: The function 'fromJSONString' is never used.
(unusedFunction)
🔇 Additional comments (12)
CHANGELOG.md (1)
14-17: LGTM! Changelog entry correctly placed and formatted.The entry is properly positioned in the
Unreleasedsection under### Added, which is appropriate for a new feature. The format follows the project's changelog template.src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp (6)
119-134: Well-designed helper functions for cost calculations.The
sumCubeRootsCubedandsubCubeRootsCubedhelpers use algebraic identities to avoid repeated cube root computations, improving both numerical stability and performance. The inline comments documenting the identities are helpful.
144-147: Safe distance calculation addresses potential overflow.The
distDoublelambda correctly computes the absolute difference of unsigned values without risk of underflow, addressing the previous review feedback about integer overflow in coordinate arithmetic.
163-173: Clean routing dispatch based on configuration.The method selection pattern is clear and maintainable. Early return for empty placement avoids unnecessary processing.
174-209: LGTM! Strict routing implementation is correct.The greedy independent set algorithm correctly builds groups of compatible movements by tracking conflicting atoms and iteratively forming maximal independent sets.
221-266: LGTM! Strict routing for relaxed routing preparation is well-structured.The function properly builds
GroupInfostructures with distance tracking and relaxed conflict information needed for subsequent group merging decisions.
267-385: LGTM! Group merging algorithm is correct and well-documented.The cost-based merging logic correctly implements the algorithm described in the documentation:
- Reverse iteration allows safe in-place group erasure
- The cost calculation identity at lines 338-339 is mathematically correct
- The
preferSplitthreshold provides tunable control over merging behaviorinclude/na/zoned/layout_synthesizer/router/IndependentSetRouter.hpp (5)
18-27: LGTM! Required headers are now explicitly included.The header is now self-contained with explicit includes for
<cstdint>,<optional>, and<utility>as required for the public API types (uint8_t,std::optional,std::pair).
39-67: LGTM! Configuration struct is well-designed.The
Configstruct provides clear configuration options with appropriate defaults and documentation. ThepreferSplitparameter's role and its relevance only toRELAXEDmode is clearly explained.
107-116: LGTM! GroupInfo structure supports the merging algorithm well.The struct captures all necessary metadata for the cost-based group merging decisions during relaxed routing.
261-317: LGTM! MovementCompatibility struct has safe defaults and clear semantics.The struct provides safe default values (
Status::Incompatible,std::nullopt) and well-documented factory methods. The cost calculation semantics are thoroughly explained.
333-338: LGTM! JSON enum serialization is correctly configured.The enum serialization follows the standard nlohmann pattern with conventional lowercase string representations.
src/na/zoned/layout_synthesizer/router/IndependentSetRouter.cpp
Outdated
Show resolved
Hide resolved
Description
This Pull Request adds a new RelaxedIndependentSetRouter and implements corresponding functionality to support relaxed routing within the layout synthesizer for the zoned neutral atom compiler. It includes updates to related classes and introduces new tests for the added functionality.
Main Changes
New Router Class:
Implements a router focusing on relaxed independent sets for routing optimization.
Compiler Updates:
Test Suite Enhancements:
Checklist: