Skip to content

✨ Extend Layout API for partial injections - #1956

Merged
burgholzer merged 4 commits into
munich-quantum-toolkit:mainfrom
rturrado:1867
Aug 31, 2026
Merged

✨ Extend Layout API for partial injections#1956
burgholzer merged 4 commits into
munich-quantum-toolkit:mainfrom
rturrado:1867

Conversation

@rturrado

@rturrado rturrado commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Description

Extend Layout to support M <= N program-to-hardware mappings where N - M hardware slots stay unmapped:

  • Unmapped hardware entries in hardwareToProgram_ carry an UNMAPPED sentinel.
  • Split nqubits() into nProgramQubits() and nHardwareQubits(). random() takes both sizes.
  • Add hasProgramAt(hw) to distinguish mapped from unmapped hardware slots.

A possible follow-up could exercise nProg < nHw and the new query API from inside the mapping pass.

Part of #1867

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.

If PR contains AI-assisted content:

  • Any agent that created, edited, or submitted GitHub content was explicitly authorized for that scope, as required by our AI Usage Guidelines.
  • Every agent-authored or agent-edited public text body begins with the visible disclosure 🤖 *AI text below* 🤖 (titles are exempt).
  • AI-assisted commits include an Assisted-by: [Model Name] via [Tool Name] footer.
  • I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

Assisted-by: Claude Opus 4.7 via Claude Code

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@burgholzer

Copy link
Copy Markdown
Member

@rturrado Thanks for another PR! 🙌

@MatthiasReumann care to take a first look here since this is closest to the stuff you have been working on? 😌

Comment thread mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp Outdated

@MatthiasReumann MatthiasReumann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@rturrado Thanks for the effort 🚀 Really appreciate it!

I've left one comment regarding the strategies for mapping SCF which probably shouldn't be reverted to "restore-all". The essential idea is that the SCF operations (besides scf.for) act like permutation networks changing the layout (which needs to be propagated to the parent). If you have any questions, feel free to reach out!

@rturrado

Copy link
Copy Markdown
Contributor Author

@rturrado Thanks for another PR! 🙌

Thanks! This one touches a really beautiful area.

@MatthiasReumann care to take a first look here since this is closest to the stuff you have been working on? 😌

But I have to admit that it is also more complicated code, so human review here is essential.

@rturrado

Copy link
Copy Markdown
Contributor Author

@rturrado Thanks for the effort 🚀 Really appreciate it!

Many thanks!

I've left one comment regarding the strategies for mapping SCF which probably shouldn't be reverted to "restore-all". The essential idea is that the SCF operations (besides scf.for) act like permutation networks changing the layout (which needs to be propagated to the parent). If you have any questions, feel free to reach out!

Perfect, thanks! Yes, this was my main concern about this PR. That the new wire-index-equals-hardware-index invariant always has to restore to the parent layout and never converges. That which needs to be propagated to the parent constraint is key here.

@MatthiasReumann

Copy link
Copy Markdown
Collaborator

@rturrado

That the new wire-index-equals-hardware-index invariant always has to restore to the parent layout and never converges.

I think the "wire-index-equals-hardware-index" is a pretty neat idea, which eventually I would have also looked into. In a previous version of the mapping pass (before SCF mapping), we implemented "wire-index-equals-program-index" which worked pretty nicely.

Nonetheless, I think it would make sense to split this PR into two:

  • The layout improvements + everything else regarding getComputation, etc.
  • The "wire-index-equals-hardware-index" logic.

Especially since it's very likely that #1951 is merged before this one.

@rturrado

Copy link
Copy Markdown
Contributor Author

🤖 AI text below 🤖

@MatthiasReumann Agreed on the split. Rough sketch:

PR 1 (safe): Layout API additions (nProgramQubits, nHardwareQubits, hasProgramAt, swap with an empty side), the random(nProg, nHw, seed) variant that only places nProg real programs (still with placeholder fill), and any getComputation cleanup that does not depend on the invariant. Keeps WireInfos and the current dispatch (converge for qco.if, restore-yielding-only for scf.while, and vote-and-restore for qco.index_switch from #1951).

PR 2 (invariant): wire index == hardware index everywhere, sentinel WireIterator, delete WireInfos, random drops the placeholder fill, rewrite dispatch for the invariant.

Assisted-by: Claude Opus 4.7 via Claude Code

@rturrado rturrado changed the title ♻️ Layout: non-consecutive qubit indices and injective mapping ♻️ ♻️ Layout: DenseMap storage and partial-injection API Jul 28, 2026
@rturrado rturrado changed the title ♻️ ♻️ Layout: DenseMap storage and partial-injection API ♻️ Layout: DenseMap storage and partial-injection API Jul 28, 2026
@burgholzer

Copy link
Copy Markdown
Member

I think we'll try to get #1951 in asap (either tonight or tomorrow). Then PR1 can go on top 😌

@rturrado

Copy link
Copy Markdown
Contributor Author

I think we'll try to get #1951 in asap (either tonight or tomorrow). Then PR1 can go on top 😌

Perfect. Don't rush it. I'll rebase when you're done with #1951.

@mergify mergify Bot added the conflict label Aug 3, 2026
@rturrado
rturrado force-pushed the 1867 branch 3 times, most recently from 25f536f to b036d42 Compare August 7, 2026 12:00
@mergify mergify Bot removed the conflict label Aug 7, 2026

@MatthiasReumann MatthiasReumann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great work. Looking forward for merging this 🚀

Some notes / ideas:
We can now reasonably argue that the program and hardware qubits are a consecutive range of numbers (#1993). That is, a Layout holds [M] → [N], where M <= N. Maybe this allows us to avoid DenseMaps and use SmallVectors again. The Layout is an essential data structure in the mapping pass, so we need to make sure the implementation is very efficient.

Moreover, if we now already invest the time to improve this data structure, it could also make sense to add a generalized InjectiveMap<K, V>, where the Layout would then be a subclass of that (If that is even necessary). Using templates, we may need to keep DenseMap; or specialize on integer types using SmallVector. Just an idea - maybe a follow-up!

Comment thread mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp Outdated
Comment thread mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp Outdated
Comment thread mlir/include/mlir/Dialect/QCO/Utils/Layout.h Outdated
Comment thread mlir/lib/Dialect/QCO/Utils/Layout.cpp
@mergify mergify Bot added the conflict label Aug 10, 2026
@rturrado

Copy link
Copy Markdown
Contributor Author

@MatthiasReumann Many thanks for the thorough review and the guidance. This PR was just the rebase onto main and doesn't yet take into account the new invariant from #1993: that program and hardware qubits are now consecutive range of numbers. I'll update the PR accordingly.

@rturrado rturrado changed the title ♻️ Layout: DenseMap storage and partial-injection API ✨ Extend Layout API for partial injections Aug 11, 2026
@mergify mergify Bot added conflict and removed conflict labels Aug 11, 2026
@burgholzer burgholzer added this to the MLIR Support milestone Aug 13, 2026
@burgholzer burgholzer added enhancement Improvement of existing feature MLIR Anything related to MLIR labels Aug 13, 2026
@burgholzer

Copy link
Copy Markdown
Member

@MatthiasReumann @rturrado Except for the minor conflict with main, what is the status here? This looks like it would be pretty close to ready, right?

@rturrado

Copy link
Copy Markdown
Contributor Author

@MatthiasReumann @rturrado Except for the minor conflict with main, what is the status here? This looks like it would be pretty close to ready, right?

@burgholzer Thanks for flagging this. I'll have a look into that conflict!

Extend `Layout` to support `M <= N` program-to-hardware mappings where `N - M` hardware slots stay unmapped:
- Unmapped hardware entries in `hardwareToProgram_` carry an `UNMAPPED` sentinel.
- Split `nqubits()` into `nProgramQubits()` and `nHardwareQubits()`. `random()` takes both sizes.
- Add `hasProgramAt(hw)` to distinguish mapped from unmapped hardware slots.

Adapt `Mapping.cpp`:
- rename `nqubits()` call sites to `nHardwareQubits()`,
- pass both sizes to `Layout::random()`.

Assisted-by: Claude Opus 4.7 via Claude Code
Signed-off-by: rturrado <rturrado@gmail.com>

@MatthiasReumann MatthiasReumann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your continued work on this! @rturrado Much appreciated🙏

The pull request looks really good already! I've only left some very nitpicky comments, which should be fairly easy to resolve. Lastly, I think a changelog entry is missing also. Otherwise, let's get this PR merged ASAP 🛫

Comment thread mlir/include/mlir/Dialect/QCO/Utils/Layout.h Outdated
Comment thread mlir/unittests/Dialect/QCO/Utils/test_layout.cpp Outdated
Comment thread mlir/unittests/Dialect/QCO/Utils/test_layout.cpp Outdated
@burgholzer

Copy link
Copy Markdown
Member

Lastly, I think a changelog entry is missing also.

In that regard, please just fold this PR number into the existing changelog entry for the mapping pass (and add your name to it) 😌

@rturrado

Copy link
Copy Markdown
Contributor Author

In that regard, please just fold this PR number into the existing changelog entry for the mapping pass (and add your name to it) 😌

Done, folded #1956 into the place-and-route entry and added my name.

- Doc: describe `fromMapping` as bijective.
- Test: use `llvm::count_if` / `llvm::seq` instead of `std::ranges::count_if` / `std::views::iota`.
- Test: drop the defensive `(void)` cast in `EXPECT_DEATH`.
- Add `munich-quantum-toolkit#1956` to the `place-and-route` changelog entry.

Assisted-by: Claude Opus 4.7 via Claude Code
Signed-off-by: rturrado <rturrado@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

  • 🔍 Trigger review

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.

@MatthiasReumann MatthiasReumann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great! Many, many, thanks for your work on this @rturrado.
Let's merge this 🛫

@burgholzer burgholzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! Thanks for all the work. Let's get this in! 🚀

@burgholzer
burgholzer merged commit 35d3dc2 into munich-quantum-toolkit:main Aug 31, 2026
25 checks passed
@rturrado
rturrado deleted the 1867 branch August 31, 2026 13:22
@rturrado

Copy link
Copy Markdown
Contributor Author

Many thanks for the kind words, guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement of existing feature MLIR Anything related to MLIR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants