Skip to content

Conversation

@MatthiasReumann
Copy link
Collaborator

@MatthiasReumann MatthiasReumann commented Oct 24, 2025

Description

This PR aims to improve the total runtime and memory footprint of the A*-based Routing Algorithm.

Changes

  • Remove buckets and ndepth calculation to reduce footprint of Node struct.
  • Precompute neighbors of hardware qubits (QubitIndex: [QubitIndex] map) reduces runtime of neighboursOf from O(n) to O(1)
  • Add ClosedMap: Maps previously visited layouts to their lowest search depth. Essentially: Don't revisit layouts that were discovered with a lower depth.
  • Remove interface methods getAll{In,Out}Qubits in getIns(), getOuts(), and isTwoQubitGate() to avoid creating temporary vectors.
  • Improve, refactor, ParallelOpScheduler.
  • Skip two-qubit blocks in ParallelOpScheduler.
  • Remove msTime statistics: MLIR provides a -mlir-timing CLI flag which measures how long each pass took.

Benchmarks

Options: place{strategy=identity}, route{nlookahead=15 lambda=0.5}

Benchmark runtime (ms, old) #swaps (old) runtime (ms, new) #swaps (new)
0 randomcircuit_5 3.4 26 3.9 30
1 randomcircuit_10 86333.7 336 8.9 295
2 randomcircuit_13 Timeout NA 34.2 580
3 grover_5 7.3 95 2.8 81
4 grover_10 262 7637 95.7 6367
5 grover_13 31944.9 43778 553.6 34596
6 shor_18 1222.6 8955 186.4 7944
7 shor_42 Timeout NA 5831.5 185334

Note

I've removed the QFT benchmarks for now because A* frequently runs into troubles here. There should probably be some fallback mechanism when A* gets stuck. But I will leave that for a future PR.

Checklist:

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • 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).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

Summary by CodeRabbit

  • Chores

    • Removed beta option and cleaned up runtime telemetry for placement and routing passes.
    • Simplified pass option ordering and reduced redundant traversal cases.
  • Refactors

    • Major internal redesigns of routing, scheduling, and architecture neighbor handling to improve performance and maintainability.
    • Updated layout and routing data representations and heuristic plumbing.
  • Tests

    • Expanded routing tests and updated test invocations; removed an obsolete naive test file.
  • Documentation

    • Updated CHANGELOG entry for routing-related PRs.

@MatthiasReumann MatthiasReumann changed the title ♻️ Algorithmic Improvements of A*-search-based Routing ⚡️ Algorithmic Improvements of A*-search-based Routing Oct 24, 2025
@burgholzer
Copy link
Member

This is great to see!
I hope the input qubit getter will be more efficient in the rewrite. This is something we wanted to explicitly consider.

Would you mind adding the note about compiling llvm with statistics tracking to the Issue about providing pre-compiled MLIR binaries. I guess we should always make sure to compile with statistics on.

@codecov
Copy link

codecov bot commented Oct 30, 2025

Codecov Report

❌ Patch coverage is 95.41284% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...r/Dialect/MQTOpt/Transforms/Transpilation/Router.h 89.3% 5 Missing ⚠️
...ialect/MQTOpt/Transforms/Transpilation/Scheduler.h 95.0% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@MatthiasReumann
Copy link
Collaborator Author

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

✅ Actions performed

Full review triggered.

1 similar comment
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

✅ Actions performed

Full review triggered.

@burgholzer
Copy link
Member

@MatthiasReumann I think you have to take the PR out if its draft state and the rabbit will automatically perform a review

@MatthiasReumann MatthiasReumann marked this pull request as ready for review October 30, 2025 14:30
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Reworks transpilation internals: replaces CouplingMap with CouplingSet and adds neighbour caching; redesigns A* router to a Node-based state and removes beta from HeuristicWeights; changes scheduler to a worklist/layered approach and updates related APIs, tests, and pass telemetry (removed beta option and msTime).

Changes

Cohort / File(s) Summary
Common utilities
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h, mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp
Added ValuePair alias; renamed parameter to op in isTwoQubitGate; changed getIns/getOuts to return ValuePair and handle control-qubit cases; added getUserInRegion.
Architecture & neighbours
mlir/include/.../Architecture.h, mlir/lib/.../Architecture.cpp
Replaced CouplingMap with CouplingSet and NeighbourVector; added neighbours_ and collectNeighbours(); neighboursOf now returns a SmallVector with inline capacity 4 and reads precomputed neighbours.
Layout DenseMap support
mlir/include/.../Layout.h
Added llvm::DenseMapInfo<ThinLayout> specialization, included <llvm/ADT/DenseMapInfo.h>, and declared DenseMapInfo as friend for ThinLayout.
Router & heuristics
mlir/include/.../Router.h, mlir/lib/.../sc/RoutingPass.cpp
Renamed result alias to RouterResult; removed beta from HeuristicWeights (constructor/signature change); A* router redesigned to use a Node-based state and ClosedMap; updated route-related types and usages.
Scheduler / layering
mlir/include/.../Scheduler.h
Switched scheduler to worklist/layered algorithm; schedule now takes Layout by value; replaced nlookahead_ with nlayers_; added advanceToTwoQubitGate returning std::optional.
Pass options & telemetry
mlir/include/.../Passes.td, mlir/lib/.../sc/PlacementPass.cpp
Removed beta option and msTime runtime statistics; removed timing instrumentation and reordered TypeSwitch cases to prioritize mqtopt ops.
Tests — modifications & additions
mlir/test/.../basics.mlir, mlir/test/.../grover_5.mlir, mlir/test/.../routing-placement.mlir
basics.mlir: added runs for naive and astar with per-method CHECK prefixes; added new grover_5.mlir test; routing-placement.mlir: changed to use direct --placement-sc option.
Tests — removals
mlir/test/.../naive-routing.mlir (removed)
Deleted the existing naive-routing.mlir test file.
Changelog
CHANGELOG.md
Updated Unreleased entries and PR link references (added/adjusted PR numbers).

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler as ParallelOpScheduler
    participant Worklist as Worklist/Queue
    participant Adv as advanceToTwoQubitGate
    participant Layer as LayerBuilder

    Scheduler->>Worklist: init active qubits
    loop per layer
        Scheduler->>Worklist: pop active qubits
        par find next two-qubit gate for each qubit
            Scheduler->>Adv: advanceToTwoQubitGate(q, region, layout)
            Adv-->>Scheduler: (Value, UnitaryInterface) or std::nullopt
        end
        alt gates found
            Scheduler->>Layer: collect ready gates, remap qubits
            Layer-->>Scheduler: built layer
            Scheduler->>Scheduler: emit layer, advance heads, update layout
        else none found
            Scheduler->>Worklist: mark qubits inactive
        end
    end
Loading
sequenceDiagram
    participant Caller as RoutingPass
    participant Router as AStarHeuristicRouter
    participant Arch as Architecture
    participant Layout as ThinLayout

    Caller->>Router: route(layers, layout, arch)
    Router->>Layout: read layout (getHardwareIndex...)
    Router->>Arch: neighbour queries (neighboursOf)
    Router->>Router: expand Nodes, compute g/h, update ClosedMap
    alt goal reached
        Router-->>Caller: RouterResult (swaps)
    else exhausted
        Router-->>Caller: RouterResult (best effort / empty)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~70 minutes

  • Areas needing extra attention:
    • A* Node redesign, open/closed handling, and correctness of heuristics (Router.h, A* implementation).
    • Two-qubit detection and getIns/getOuts behavior with control qubits (Common.*).
    • Scheduler worklist/layer semantics and advanceToTwoQubitGate correctness (Scheduler.h).
    • Architecture::collectNeighbours() and use of CouplingSet vs. prior CouplingMap (Architecture.*).
    • Consistency of ThinLayout DenseMap specialization and friend access (Layout.h).

Possibly related PRs

Suggested labels

feature, c++, MLIR

Suggested reviewers

  • burgholzer
  • ystade

Poem

🐰 I hopped through maps and cached every lane,
Nodes stacked softly as I pruned beta's chain,
Layers bloom in orderly queues at dawn,
Neighbours precomputed, old detours gone,
Small hops, big leaps — the quantum meadow's drawn ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "⚡️ Algorithmic Improvements of A*-search-based Routing" directly and clearly summarizes the primary change in the PR. The changes across multiple files (Router.h, Architecture.h, Scheduler.h, and related implementations) are all focused on optimizing the A*-based routing algorithm, which is precisely what the title conveys. The title is concise, specific enough for a teammate reviewing the git history to understand the core objective, and avoids vague terminology.
Description Check ✅ Passed The pull request description meets the template requirements with a complete "Description" section explaining the objective (improving runtime and memory footprint of A*-based routing), a detailed "Changes" section itemizing specific modifications, a "Benchmarks" section demonstrating quantifiable improvements, and all required checklist items present. The description is specific and substantive rather than vague, providing clear context and motivation for the changes. However, all checklist items are marked as unchecked, which should be verified and completed before merging to ensure the PR meets quality standards for tests, documentation, changelog entries, and CI validation.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e719864 and 2db8b1d.

📒 Files selected for processing (1)
  • CHANGELOG.md (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (2)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Passes.td (1)

113-124: Complete documentation of breaking changes before merge.

The code changes are correct and properly implemented—no remaining references to beta or msTime exist. However, this PR introduces breaking changes requiring documentation:

  • Changelog: Must document removal of --beta parameter and msTime statistic
  • Upgrade guide: Must guide users to use -mlir-timing instead of the removed msTime metric
  • Documentation: Any user-facing docs referencing beta parameter need updates

These are currently unchecked in the PR checklist and must be completed before merge.

mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Router.h (1)

55-63: Fix return-type mismatch in NaiveRouter::route.

Line 55 declares SmallVector<QubitIndexPair, 16> swaps;, but the function now returns RouterResult (SmallVector<QubitIndexPair>). These template instantiations are distinct types, so return swaps; no longer compiles. Use the aliased type (and reserve inline capacity if needed) so the return statement matches the signature.

Apply this diff:

-    SmallVector<QubitIndexPair, 16> swaps;
+    RouterResult swaps;
+    swaps.reserve(16);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d290966 and 0cfdb7e.

📒 Files selected for processing (11)
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Passes.td (1 hunks)
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h (4 hunks)
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h (2 hunks)
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h (3 hunks)
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Router.h (4 hunks)
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h (3 hunks)
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Architecture.cpp (3 hunks)
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp (1 hunks)
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp (1 hunks)
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingPass.cpp (4 hunks)
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/astar-routing.mlir (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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.
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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.
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
PR: munich-quantum-toolkit/core#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:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/astar-routing.mlir
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp
📚 Learning: 2025-10-09T04:30:29.071Z
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/astar-routing.mlir
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Router.h
📚 Learning: 2025-10-07T15:58:19.247Z
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/astar-routing.mlir
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Router.h
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/astar-routing.mlir
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Router.h
📚 Learning: 2025-10-09T13:28:29.237Z
Learnt from: DRovara
PR: munich-quantum-toolkit/core#1108
File: mlir/lib/Dialect/MQTOpt/Transforms/DeadGateEliminationPattern.cpp:42-45
Timestamp: 2025-10-09T13:28:29.237Z
Learning: In the MQTOpt MLIR dialect, linear types enforce single-use semantics where each qubit value can only be consumed once, preventing duplicate deallocations and making RAUW operations safe when replacing output qubits with corresponding input qubits in transformation patterns.

Applied to files:

  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
PR: munich-quantum-toolkit/core#1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:171-180
Timestamp: 2025-10-09T13:13:51.224Z
Learning: In MQT Core MLIR, UnitaryInterface operations guarantee 1-1 correspondence between input and output qubits in the same order. When cloning or modifying unitary operations (e.g., removing controls), this correspondence is maintained by construction, so yielding getAllInQubits() in else-branches matches the result types from the operation's outputs.

Applied to files:

  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h
  • mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingPass.cpp
  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
PR: munich-quantum-toolkit/core#1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.

Applied to files:

  • mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h
🔇 Additional comments (1)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Passes.td (1)

125-127: msTime removal is correct and complete across the codebase.

Verification confirms:

  • No msTime references exist in any MQTOpt implementation files
  • numSwaps statistic is properly defined in RoutingPassSC and actively used in RoutingPass.cpp (constructor, increment at line 207, member at line 218)
  • PlacementPassSC defines no statistics (as expected)
  • Code is consistent with the .td definitions

The change is sound. Consider updating the project changelog or upgrade guide to document that users should use MLIR's -mlir-timing flag for pass timing measurements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cfdb7e and e719864.

📒 Files selected for processing (4)
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/basics.mlir (8 hunks)
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/grover_5.mlir (1 hunks)
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/naive-routing.mlir (0 hunks)
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/routing-placement.mlir (1 hunks)
💤 Files with no reviewable changes (1)
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/naive-routing.mlir
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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.
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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.
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
PR: munich-quantum-toolkit/core#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:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/grover_5.mlir
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/basics.mlir
  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/routing-placement.mlir
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/grover_5.mlir
📚 Learning: 2025-10-09T13:28:29.237Z
Learnt from: DRovara
PR: munich-quantum-toolkit/core#1108
File: mlir/lib/Dialect/MQTOpt/Transforms/DeadGateEliminationPattern.cpp:42-45
Timestamp: 2025-10-09T13:28:29.237Z
Learning: In the MQTOpt MLIR dialect, linear types enforce single-use semantics where each qubit value can only be consumed once, preventing duplicate deallocations and making RAUW operations safe when replacing output qubits with corresponding input qubits in transformation patterns.

Applied to files:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/grover_5.mlir
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
PR: munich-quantum-toolkit/core#1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:171-180
Timestamp: 2025-10-09T13:13:51.224Z
Learning: In MQT Core MLIR, UnitaryInterface operations guarantee 1-1 correspondence between input and output qubits in the same order. When cloning or modifying unitary operations (e.g., removing controls), this correspondence is maintained by construction, so yielding getAllInQubits() in else-branches matches the result types from the operation's outputs.

Applied to files:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/grover_5.mlir
📚 Learning: 2025-10-07T15:58:19.247Z
Learnt from: MatthiasReumann
PR: munich-quantum-toolkit/core#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:

  • mlir/test/Dialect/MQTOpt/Transforms/Transpilation/basics.mlir
🔇 Additional comments (6)
mlir/test/Dialect/MQTOpt/Transforms/Transpilation/routing-placement.mlir (1)

9-9: Verify whether placement-only validation catches qubit limit errors, or restore routing verification.

The routing and routing verification passes have been removed from this test, reducing coverage. While other files (basics.mlir, grover_5.mlir) contain comprehensive routing tests, they don't test error cases like qubit allocation limits.

Concerns:

  1. Qubit limit errors are typically caught during routing, not placement. Placement maps logical qubits to physical ones; routing validates those mappings against architecture constraints. Removing route-sc and verify-routing-sc may prevent this error from being caught.
  2. Filename mismatch: The file is named routing-placement.mlir but now tests only placement.

Recommendations:

  • Confirm that placement-sc="strategy=identity" alone still catches the "requires one too many qubits" error on line 22. If not, restore the routing pass and verification.
  • If placement alone is sufficient, rename the file to placement-errors.mlir or similar to reflect the actual scope.
mlir/test/Dialect/MQTOpt/Transforms/Transpilation/basics.mlir (3)

11-12: LGTM! Good test coverage for both routing methods.

Adding separate RUN commands for both naive and astar routing methods ensures that the algorithmic improvements work correctly while maintaining backward compatibility with the naive approach.


356-356: Clarify the semantic changes and consistency with entryBranching.

The changes alter the quantum circuit semantics in the entryAll function's branching section:

  • Line 356: ctrlnctrl (negatively-controlled gate)
  • Line 361: controlled-X → swap with result operand reordering

However, the entryBranching function (lines 217-227) still uses ctrl in both branches. Given that the comment on line 241 states entryAll contains "All of the above quantum computations in a single entry point", this inconsistency is potentially confusing.

Could you clarify:

  1. Is entryAll meant to exactly duplicate entryBranching logic, or is it adding coverage for different gate types (nctrl and swap)?
  2. Do both naive and astar routing methods correctly handle these different gate semantics, given both use the same function labels (lines 235-236)?

If the intent is to add test coverage for nctrl and swap gates specifically, consider adding a comment documenting this deviation from the entryBranching pattern.

Also applies to: 361-361


15-16: LGTM! Consistent label structure for dual-method testing.

The addition of both NAIVE-LABEL and ASTAR-LABEL for all test functions correctly enables verification that both routing methods handle the same test cases successfully.

Also applies to: 76-77, 108-109, 145-146, 195-196, 235-236, 372-373

mlir/test/Dialect/MQTOpt/Transforms/Transpilation/grover_5.mlir (2)

666-682: LGTM: Barrier and measurement structure is correct.

The barrier operation correctly groups all final qubits before measurement, and the measurement + storage sequence properly handles all 5 qubits with correct indexing.


11-11: The -verify-diagnostics flag usage is consistent with established test patterns across all routing transpilation tests.

Verification shows that grover_5.mlir contains no diagnostic check directives (expected-error, expected-warning, etc.), which aligns with your observation. However, this same pattern is uniformly used across all similar routing tests in the Transpilation directory (basics.mlir, routing-placement.mlir, routing-verification.mlir). The file's comment explicitly states "the routing verifier pass ensures the validity of this program," indicating this design is intentional. The flag appears to be part of the established test infrastructure conventions for routing validation tests rather than a misuse of the diagnostic verification feature.

Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me 👍🏻
You might want to add this PR number to the Changelog entry for the QMAP addition.
Afterwards, feel free to hit merge here 🎉

@burgholzer burgholzer enabled auto-merge (squash) October 31, 2025 07:25
@burgholzer burgholzer merged commit e9991b0 into main Oct 31, 2025
29 checks passed
@burgholzer burgholzer deleted the enh/mlir-routing-a-star-improvements branch October 31, 2025 07:26
@denialhaag denialhaag added enhancement New feature or request MLIR Anything related to MLIR labels Nov 4, 2025
@coderabbitai coderabbitai bot mentioned this pull request Dec 17, 2025
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request MLIR Anything related to MLIR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants