MLIR Plugin #881
MLIR Plugin #881
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
flowerthrower
added
MLIR
|
Now that #879 is merged, this needs to be rebased |
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Just did a very quick pass through the whole PR here to leave some feedback.
Some Of that is really only for when we get close to merging this PR, which I feel is still a bit out in the future.
Cpp-Linter Report
|
## Description Pulled out from #881. Adds a convenience macro for adding MQT Core libraries and corresponding aliases. ## 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. - [x] 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: burgholzer <[email protected]> Co-authored-by: Yannick Stade <[email protected]>
…#962) ## Description Also pulled out from #881. Builds on #961 and adds a dedicated option for only building the MQT Core libraries as shared libraries without forcing this on every other library. ## 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. - [x] 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: burgholzer <[email protected]> Co-authored-by: Yannick Stade <[email protected]>
## Description Also pulled out of #881. Adds a top-level MLIR setup call, which ensures that other top-level directories (e.g., the future `plugins` directory) also have access to MLIR and the respective variables. ## 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. - [x] 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: burgholzer <[email protected]> Co-authored-by: Yannick Stade <[email protected]>
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Thanks for the continued work on this @ystade and @flowerthrower 🙏🏼
We are getting somewhere. 🚀
However, there is still a lot to be done before this is close to mergeable.
I hope the inline comments outline some actionable steps.
I think a key part of all of this is to get it tested and running in CI.
This should be a top priority right now so that we can make sure that the Plugin works on all systems we are currently testing.
I believe this can be part of the existing MLIR CI as an additional step in the respective jobs.
Probably starting with runs only on Ubuntu.
The packaging of the plugin is also still very much in need of change. The early we get a continuous testing setup, the early we see how things turn out.
Important aspects:
- we want a working sdist
- we want a working cibuildwheel setup
- we want a working/ready publishing workflow for the plugin
Additionally, a local testing setup (probably as an addition to the existing nox setup) would also be very convenient to have. Ideally there would be a nox session for testing the plugin.
What also crossed my mind is https://docs.astral.sh/uv/concepts/projects/workspaces/
Maybe someone could give that a read and provide a brief overview on whether this is something we might want to consider using.
| - IOp | ||
| - GPhaseOp | ||
| - BarrierOp | ||
| - SOp, SdgOp | ||
| - TOp, TdgOp | ||
| - VOp, VdgOp | ||
| - UOp, U2Op | ||
| - SXOp, SXdgOp | ||
| - iSWAPOp, iSWAPdgOp | ||
| - PeresOp, PeresdgOp | ||
| - DCXOp | ||
| - ECROp | ||
| - RXXOp, RYYOp, RZZOp, RZXOp | ||
| - XXminusYY, XXplusYY |
There was a problem hiding this comment.
I know that I noted that in the other PR with the conversions already, but most of these actually have an equivalent in Catalyst.
And I also believe it is very much desirable that we can translate all quantum gates back and forth. Otherwise, we will always be stuck with a mixture of dialects, which will almost surely hinder optimizations.
Nothing to be tackled in this PR, but definitely in a direct follow-up.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (1)
577-585: Guard RZX parameter access to prevent out-of-bounds read.Line 584 accesses
adaptor.getParams()[0]without verifying the array is non-empty, risking undefined behavior if the verifier changes or params are missing.Apply this diff to add a safety check:
// RZZ on (q0, q1') + if (adaptor.getParams().empty()) { + return op.emitError("RZX expects exactly one parameter"); + } auto rzz = rewriter.create<catalyst::quantum::CustomOp>( op.getLoc(), /*gate=*/"IsingZZ", /*in_qubits=*/ValueRange{extracted.inQubits[0], h1.getOutQubits()[0]}, /*in_ctrl_qubits=*/h1.getOutCtrlQubits(), /*in_ctrl_values=*/extracted.ctrlInfo.ctrlValues, /*params=*/adaptor.getParams()[0], /*adjoint=*/false);plugins/catalyst/test/Conversion/mqtopt_ising.mlir (1)
36-54: Fix FileCheck var redefs; capture the f64 constant; update references.
- Rename Hadamard captures to avoid redefinition (H1U/H2U for uncontrolled, H1C/H2C for controlled).
- Capture the constant once and replace literal “%cst” with “%[[CST]]” in all Ising* checks (use CHECK-DAG to avoid ordering brittleness).
- Controlled block depends on the uncontrolled H2; update that use to H2U.
- Update reinsertion to use H2C.
Apply this patch:
@@ - // --- Uncontrolled ------------------------------------------------------------------- + // --- Uncontrolled ------------------------------------------------------------------- + // CHECK-DAG: %[[CST:.*]] = arith.constant {{.*}} : f64 @@ - // CHECK: %[[RZ0:.*]] = quantum.custom "RZ"(%{{.*}}) %[[Q1]] : !quantum.bit - // CHECK: %[[XY_P:.*]]:2 = quantum.custom "IsingXY"(%cst) %[[Q0]], %[[RZ0]] : !quantum.bit, !quantum.bit + // CHECK: %[[RZ0:.*]] = quantum.custom "RZ"(%{{.*}}) %[[Q1]] : !quantum.bit + // CHECK: %[[XY_P:.*]]:2 = quantum.custom "IsingXY"(%[[CST]]) %[[Q0]], %[[RZ0]] : !quantum.bit, !quantum.bit @@ - // CHECK: %[[X1:.*]] = quantum.custom "X"() %[[XY_P]]#0 : !quantum.bit - // CHECK: %[[RZ2:.*]] = quantum.custom "RZ"(%{{.*}}) %[[RZ1]] : !quantum.bit - // CHECK: %[[XY_M:.*]]:2 = quantum.custom "IsingXY"(%cst) %[[X1]], %[[RZ2]] : !quantum.bit, !quantum.bit + // CHECK: %[[X1:.*]] = quantum.custom "X"() %[[XY_P]]#0 : !quantum.bit + // CHECK: %[[RZ2:.*]] = quantum.custom "RZ"(%{{.*}}) %[[RZ1]] : !quantum.bit + // CHECK: %[[XY_M:.*]]:2 = quantum.custom "IsingXY"(%[[CST]]) %[[X1]], %[[RZ2]] : !quantum.bit, !quantum.bit @@ - // CHECK: %[[X2:.*]] = quantum.custom "X"() %[[XY_M]]#0 : !quantum.bit + // CHECK: %[[X2:.*]] = quantum.custom "X"() %[[XY_M]]#0 : !quantum.bit @@ - // CHECK: %[[XX_P:.*]]:2 = quantum.custom "IsingXX"(%cst) %[[X2]], %[[RZ3]] : !quantum.bit, !quantum.bit - // CHECK: %[[YY_P:.*]]:2 = quantum.custom "IsingYY"(%cst) %[[XX_P]]#0, %[[XX_P]]#1 : !quantum.bit, !quantum.bit - // CHECK: %[[ZZ_P1:.*]]:2 = quantum.custom "IsingZZ"(%cst) %[[YY_P]]#0, %[[YY_P]]#1 : !quantum.bit, !quantum.bit + // CHECK: %[[XX_P:.*]]:2 = quantum.custom "IsingXX"(%[[CST]]) %[[X2]], %[[RZ3]] : !quantum.bit, !quantum.bit + // CHECK: %[[YY_P:.*]]:2 = quantum.custom "IsingYY"(%[[CST]]) %[[XX_P]]#0, %[[XX_P]]#1 : !quantum.bit, !quantum.bit + // CHECK: %[[ZZ_P1:.*]]:2 = quantum.custom "IsingZZ"(%[[CST]]) %[[YY_P]]#0, %[[YY_P]]#1 : !quantum.bit, !quantum.bit @@ - // CHECK: %[[H1:.*]] = quantum.custom "Hadamard"() %[[ZZ_P1]]#1 : !quantum.bit - // CHECK: %[[ZZ_P2:.*]]:2 = quantum.custom "IsingZZ"(%cst) %[[ZZ_P1]]#0, %[[H1]] : !quantum.bit, !quantum.bit - // CHECK: %[[H2:.*]] = quantum.custom "Hadamard"() %[[ZZ_P2]]#1 : !quantum.bit + // CHECK: %[[H1U:.*]] = quantum.custom "Hadamard"() %[[ZZ_P1]]#1 : !quantum.bit + // CHECK: %[[ZZ_P2:.*]]:2 = quantum.custom "IsingZZ"(%[[CST]]) %[[ZZ_P1]]#0, %[[H1U]] : !quantum.bit, !quantum.bit + // CHECK: %[[H2U:.*]] = quantum.custom "Hadamard"() %[[ZZ_P2]]#1 : !quantum.bit @@ - // CHECK: %[[RZ2:.*]], %[[CTRL1A:.*]] = quantum.custom "RZ"(%{{.*}}) %[[H2]] ctrls(%[[Q2]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit - // CHECK: %[[XY_C:.*]]:2, %[[CTRL1B:.*]] = quantum.custom "IsingXY"(%cst) %[[ZZ_P2]]#0, %[[RZ2]] ctrls(%[[CTRL1A]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit - // CHECK: %[[RZ3:.*]], %[[CTRL1:.*]] = quantum.custom "RZ"(%{{.*}}) %[[XY_C]]#1 ctrls(%[[CTRL1B]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit + // CHECK: %[[RZ2C:.*]], %[[CTRL1A:.*]] = quantum.custom "RZ"(%{{.*}}) %[[H2U]] ctrls(%[[Q2]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit + // CHECK: %[[XY_C:.*]]:2, %[[CTRL1B:.*]] = quantum.custom "IsingXY"(%[[CST]]) %[[ZZ_P2]]#0, %[[RZ2C]] ctrls(%[[CTRL1A]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit + // CHECK: %[[RZ3C:.*]], %[[CTRL1:.*]] = quantum.custom "RZ"(%{{.*}}) %[[XY_C]]#1 ctrls(%[[CTRL1B]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit @@ - // CHECK: %[[X1C:.*]], %[[CTRL2A:.*]] = quantum.custom "X"() %[[XY_C]]#0 ctrls(%[[CTRL1]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit - // CHECK: %[[RZ4:.*]], %[[CTRL2B:.*]] = quantum.custom "RZ"(%{{.*}}) %[[RZ3]] ctrls(%[[CTRL2A]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit - // CHECK: %[[XY_CM:.*]]:2, %[[CTRL2C:.*]] = quantum.custom "IsingXY"(%cst) %[[X1C]], %[[RZ4]] ctrls(%[[CTRL2B]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit + // CHECK: %[[X1C:.*]], %[[CTRL2A:.*]] = quantum.custom "X"() %[[XY_C]]#0 ctrls(%[[CTRL1]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit + // CHECK: %[[RZ4:.*]], %[[CTRL2B:.*]] = quantum.custom "RZ"(%{{.*}}) %[[RZ3C]] ctrls(%[[CTRL2A]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit + // CHECK: %[[XY_CM:.*]]:2, %[[CTRL2C:.*]] = quantum.custom "IsingXY"(%[[CST]]) %[[X1C]], %[[RZ4]] ctrls(%[[CTRL2B]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit @@ - // CHECK: %[[XX_C:.*]]:2, %[[CTRL3:.*]] = quantum.custom "IsingXX"(%cst) %[[X2C]], %[[RZ5]] ctrls(%[[CTRL2]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit - // CHECK: %[[YY_C:.*]]:2, %[[CTRL4:.*]] = quantum.custom "IsingYY"(%cst) %[[XX_C]]#0, %[[XX_C]]#1 ctrls(%[[CTRL3]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit - // CHECK: %[[ZZ_C1:.*]]:2, %[[CTRL5:.*]] = quantum.custom "IsingZZ"(%cst) %[[YY_C]]#0, %[[YY_C]]#1 ctrls(%[[CTRL4]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit + // CHECK: %[[XX_C:.*]]:2, %[[CTRL3:.*]] = quantum.custom "IsingXX"(%[[CST]]) %[[X2C]], %[[RZ5]] ctrls(%[[CTRL2]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit + // CHECK: %[[YY_C:.*]]:2, %[[CTRL4:.*]] = quantum.custom "IsingYY"(%[[CST]]) %[[XX_C]]#0, %[[XX_C]]#1 ctrls(%[[CTRL3]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit + // CHECK: %[[ZZ_C1:.*]]:2, %[[CTRL5:.*]] = quantum.custom "IsingZZ"(%[[CST]]) %[[YY_C]]#0, %[[YY_C]]#1 ctrls(%[[CTRL4]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit @@ - // CHECK: %[[H1:.*]], %[[CTRL6:.*]] = quantum.custom "Hadamard"() %[[ZZ_C1]]#1 ctrls(%[[CTRL5]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit - // CHECK: %[[CZZ_P2:.*]]:2, %[[CTRL7:.*]] = quantum.custom "IsingZZ"(%cst) %[[ZZ_C1]]#0, %[[H1]] ctrls(%[[CTRL6]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit - // CHECK: %[[H2:.*]], %[[CTRL8:.*]] = quantum.custom "Hadamard"() %[[CZZ_P2]]#1 ctrls(%[[CTRL7]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit + // CHECK: %[[H1C:.*]], %[[CTRL6:.*]] = quantum.custom "Hadamard"() %[[ZZ_C1]]#1 ctrls(%[[CTRL5]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit + // CHECK: %[[CZZ_P2:.*]]:2, %[[CTRL7:.*]] = quantum.custom "IsingZZ"(%[[CST]]) %[[ZZ_C1]]#0, %[[H1C]] ctrls(%[[CTRL6]]) ctrlvals(%true{{.*}}) : !quantum.bit, !quantum.bit ctrls !quantum.bit + // CHECK: %[[H2C:.*]], %[[CTRL8:.*]] = quantum.custom "Hadamard"() %[[CZZ_P2]]#1 ctrls(%[[CTRL7]]) ctrlvals(%true{{.*}}) : !quantum.bit ctrls !quantum.bit @@ - // CHECK: quantum.insert %[[QREG]][{{.*}}], %[[CZZ_P2]]#0 : !quantum.reg, !quantum.bit - // CHECK: quantum.insert %[[QREG]][{{.*}}], %[[H2]] : !quantum.reg, !quantum.bit + // CHECK: quantum.insert %[[QREG]][{{.*}}], %[[CZZ_P2]]#0 : !quantum.reg, !quantum.bit + // CHECK: quantum.insert %[[QREG]][{{.*}}], %[[H2C]] : !quantum.reg, !quantum.bitRun to detect remaining duplicate captures and stray “%cst”:
#!/bin/bash rg -nP '// CHECK: .*%\[\[([A-Za-z0-9_]+)' plugins/catalyst/test/Conversion/mqtopt_ising.mlir \ | perl -ne 'while(/\[\[([A-Za-z0-9_]+)\b/g){print "$1\n"}' \ | sort | uniq -c | sort -nr | awk '$1>1 {print "dup", $0}' rg -n '%cst' plugins/catalyst/test/Conversion/mqtopt_ising.mlirAlso applies to: 55-73, 74-77
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp(1 hunks)plugins/catalyst/test/Conversion/mqtopt_ising.mlir(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (1)
plugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cpp (17)
op(77-99)op(78-79)op(106-122)op(107-108)op(129-148)op(130-131)op(155-209)op(156-157)op(216-260)op(217-218)op(267-324)op(268-269)op(331-566)op(332-333)context(573-640)patterns(590-590)typeConverter(591-591)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on ubuntu-24.04-arm from exact MLIR source
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on macos-14 from exact MLIR source
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on ubuntu-24.04 from exact MLIR source
🔇 Additional comments (1)
plugins/catalyst/test/Conversion/mqtopt_ising.mlir (1)
77-77: Confirm inserting CTRL8 as a bit is intended.You insert %[[CTRL8]] with type hint “!quantum.bit”. If the second result of controlled ops is a control token (not a qubit), this may be semantically wrong. Please confirm dialect semantics or adjust the CHECK to insert the actual qubit result(s) only.
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
Show resolved
Hide resolved
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
Show resolved
Hide resolved
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Hey @flowerthrower 👋🏻
Many thanks for your continued work on this!
I just had a bit of time and thought I'd give this a proper review (well, as proper as it gets on the phone without an IDE).
Most, if not all, of the comments are pretty minor. I also believe that many of the CodeRabbit suggestions make sense and should be adopted.
I have one bigger comments/request for changes: given how we probably do not want to maintain this plugin (at least in its current form) as part of MQT Core, but we'd still like to keep it for demonstration purposes, it only feels right that the plugin code lives in its own repository, where we can also clearly mark it as "proof of concept" with a respective disclaimer.
With the dialect rewrite coming up, I already feel that we won't have the time and energy to refactor the plugin implementation.
In my opinion, the goal should be to have at least one working release of the plugin out on PyPI that is compatible with one Catalyst and one MQT Core minor version.
At the same time, we definitely do not want to lose the commit history of everyone involved here. So what I'd propose is creating a plugin repository in the MQT organization and making this PR the first PR over there (with all the commits) and to not squash that PR once it is ready, but to do a regular merge. That way, all the contributors and commits are preserved. LLMs should be pretty good in setting this up.
As part of this move, I would strongly argue that the MLIR-to-QuantumComputation, the reverse direction, and the roundtrip can be moved to the plugin as we won't be including them in the dialect rewrite given how feasible it seems to implement passes on top of MLIR itself.
Moving the plugin out of the MQT Core repository will require some slight refactoring in the build system setup on the C++ and Python side, as well as some changes in the linting and CI configuration. Nothing too bad though I suppose.
We are definitely getting there and I think it might not be too long before we can call this completed.
I'd propose to first address everything that is still open here in terms of comments and feedback before starting the move.
Hope this makes sense. Feel free to disagree and discuss any aspects of it. Of course, nothing of that is set in stone. But I believe this could be a solid way forward.
.github/workflows/ci.yml
Outdated
| needs: change-detection | ||
| if: fromJSON(needs.change-detection.outputs.run-cpp-linter) | ||
| uses: munich-quantum-toolkit/workflows/.github/workflows/reusable-cpp-linter.yml@56cf3608b07dc10bda5b98d77ed6ad21ecf7ef5d # v1.17.0 | ||
| uses: munich-quantum-toolkit/workflows/.github/workflows/reusable-cpp-linter.yml@33b0da0cc448ad3abc9df3716ccbd89332d3d491 # v1.17.0 |
There was a problem hiding this comment.
This cannot really be correct as the hash was changed, but the tag was not.
|
|
||
| QubitInfo(QubitIndex p, QubitIndex h) : prog(p), hw(h) {} |
There was a problem hiding this comment.
I fail to see why this change would be necessary.
There was a problem hiding this comment.
We depend on LLVM 21. So I don't see why this would be necessary.
There was a problem hiding this comment.
Looks like unrelated changes to me. Please revert.
There was a problem hiding this comment.
Looks like unrelated changes to me. Please revert.
| // CHECK: %[[CST:.*]] = arith.constant {{.*}} : f64 | ||
| // CHECK: %[[RZ1:.*]] = quantum.custom "RZ"(%[[CST]]) %[[H]] : !quantum.bit | ||
| // CHECK: %[[RY1:.*]] = quantum.custom "RY"(%[[CST]]) %[[RZ1]] : !quantum.bit | ||
| // CHECK: %[[RZ2:.*]] = quantum.custom "RZ"(%[[CST]]) %[[RY1]] adj : !quantum.bit | ||
|
|
||
|
|
||
| // CHECK: %[[NEG_CST:.*]] = arith.constant -{{.*}} : f64 | ||
| // CHECK: %[[RZ3:.*]] = quantum.custom "RZ"(%[[NEG_CST]]) %[[RZ2]] adj : !quantum.bit | ||
| // CHECK: %[[RY2:.*]] = quantum.custom "RY"(%[[NEG_CST]]) %[[RZ3]] : !quantum.bit | ||
| // CHECK: %[[RZ4:.*]] = quantum.custom "RZ"(%[[NEG_CST]]) %[[RY2]] : !quantum.bit | ||
|
|
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
Outdated
Show resolved
Hide resolved
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (11)
.github/workflows/ci_mlir_plugin.yml (5)
47-51: ** ccache is configured but not in use for plugin builds.**The workflow sets up ccache but does not enable it for the plugin CMake configure step. With
prepend_symlinks_to_path: false, ccache will not intercept compiler calls unless explicit compiler launchers are added to CMake.Either set
prepend_symlinks_to_path: trueto prepend ccache symlinks to PATH, or add compiler launchers to the plugin CMake configure step (line 92-93). Apply one of these approaches:Option 1: Enable PATH prepending
- name: Setup ccache uses: Chocobo1/setup-ccache-action@v1 with: - prepend_symlinks_to_path: false + prepend_symlinks_to_path: true override_cache_key: mlir-plugin-${{ matrix.runs-on }}-exact-mlirOption 2: Add compiler launchers to plugin CMake (if avoiding PATH changes)
If Option 1 is not preferred, add compiler launchers to the
uv syncinvocation or to a later CMake reconfig step:- name: Build mqt-core-catalyst-plugin working-directory: plugins/catalyst - run: uv sync --verbose --active + run: | + uv sync --verbose --active -- \ + -DCMAKE_C_COMPILER_LAUNCHER=ccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=ccache(Verify that
uv syncpasses through CMake arguments correctly for your setup.)
68-86: ** LLVM build does not enable ccache compiler launchers.**The LLVM CMake configure omits compiler launcher options, so ccache cannot cache LLVM compilations despite being installed.
Add compiler launcher flags to the CMake invocation:
cmake \ -S llvm \ -B build_llvm \ -G Ninja \ + -DCMAKE_C_COMPILER_LAUNCHER=ccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ -DLLVM_ENABLE_PROJECTS=mlir \
99-102: ** Hard-coded Python version and incorrect otool flag for plugin dependencies.**The script uses a hard-coded
python3.13site-packages path (brittle if Python version changes) andotool -l(which lists load commands, not linked libraries).Use dynamic path detection and the correct
otool -Lflag:- if [ "$(uname)" = "Linux" ]; then - ldd .venv/lib/python3.13/site-packages/mqt/core/plugins/catalyst/mqt-core-catalyst-plugin.so || true - elif [ "$(uname)" = "Darwin" ]; then - otool -l .venv/lib/python3.13/site-packages/mqt/core/plugins/catalyst/mqt-core-catalyst-plugin.dylib || true - fi + PYLIB="$(python -c 'import importlib, sys, os; m=importlib.import_module("mqt.core.plugins.catalyst"); print(os.path.dirname(m.__file__))')" + if [ "$(uname)" = "Linux" ]; then + ldd "${PYLIB}/mqt-core-catalyst-plugin.so" || true + elif [ "$(uname)" = "Darwin" ]; then + otool -L "${PYLIB}/mqt-core-catalyst-plugin.dylib" || true + fi
108-111: ** Incorrect otool flag for Catalyst dependencies on macOS.**Uses
otool -l(load commands) instead ofotool -L(linked dylibs).- otool -l $(which catalyst) || true + otool -L "$(which catalyst)" || true
87-88: ** Missing runtime library path exports after LLVM build.**The workflow sets
MLIR_DIRandLLVM_DIRbut does not exportLD_LIBRARY_PATHorDYLD_LIBRARY_PATH. This can cause plugin/tool runtime failures due to missing libMLIR libraries, as noted in PR review comments (e.g., "missing libMLIR.so.20.1").Add library path exports after setting CMake directories:
echo "MLIR_DIR=$(pwd)/build_llvm/lib/cmake/mlir" >> $GITHUB_ENV echo "LLVM_DIR=$(pwd)/build_llvm/lib/cmake/llvm" >> $GITHUB_ENV + echo "LD_LIBRARY_PATH=$(pwd)/build_llvm/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" >> $GITHUB_ENV + echo "DYLD_LIBRARY_PATH=$(pwd)/build_llvm/lib${DYLD_LIBRARY_PATH:+:$DYLD_LIBRARY_PATH}" >> $GITHUB_ENVplugins/catalyst/README.md (2)
61-61: Update LLVM commit hash to match Catalyst 0.12.0 dependency.The hardcoded LLVM commit hash is incorrect. PennyLane Catalyst 0.12.0 pins to commit
a8513158, notf8cb7987c64dcffb72414a40560055cb717dbf74. This mismatch will cause build failures and version conflicts during plugin builds and runtime.Apply this diff:
-git checkout f8cb7987c64dcffb72414a40560055cb717dbf74 +git checkout a8513158Also update the reference at line 158 to remain consistent.
79-81: Add runtime library path exports for local LLVM/MLIR builds.When building against a locally compiled LLVM/MLIR, the runtime loader needs to find the shared libraries. Add
LD_LIBRARY_PATH(Linux) andDYLD_LIBRARY_PATH(macOS) exports after the CMake directory exports.Apply this diff:
export MLIR_DIR="$PWD/build_llvm/lib/cmake/mlir" export LLVM_DIR="$PWD/build_llvm/lib/cmake/llvm" +export LD_LIBRARY_PATH="$PWD/build_llvm/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" +export DYLD_LIBRARY_PATH="$PWD/build_llvm/lib${DYLD_LIBRARY_PATH:+:$DYLD_LIBRARY_PATH}"plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (4)
130-166: AllocOp: setnqubits_attrfor round‑trip and consider rejecting dynamic qubit memrefs.
ConvertMQTOptAlloccurrently callscatalyst::quantum::AllocOpwithnqubits_attr = nullptr(Line 163), while the reverse conversion (ConvertQuantumAllocinCatalystQuantumToMQTOpt.cpp) explicitly errors out if this attribute is missing (AllocOp missing nqubits_attr). This breaks MQTOpt↔Catalyst round‑trip for qubit registers.For statically‑shaped
memref<...x!mqtopt.Qubit>you can compute the number of qubits from the shape and populate the attribute; for dynamic shapes it’s probably better to fail the match (or at least clearly diagnose that dynamic qubit memrefs aren’t supported yet) until the reverse path supports them.Suggested change:
- // Get the size from memref type or dynamic operands - Value size = nullptr; - - // Check if this is a statically shaped memref - if (memrefType.hasStaticShape() && memrefType.getNumElements() >= 0) { - // For static memref like memref<2x!mqtopt.Qubit>, create constant size - auto numQubits = memrefType.getNumElements(); - auto sizeAttr = rewriter.getI64IntegerAttr(numQubits); - size = rewriter.create<arith::ConstantOp>(op.getLoc(), sizeAttr); - } else { - // For dynamic memref, get size from dynamic operands - auto dynamicOperands = adaptor.getDynamicSizes(); - size = dynamicOperands.empty() ? nullptr : dynamicOperands[0]; - } - - // Replace with quantum alloc operation - rewriter.replaceOpWithNewOp<catalyst::quantum::AllocOp>(op, resultType, - size, nullptr); + // Get the size from memref type or dynamic operands and set nqubits_attr + Value size = nullptr; + IntegerAttr nqubitsAttr = nullptr; + + if (memrefType.hasStaticShape() && memrefType.getNumElements() >= 0) { + // For static memref like memref<2x!mqtopt.Qubit>, use the static size + const auto numQubits = memrefType.getNumElements(); + nqubitsAttr = rewriter.getI64IntegerAttr(numQubits); + auto sizeAttr = rewriter.getI64IntegerAttr(numQubits); + size = rewriter.create<arith::ConstantOp>(op.getLoc(), sizeAttr); + } else { + // Dynamic qubit registers are currently not round‑trip compatible + auto dynamicOperands = adaptor.getDynamicSizes(); + if (dynamicOperands.empty()) { + return op.emitError( + "dynamic qubit memref alloc without explicit size not supported"); + } + size = dynamicOperands[0]; + // Leave nqubitsAttr null here unless/until the reverse path is extended. + } + + // Replace with quantum alloc operation + rewriter.replaceOpWithNewOp<catalyst::quantum::AllocOp>( + op, resultType, size, nqubitsAttr);This will satisfy the reverse converter’s expectations and make the round‑trip robust for static qregs.
542-596: RZX decomposition: guard parameter access to avoid out‑of‑bounds.In the
RZXOpspecialization, the code directly indexesadaptor.getParams()[0](Line 574) without checking that a parameter is present. If the op is ever malformed or the verifier changes, this is a silent out‑of‑bounds access and undefined behaviour.Given RZX logically requires exactly one parameter, the pattern should validate this before creating the
IsingZZcustom gate.Minimal, local fix:
- // Extract operands and control information using helper function - auto extracted = extractOperands(adaptor, rewriter, op.getLoc()); + // Extract operands and control information using helper function + auto extracted = extractOperands(adaptor, rewriter, op.getLoc()); + + const auto params = adaptor.getParams(); + if (params.empty()) { + return op.emitError("RZX expects exactly one parameter"); + } @@ - /*in_ctrl_qubits=*/h1.getOutCtrlQubits(), - /*in_ctrl_values=*/extracted.ctrlInfo.ctrlValues, - /*params=*/adaptor.getParams()[0], + /*in_ctrl_qubits=*/h1.getOutCtrlQubits(), + /*in_ctrl_values=*/extracted.ctrlInfo.ctrlValues, + /*params=*/params[0], /*adjoint=*/false);This keeps the decomposition as‑is but makes the pattern robust against inconsistent IR and future verifier changes.
599-627: GlobalPhase: guardparams[0]access and emit a clear diagnostic when missing.
ConvertMQTOptSimpleGate<opt::GPhaseOp>assumesadaptor.getParams()contains at least one element and unconditionally usesparams[0](Line 620). If the MQTOpt op is ever constructed without a parameter, this becomes undefined behaviour.Since a global phase angle is required, it’s better to check and diagnose explicitly:
- auto ctrlInfo = - extractControlInfo(adaptor.getPosCtrlInQubits(), - adaptor.getNegCtrlInQubits(), rewriter, op.getLoc()); - const auto params = adaptor.getParams(); + auto ctrlInfo = + extractControlInfo(adaptor.getPosCtrlInQubits(), + adaptor.getNegCtrlInQubits(), rewriter, op.getLoc()); + const auto params = adaptor.getParams(); + if (params.empty()) { + return op.emitError("GlobalPhaseOp requires exactly one parameter"); + } @@ - auto gphase = rewriter.create<catalyst::quantum::GlobalPhaseOp>( - op.getLoc(), TypeRange(outCtrlTypes), params[0], false, - ctrlInfo.ctrlQubits, ctrlInfo.ctrlValues); + auto gphase = rewriter.create<catalyst::quantum::GlobalPhaseOp>( + op.getLoc(), TypeRange(outCtrlTypes), params[0], false, + ctrlInfo.ctrlQubits, ctrlInfo.ctrlValues);This mirrors the checks you already have elsewhere and prevents hard‑to‑debug crashes if the IR is malformed.
1143-1253: XXminusYY decomposition: use"PauliX"gate name to stay consistent with reverse conversion.In the
XXminusYYOpspecialization, the pre‑ and post‑conjugation X gates use the gate string"X"(Lines 1194 and 1237):/*gate=*/"X",However, the reverse conversion in
CatalystQuantumToMQTOpt.cpponly recognizes"PauliX"(and elsewhere in this file you already use"PauliX"for X operations, e.g., in the Peres/Peresdg decompositions). Using"X"here will either hit the “Unsupported gate” path or break round‑trip equivalence.Align these with the rest of the code and the reverse converter:
- auto xPre = rewriter.create<catalyst::quantum::CustomOp>( + auto xPre = rewriter.create<catalyst::quantum::CustomOp>( op.getLoc(), - /*gate=*/"X", + /*gate=*/"PauliX", @@ - auto xPost = rewriter.create<catalyst::quantum::CustomOp>( + auto xPost = rewriter.create<catalyst::quantum::CustomOp>( op.getLoc(), - /*gate=*/"X", + /*gate=*/"PauliX",This makes the XXminusYY decomposition consistent with the Peres/Peresdg decompositions and ensures the reverse pass can map these
CustomOps back toopt::XOp.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci_mlir_plugin.yml(1 hunks).gitignore(1 hunks)plugins/catalyst/README.md(1 hunks)plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:219-221
Timestamp: 2025-10-09T13:14:10.178Z
Learning: The MQT Core project (munich-quantum-toolkit/core repository) uses the C++20 standard, not C++17. C++20 features such as abbreviated function templates (e.g., `const auto&` parameters) are supported and valid in this codebase.
📚 Learning: 2025-10-09T13:28:29.237Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 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:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/test/Dialect/MQTOpt/Transforms/lift-measurements.mlir:269-288
Timestamp: 2025-10-09T13:20:11.483Z
Learning: In the MQT MLIR dialect, the `rz` gate should not be included in the `DIAGONAL_GATES` set for the `ReplaceBasisStateControlsWithIfPattern` because its operator matrix does not have the required shape | 1 0 | / | 0 x | for the targets-as-controls optimization. It is only included in `LiftMeasurementsAboveGatesPatterns` where the matrix structure requirement differs.
Applied to files:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-11-01T15:57:31.153Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1283
File: src/qir/runtime/QIR.cpp:196-201
Timestamp: 2025-11-01T15:57:31.153Z
Learning: In the QIR runtime (src/qir/runtime/QIR.cpp), the PRX gate (__quantum__qis__prx__body) is an alias for the R gate (Phased X-Rotation) and should call runtime.apply<qc::R>(theta, phi, qubit), not runtime.apply<qc::RX>() which is a single-parameter rotation gate.
Applied to files:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 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:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-10-09T13:14:10.178Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:219-221
Timestamp: 2025-10-09T13:14:10.178Z
Learning: The MQT Core project (munich-quantum-toolkit/core repository) uses the C++20 standard, not C++17. C++20 features such as abbreviated function templates (e.g., `const auto&` parameters) are supported and valid in this codebase.
Applied to files:
plugins/catalyst/README.md
📚 Learning: 2025-10-08T15:58:03.774Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: docs/qdmi/qiskit_backend.md:24-35
Timestamp: 2025-10-08T15:58:03.774Z
Learning: In MQT Core documentation, console code blocks intentionally use `$` prompt symbols (e.g., `$ uv pip install ...` or `(.venv) $ python -m pip ...`). This is a project-wide documentation style that should not be flagged as an MD014 markdownlint violation.
Applied to files:
plugins/catalyst/README.md
📚 Learning: 2025-10-10T08:09:54.528Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.
Applied to files:
plugins/catalyst/README.md
🧬 Code graph analysis (1)
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (1)
plugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cpp (17)
op(77-99)op(78-79)op(106-122)op(107-108)op(129-148)op(130-131)op(155-209)op(156-157)op(216-260)op(217-218)op(267-324)op(268-269)op(331-566)op(332-333)context(573-640)patterns(590-590)typeConverter(591-591)
🪛 LanguageTool
plugins/catalyst/README.md
[grammar] ~24-~24: Essayez « créer une »
Context: ...If you have any questions, feel free to create a [discussion](https://github.com/munic...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~24-~24: Essayez « créer une »
Context: ...have any questions, feel free to create a [discussion](https://github.com/munich-...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~24-~24: Essayez « ou un »
Context: ...unich-quantum-toolkit/core/discussions) or an [issue](https://github.com/munich-qu...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~24-~24: Essayez « ou un »
Context: ...ch-quantum-toolkit/core/discussions) or an [issue](https://github.com/munich-quant...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[uncategorized] ~24-~24: Hint : « issue » (French) means « exit » (English). Did you mean « problème|question » ?
Context: ...uantum-toolkit/core/discussions) or an [issue](https://github.com/munich-quantum-tool...
(ISSUE)
[grammar] ~24-~24: Le mot « sur » est plus probable.
Context: ...com/munich-quantum-toolkit/core/issues) on [GitHub](https://github.com/munich-quan...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_CONFUSION_ON_SUR)
[grammar] ~28-~28: Envisagez de supprimer « _[ »
Context: ...). ## Contributors and Supporters The _[Munich Quantum Toolkit (MQT)](https://m...
(QB_NEW_FR_OTHER_ERROR_IDS_UNNECESSARY_PUNCTUATION__OPEN_BRACKET)
[grammar] ~28-~28: Envisagez un remplacement
Context: ...ters The Munich Quantum Toolkit (MQT) is developed by the [Chair for Design A...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_PUNCTUATION_CLOSE_PARENTHESIS__CLOSE_PARENTHESIS)
[grammar] ~28-~28: Essayez « automation](https ://wwwwww.cda.cit.tum.de/ »
Context: ...oped by the Chair for Design Automation at the [Technical Univ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_NOUN_AUTOMATIONCLOSE_BRACKETOPEN_PARENTHESISHTTPSCOLONSLASHSLASH_W3PERIODCDAPERIODCITPERIODTUMPERIODDESLASH_AUTOMATIONCLOSE_BRACKETOPEN_PARENT)
[grammar] ~29-~29: Essayez « mqv)](https ://wwwwww.munich »
Context: ...part of the Munich Quantum Valley (MQV) initiative. ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_NOUN_MQVCLOSE_PARENTHESISCLOSE_BRACKETOPEN_PARENTHESISHTTPSCOLONSLASHSLASH_W3PERIODMUNICH_MQVCLOSE_PARENTHESISCLOSE_BRACKETOPEN_PARENTHESISHTT)
[grammar] ~38-~38: Typographie
Context: ...ibutors who have helped make MQT Core a reality!
mqt.core.catalyst is ...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[grammar] ~52-~52: Il y a peut-être une erreur ici
Context: ...d the exact LLVM/MLIR revision (locally) bash # Pick a workspace (optional) mkdir -p ~/dev && cd ~/dev # Clone the exact LLVM revision Catalyst expects git clone https://github.com/llvm/llvm-project.git cd llvm-project git checkout f8cb7987c64dcffb72414a40560055cb717dbf74 # Configure & build MLIR (Release is recommended) cmake -S llvm -B build_llvm -G Ninja \ -DLLVM_ENABLE_PROJECTS=mlir \ -DLLVM_BUILD_EXAMPLES=OFF \ -DLLVM_BUILD_TESTS=OFF \ -DLLVM_INCLUDE_TESTS=OFF \ -DLLVM_INCLUDE_EXAMPLES=OFF \ -DLLVM_ENABLE_ASSERTIONS=ON \ -DLLVM_ENABLE_ZLIB=FORCE_ON \ -DLLVM_ENABLE_ZSTD=OFF \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_CXX_VISIBILITY_PRESET=default cmake --build build_llvm --config Release # Export these for your shell/session export MLIR_DIR="$PWD/build_llvm/lib/cmake/mlir" export LLVM_DIR="$PWD/build_llvm/lib/cmake/llvm" ### 2) Create a local env and build the plug...
(QB_NEW_FR)
[grammar] ~103-~103: Il y a peut-être une erreur ici
Context: ... the MQT plugin with your PennyLane code The MQT plugin provides device configura...
(QB_NEW_FR)
[style] ~107-~107: L’espace doit être omise.
Context: ...ndtrip conversions. Important: Use get_device() from the MQT plugin instead of `qml.device()...
(PONCTUATION_DOUBLE)
[style] ~107-~107: L’espace doit être omise.
Context: ...evice()from the MQT plugin instead ofqml.device()` directly: ```python3 import catalyst import pen...
(PONCTUATION_DOUBLE)
[grammar] ~141-~141: Il y a peut-être une erreur ici
Context: ...on mlir_output = module.mlir_opt ``` Alternative: You can also configure an existing de...
(QB_NEW_FR)
[grammar] ~141-~141: Typographie
Context: ... = module.mlir_opt ``` Alternative: You can also configure an existing devic...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[grammar] ~141-~141: Essayez de remplacer cet élément par « :** »
Context: ...e:** You can also configure an existing device: python3 from mqt.core.plugins.catalyst import configure_device_for_mqt device = qml.device("lightning.qubit", wires=2) device = configure_device_for_mqt(device) ## System Requirements Building the MQT Co...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « sous »
Context: ...ng (and running) is continuously tested under Linux and macOS using the [latest avail...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « et »
Context: ...ing) is continuously tested under Linux and macOS using the [latest available syste...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « en utilisant les »
Context: ...ntinuously tested under Linux and macOS using the [latest available system versions f...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « en utilisant les »
Context: ...usly tested under Linux and macOS using the [latest available system versions for G...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...tested under Linux and macOS using the [latest available system versions for GitHub Ac...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...under Linux and macOS using the [latest available system versions for GitHub Actions](htt...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...x and macOS using the [latest available system versions for GitHub Actions](https://gi...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Il y a peut-être une erreur ici
Context: ...acOS using the [latest available system versions for GitHub Actions](https://github.com/...
(QB_NEW_FR)
[grammar] ~153-~153: Essayez de remplacer cet élément par « disponibles du système pour »
Context: ...g the [latest available system versions for GitHub Actions](https://github.com/acti...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~156-~156: Typographie
Context: ...Catalyst Plugin relies on some external dependencies: - [llvm/llvm-project](https://github.com/ll...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[typographical] ~158-~158: Pas de correspondance fermante ou ouvrante pour le caractère « [ »
Context: ...rnal dependencies: - llvm/llvm-project: ...
(UNPAIRED_BRACKETS)
[typographical] ~158-~158: Caractère d’apostrophe incorrect.
Context: ...n-time environments (specific revision: f8cb7987c64dcffb72414a40560055cb717dbf74). - [PennyLaneAI/catalyst](https://gith...
(APOS_INCORRECT)
[typographical] ~159-~159: Pas de correspondance fermante ou ouvrante pour le caractère « [ »
Context: ...055cb717dbf74`). - [PennyLaneAI/catalyst](https://github.com/PennyLaneAI/catalyst...
(UNPAIRED_BRACKETS)
[typographical] ~160-~160: Pas de correspondance fermante ou ouvrante pour le caractère « [ »
Context: ...ennyLane (version > 0.12.0). - [MQT Core](https://github.com/munich-quantum-toolk...
(UNPAIRED_BRACKETS)
[grammar] ~160-~160: Essayez de remplacer cet élément par « ) : fournit le »
Context: ...ithub.com/munich-quantum-toolkit/core): Provides the MQTOpt MLIR dialect and supporting ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~160-~160: Essayez de remplacer cet élément par « ) : fournit le »
Context: .../munich-quantum-toolkit/core): Provides the MQTOpt MLIR dialect and supporting infr...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~160-~160: Essayez « et l’ »
Context: ...core): Provides the MQTOpt MLIR dialect and supporting infrastructure. Note, both ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~160-~160: Essayez « et l’ »
Context: ...): Provides the MQTOpt MLIR dialect and supporting infrastructure. Note, both LLVM/MLIR a...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~160-~160: Élément manquant
Context: ... the MQTOpt MLIR dialect and supporting infrastructure. Note, both LLVM/MLIR and Catalyst are...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_OTHER)
[grammar] ~166-~166: Il y a peut-être une erreur ici
Context: ...ns). ## Cite This If you want to cite MQT Core's MLIR Plugin, please use the foll...
(QB_NEW_FR)
[grammar] ~166-~166: Il y a peut-être une erreur ici
Context: ... ## Cite This If you want to cite MQT Core's MLIR Plugin, please use the following BibTeX entry:...
(QB_NEW_FR)
[grammar] ~166-~166: Typographie
Context: ...Plugin, please use the following BibTeX entry: bibtex @inproceedings{Hopf_2026, author = {Hopf, Patrick and Ochoa, Erick and Stade, Yannick and Rovara, Damian and Quetschlich, Nils and Florea, Ioan Albert and Izaac, Josh and Wille, Robert and Burgholzer, Lukas}, title = {Integrating Quantum Software Tools with(in) {MLIR}}, year = {2026}, publisher = {Association for Computing Machinery}, address = {New York, NY, USA}, url = {https://doi.org/10.1145/3773656.3773658}, doi = {10.1145/3773656.3773658}, booktitle = {Proceedings of the International Conference on High Performance Computing in Asia-Pacific Region}, keywords = {quantum software development, quantum compilation, intermediate representation, MLIR}, series = {HPCASIA '26} } --- ## Acknowledgements The Munich Quantum Too...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[typographical] ~188-~188: « No. » (abréviation de numéro) est une expression fautive.
Context: ...and innovation program (grant agreement No. 101001318), the Bavarian State Ministry...
(NO)
🪛 markdownlint-cli2 (0.18.1)
plugins/catalyst/README.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
42-42: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on ubuntu-24.04-arm from exact MLIR source
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on ubuntu-24.04 from exact MLIR source
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on macos-14 from exact MLIR source
🔇 Additional comments (4)
.gitignore (2)
139-141: ✅ Build directory patterns appropriately targeted.The change from
test/*/buildtotest/**/build/and addition ofplugins/**/build/correctly supports the plugin architecture while remaining focused. The refined patterns address the previous feedback concern about overly broad ignores—these specifically target test and plugin build directories rather than a blanket**/build/rule.
177-178: ✅ MLIR-specific ignores are appropriate.The new
**/.litpattern correctly ignores LLVM Integrated Tester cache/output directories, which is essential for MLIR plugin development and testing workflows. This aligns with the Catalyst/MLIR infrastructure referenced in PR comments..github/workflows/ci_mlir_plugin.yml (1)
114-132: Good: lit availability check is now in place.The workflow now verifies that
litis installed and installs it if missing (lines 123–126), addressing the past concern about missing lit binary. The BUILD_DIR fallback to"build"(lines 118–120) is also a robust improvement.plugins/catalyst/README.md (1)
164-181: Verify BibTeX citation accuracy.The BibTeX entry at lines 169–180 appears to reference a 2026 conference publication. Ensure the year, DOI, and publication details are correct and will remain stable before the plugin is released.
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
Show resolved
Hide resolved
| [](https://pypi.org/project/mqt.core/) | ||
|  | ||
| [](https://opensource.org/licenses/MIT) | ||
| [](https://doi.org/10.21105/joss.07478) | ||
| [](https://github.com/munich-quantum-toolkit/core/actions/workflows/ci.yml) | ||
| [](https://github.com/munich-quantum-toolkit/core/actions/workflows/cd.yml) | ||
| [](https://mqt.readthedocs.io/projects/core) | ||
| [](https://codecov.io/gh/munich-quantum-toolkit/core) |
There was a problem hiding this comment.
Start the README with a top-level heading instead of badges.
Per markdownlint (MD041), the first line of the file should be a top-level heading (#), not badges. Move badges below the main heading.
Apply this diff to reorganize the header:
-[](https://pypi.org/project/mqt.core/)
-
-[](https://opensource.org/licenses/MIT)
-[](https://doi.org/10.21105/joss.07478)
-[](https://github.com/munich-quantum-toolkit/core/actions/workflows/ci.yml)
-[](https://github.com/munich-quantum-toolkit/core/actions/workflows/cd.yml)
-[](https://mqt.readthedocs.io/projects/core)
-[](https://codecov.io/gh/munich-quantum-toolkit/core)
+# MQT Core Catalyst MLIR Plugin
+
+[](https://pypi.org/project/mqt.core/)
+[](https://opensource.org/licenses/MIT)
+[](https://doi.org/10.21105/joss.07478)
+[](https://github.com/munich-quantum-toolkit/core/actions/workflows/ci.yml)
+[](https://github.com/munich-quantum-toolkit/core/actions/workflows/cd.yml)
+[](https://mqt.readthedocs.io/projects/core)
+[](https://codecov.io/gh/munich-quantum-toolkit/core)Also remove the duplicate heading at line 19 since it is now at the top.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [](https://pypi.org/project/mqt.core/) | |
|  | |
| [](https://opensource.org/licenses/MIT) | |
| [](https://doi.org/10.21105/joss.07478) | |
| [](https://github.com/munich-quantum-toolkit/core/actions/workflows/ci.yml) | |
| [](https://github.com/munich-quantum-toolkit/core/actions/workflows/cd.yml) | |
| [](https://mqt.readthedocs.io/projects/core) | |
| [](https://codecov.io/gh/munich-quantum-toolkit/core) | |
| # MQT Core Catalyst MLIR Plugin | |
| [](https://pypi.org/project/mqt.core/) | |
| [](https://opensource.org/licenses/MIT) | |
| [](https://doi.org/10.21105/joss.07478) | |
| [](https://github.com/munich-quantum-toolkit/core/actions/workflows/ci.yml) | |
| [](https://github.com/munich-quantum-toolkit/core/actions/workflows/cd.yml) | |
| [](https://mqt.readthedocs.io/projects/core) | |
| [](https://codecov.io/gh/munich-quantum-toolkit/core) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In plugins/catalyst/README.md around lines 1 to 8, the file currently starts
with badge images which violates markdownlint MD041; move a top-level heading to
the very first line (e.g., project name with a single leading '#'), then
relocate the existing badge block to directly below that heading, and remove the
duplicate heading found at line 19 so there is only one top-level heading in the
document.
| @@ -0,0 +1,197 @@ | |||
| [](https://pypi.org/project/mqt.core/) | |||
|  | |||
There was a problem hiding this comment.
Correct OS badge claim and add alt text to contributor image.
The OS badge claims Windows support, but burgholzer noted this is untested. Additionally, the contributor image at line 42 lacks alt text (MD045 violation).
Apply this diff:
-
+And add alt text to the image:
- <img src="https://contrib.rocks/image?repo=munich-quantum-toolkit/core" />
+ <img src="https://contrib.rocks/image?repo=munich-quantum-toolkit/core" alt="MQT Core contributors" />Also applies to: 42-44
🤖 Prompt for AI Agents
In plugins/catalyst/README.md around line 2, update the OS badge to remove
"windows" since Windows support is untested (e.g., change "linux | macos |
windows" to "linux | macos"), and in lines 42-44 add descriptive alt text to the
contributor image markdown (replace the image without alt text with one that
includes a concise alt attribute describing the contributor or avatar) so the
badge accurately reflects supported OSes and the image satisfies
accessibility/MD045.
| ```python3 | ||
| import catalyst | ||
| import pennylane as qml | ||
| from catalyst.passes import apply_pass | ||
| from mqt.core.plugins.catalyst import get_device | ||
|
|
||
| # Use get_device() to configure the device for MQT plugin compatibility | ||
| # This prevents gates from being decomposed into unitary matrices | ||
| device = get_device("lightning.qubit", wires=2) | ||
|
|
||
|
|
||
| @apply_pass("mqt.mqtopt-to-catalystquantum") | ||
| @apply_pass("mqt.catalystquantum-to-mqtopt") | ||
| @qml.qnode(device) | ||
| def circuit() -> None: | ||
| qml.Hadamard(wires=[0]) | ||
| qml.CNOT(wires=[0, 1]) | ||
| # Controlled gates will NOT be decomposed to matrices | ||
| qml.ctrl(qml.PauliX(wires=0), control=1) | ||
| catalyst.measure(0) | ||
| catalyst.measure(1) | ||
|
|
||
|
|
||
| @qml.qjit(target="mlir", autograph=True) | ||
| def module() -> None: | ||
| return circuit() | ||
|
|
||
|
|
||
| # Get the optimized MLIR representation | ||
| mlir_output = module.mlir_opt | ||
| ``` | ||
|
|
||
| **Alternative:** You can also configure an existing device: | ||
|
|
||
| ```python3 | ||
| from mqt.core.plugins.catalyst import configure_device_for_mqt | ||
|
|
||
| device = qml.device("lightning.qubit", wires=2) | ||
| device = configure_device_for_mqt(device) | ||
| ``` |
There was a problem hiding this comment.
Correct decorator order for round-trip conversion and add missing import.
For a Catalyst→MQTOpt→Catalyst round-trip, the decorators must be applied in the correct order: catalystquantum-to-mqtopt first, then mqtopt-to-catalystquantum. The current order is reversed. Additionally, the alternative code snippet is missing the import pennylane as qml statement and uses python3 instead of python for the code block language.
Apply this diff:
-@apply_pass("mqt.mqtopt-to-catalystquantum")
-@apply_pass("mqt.catalystquantum-to-mqtopt")
+@apply_pass("mqt.catalystquantum-to-mqtopt")
+@apply_pass("mqt.mqtopt-to-catalystquantum")
@qml.qnode(device)
def circuit() -> None:
qml.Hadamard(wires=[0])
qml.CNOT(wires=[0, 1])
# Controlled gates will NOT be decomposed to matrices
qml.ctrl(qml.PauliX(wires=0), control=1)
catalyst.measure(0)
catalyst.measure(1)And add the missing import to the alternative section:
-```python3
+```python
+import pennylane as qml
from mqt.core.plugins.catalyst import configure_device_for_mqt🧰 Tools
🪛 LanguageTool
[grammar] ~141-~141: Il y a peut-être une erreur ici
Context: ...on mlir_output = module.mlir_opt ``` Alternative: You can also configure an existing de...
(QB_NEW_FR)
[grammar] ~141-~141: Typographie
Context: ... = module.mlir_opt ``` Alternative: You can also configure an existing devic...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[grammar] ~141-~141: Essayez de remplacer cet élément par « :** »
Context: ...e:** You can also configure an existing device: python3 from mqt.core.plugins.catalyst import configure_device_for_mqt device = qml.device("lightning.qubit", wires=2) device = configure_device_for_mqt(device) ## System Requirements Building the MQT Co...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
🤖 Prompt for AI Agents
In plugins/catalyst/README.md around lines 109 to 148, the example uses the
decorators in the wrong order and the alternative snippet is missing an import
and uses the wrong code block language; swap the two @apply_pass decorators so
"@apply_pass('mqt.catalystquantum-to-mqtopt')" is applied first and
"@apply_pass('mqt.mqtopt-to-catalystquantum')" second, change the alternative
code block language from "python3" to "python", and add "import pennylane as
qml" at the top of the alternative snippet before importing
configure_device_for_mqt.
| ## System Requirements | ||
|
|
||
| Building the MQT Core MLIR Catalyst Plugin requires a C++ compiler with support for C++20 and CMake 3.24 or newer. | ||
| Building (and running) is continuously tested under Linux and macOS using the [latest available system versions for GitHub Actions](https://github.com/actions/runner-images). |
There was a problem hiding this comment.
Align system requirements documentation with actual testing scope.
The README claims that building and running is tested on Linux and macOS, but the OS badge at line 2 claims Windows support. Per burgholzer's feedback, Windows is not currently tested. Either remove Windows from the OS badge or clarify that Windows support is experimental/untested.
🧰 Tools
🪛 LanguageTool
[grammar] ~153-~153: Essayez de remplacer cet élément par « sous »
Context: ...ng (and running) is continuously tested under Linux and macOS using the [latest avail...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « et »
Context: ...ing) is continuously tested under Linux and macOS using the [latest available syste...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « en utilisant les »
Context: ...ntinuously tested under Linux and macOS using the [latest available system versions f...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « en utilisant les »
Context: ...usly tested under Linux and macOS using the [latest available system versions for G...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...tested under Linux and macOS using the [latest available system versions for GitHub Ac...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...under Linux and macOS using the [latest available system versions for GitHub Actions](htt...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...x and macOS using the [latest available system versions for GitHub Actions](https://gi...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Il y a peut-être une erreur ici
Context: ...acOS using the [latest available system versions for GitHub Actions](https://github.com/...
(QB_NEW_FR)
[grammar] ~153-~153: Essayez de remplacer cet élément par « disponibles du système pour »
Context: ...g the [latest available system versions for GitHub Actions](https://github.com/acti...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
🤖 Prompt for AI Agents
In plugins/catalyst/README.md around line 153, the README states builds are
continuously tested on Linux and macOS while the OS badge on line 2 includes
Windows; update the documentation to match actual CI testing: either remove
Windows from the OS badge or change the badge/text to indicate Windows is
experimental/untested. Ensure the README explicitly notes that CI verifies Linux
and macOS only and, if keeping Windows in the badge, add a clear parenthetical
note that Windows support is untested/experimental.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.gitignore (1)
176-178: The MLIR section appears incomplete.Lines 176–178 add a comment for MLIR but contain no ignore patterns. Either add MLIR-specific build artifacts to ignore (e.g., MLIR standalone plugin builds or related output), or remove this section if no patterns are needed at this time.
♻️ Duplicate comments (16)
.github/workflows/ci_mlir_plugin.yml (4)
47-51: ccache is not configured to intercept compiler calls.With
prepend_symlinks_to_path: falseand no CMAKE compiler launcher flags in the LLVM and plugin build steps, ccache is set up but not actually used. This was flagged in prior review.To enable ccache caching, either:
- Set
prepend_symlinks_to_path: true(recommended for simplicity), or- Keep
prepend_symlinks_to_path: falseand add explicit CMAKE launchers to both LLVM and plugin build steps.Apply this diff to enable ccache symlink prepending:
- - name: Setup ccache + - name: Setup ccache uses: Chocobo1/setup-ccache-action@v1 with: - prepend_symlinks_to_path: false + prepend_symlinks_to_path: true override_cache_key: mlir-plugin-${{ matrix.runs-on }}-exact-mlir
72-86: LLVM build does not use ccache; add compiler launcher flags.The LLVM cmake invocation does not pass compiler launcher options, so ccache cannot intercept builds. This was flagged in prior review.
Apply this diff to add ccache launchers to the LLVM build:
cmake \ -S llvm \ -B build_llvm \ -G Ninja \ + -DCMAKE_C_COMPILER_LAUNCHER=ccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ -DLLVM_ENABLE_PROJECTS=mlir \
87-88: Missing runtime library path exports; linker may fail to find built LLVM/MLIR libraries.Lines 87–88 export MLIR_DIR and LLVM_DIR for CMake discovery but do not set LD_LIBRARY_PATH (Linux) or DYLD_LIBRARY_PATH (macOS) for runtime linking. Without these, subsequent steps (plugin build, tests) may fail with "cannot find libMLIR.so" errors. This was flagged as a critical issue in prior review.
Apply this diff to export runtime library paths:
echo "MLIR_DIR=$(pwd)/build_llvm/lib/cmake/mlir" >> $GITHUB_ENV echo "LLVM_DIR=$(pwd)/build_llvm/lib/cmake/llvm" >> $GITHUB_ENV + echo "LD_LIBRARY_PATH=$(pwd)/build_llvm/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" >> $GITHUB_ENV + echo "DYLD_LIBRARY_PATH=$(pwd)/build_llvm/lib${DYLD_LIBRARY_PATH:+:$DYLD_LIBRARY_PATH}" >> $GITHUB_ENV
95-111: Hard-coded python3.13 path and incorrect otool flag.This section has two issues flagged in prior review:
- Hard-coded
python3.13site-packages path may break on Python 3.14+ or alternate environments.otool -llists load commands, not dylib dependencies; should beotool -L.Apply this diff to use dynamic path lookup and correct otool flag:
- # Print shared library dependencies for the plugin - - name: Print shared library dependencies for mqt-core-catalyst-plugin - run: | - if [ "$(uname)" = "Linux" ]; then - ldd .venv/lib/python3.13/site-packages/mqt/core/plugins/catalyst/mqt-core-catalyst-plugin.so || true - elif [ "$(uname)" = "Darwin" ]; then - otool -l .venv/lib/python3.13/site-packages/mqt/core/plugins/catalyst/mqt-core-catalyst-plugin.dylib || true - fi - - # Print shared library dependencies for Catalyst - - name: Print shared library dependencies for Catalyst - run: | - if [ "$(uname)" = "Linux" ]; then - ldd $(which catalyst) || true - elif [ "$(uname)" = "Darwin" ]; then - otool -l $(which catalyst) || true - fi + # Print shared library dependencies for the plugin + - name: Print shared library dependencies for mqt-core-catalyst-plugin + run: | + PLUGIN_PATH=$(python -c "import mqt.core.plugins.catalyst as m; import os; p=os.path.dirname(m.__file__); ext='.dylib' if os.uname().sysname=='Darwin' else '.so'; print(os.path.join(p, f'mqt-core-catalyst-plugin{ext}'))" 2>/dev/null || echo "") + if [ -z "$PLUGIN_PATH" ]; then + echo "Plugin module not found" + exit 0 + fi + if [ "$(uname)" = "Linux" ]; then + ldd "$PLUGIN_PATH" || true + elif [ "$(uname)" = "Darwin" ]; then + otool -L "$PLUGIN_PATH" || true + fi + + # Print shared library dependencies for Catalyst + - name: Print shared library dependencies for Catalyst + run: | + CATALYST_BIN=$(command -v catalyst || echo "") + if [ -z "$CATALYST_BIN" ]; then + echo "Catalyst binary not found" + exit 0 + fi + if [ "$(uname)" = "Linux" ]; then + ldd "$CATALYST_BIN" || true + elif [ "$(uname)" = "Darwin" ]; then + otool -L "$CATALYST_BIN" || true + fiplugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (7)
142-166: CRITICAL: Set nqubits_attr on AllocOp to enable round-trip conversion.The reverse conversion (CatalystQuantumToMQTOpt.cpp:81-83) explicitly checks for
nqubits_attrand emits an error if null. Currently line 163 passesnullptras the 4th argument, which will break the Catalyst→MQTOpt→Catalyst round-trip. The code at lines 150-154 already computesnumQubitsfor static memrefs—use it to populate the attribute.Apply this diff:
// Check if this is a statically shaped memref + mlir::IntegerAttr nqubitsAttr = nullptr; if (memrefType.hasStaticShape() && memrefType.getNumElements() >= 0) { // For static memref like memref<2x!mqtopt.Qubit>, create constant size auto numQubits = memrefType.getNumElements(); - auto sizeAttr = rewriter.getI64IntegerAttr(numQubits); - size = rewriter.create<arith::ConstantOp>(op.getLoc(), sizeAttr); + nqubitsAttr = rewriter.getI64IntegerAttr(numQubits); + size = rewriter.create<arith::ConstantOp>(op.getLoc(), nqubitsAttr); } else { // For dynamic memref, get size from dynamic operands auto dynamicOperands = adaptor.getDynamicSizes(); size = dynamicOperands.empty() ? nullptr : dynamicOperands[0]; } // Replace with quantum alloc operation rewriter.replaceOpWithNewOp<catalyst::quantum::AllocOp>(op, resultType, - size, nullptr); + size, nqubitsAttr);
558-565: Remove controls from RZX Hadamard gates.In the standard RZX decomposition (H–RZZ–H), the Hadamard gates are uncontrolled; only the middle RZZ gate carries controls. Lines 562-563 incorrectly pass
extracted.ctrlInfo.ctrlQubitsandctrlValuesto the first H gate, which will apply controls that should not exist.Apply this diff:
// H on q1 auto h1 = rewriter.create<catalyst::quantum::CustomOp>( op.getLoc(), /*gate=*/"Hadamard", /*in_qubits=*/ValueRange{extracted.inQubits[1]}, - /*in_ctrl_qubits=*/extracted.ctrlInfo.ctrlQubits, - /*in_ctrl_values=*/extracted.ctrlInfo.ctrlValues, + /*in_ctrl_qubits=*/ValueRange{}, + /*in_ctrl_values=*/ValueRange{}, /*params=*/ValueRange{}, /*adjoint=*/false);
578-585: Remove controls from second RZX Hadamard gate.The second H gate in the RZX decomposition should also be uncontrolled (same rationale as the first H gate). Lines 582-583 incorrectly pass control qubits/values.
Apply this diff:
// H on q1'' auto h2 = rewriter.create<catalyst::quantum::CustomOp>( op.getLoc(), /*gate=*/"Hadamard", /*in_qubits=*/ValueRange{rzz.getOutQubits()[1]}, - /*in_ctrl_qubits=*/rzz.getOutCtrlQubits(), - /*in_ctrl_values=*/extracted.ctrlInfo.ctrlValues, + /*in_ctrl_qubits=*/ValueRange{}, + /*in_ctrl_values=*/ValueRange{}, /*params=*/ValueRange{}, /*adjoint=*/false);
605-626: Guard GlobalPhase parameter access.Line 620 indexes
params[0]without verifying thatparamsis non-empty. If the operation is malformed or the verifier is relaxed, this could lead to undefined behavior.Apply this diff:
const auto params = adaptor.getParams(); + if (params.empty()) { + return op.emitError("GPhaseOp requires exactly one parameter"); + } // Create output types for GlobalPhaseOp (control qubits only) const Type qubitType =
305-318: Refactor getGateName to return FailureOr instead of empty-string sentinel.Returning
""fromgetGateName(line 306) for unsupported gates is error-prone. Multiple specializations (SWAPOp lines 1060, RXOp/RYOp/RZOp/POp lines 1080/1093/1106/1119) return""for 2+ controls, which allows downstream code to create gates with empty names. Change the signature toFailureOr<StringRef>and fail fast when no mapping exists.Apply this pattern to the method signature and callsite:
private: // Is specialized for each gate type - static StringRef getGateName(std::size_t numControls); + static FailureOr<StringRef> getGateName(std::size_t numControls);Update the callsite:
// Gate name may depend on number of controls - const StringRef gateName = - getGateName(extracted.ctrlInfo.ctrlQubits.size()); - if (gateName.empty()) { + auto maybeGateName = getGateName(extracted.ctrlInfo.ctrlQubits.size()); + if (failed(maybeGateName)) { op->emitError() << "Unsupported controlled gate for op: " << op->getName(); return failure(); } + const StringRef gateName = *maybeGateName;Update specializations (e.g., SWAPOp at lines 1052-1061):
-StringRef ConvertMQTOptSimpleGate<opt::SWAPOp>::getGateName( +FailureOr<StringRef> ConvertMQTOptSimpleGate<opt::SWAPOp>::getGateName( const std::size_t numControls) { if (numControls == 0) { - return "SWAP"; + return StringRef("SWAP"); } if (numControls == 1) { - return "CSWAP"; + return StringRef("CSWAP"); } - return ""; + return failure(); }Apply similar changes to RXOp, RYOp, RZOp, and POp specializations.
1191-1199: Use "PauliX" gate name for consistency with reverse conversion.Line 1194 uses
"X"as the gate name, but the reverse conversion (CatalystQuantumToMQTOpt.cpp) expects"PauliX". This inconsistency will break round-trip conversion. The code at line 908 in this file already uses"PauliX"correctly.Apply this diff:
// Pre-conjugation X on qubit 0 (respect original control semantics). auto xPre = rewriter.create<catalyst::quantum::CustomOp>( op.getLoc(), - /*gate=*/"X", + /*gate=*/"PauliX", /*in_qubits=*/ValueRange{extracted.inQubits[0]}, /*in_ctrl_qubits=*/extracted.ctrlInfo.ctrlQubits, /*in_ctrl_values=*/extracted.ctrlInfo.ctrlValues, /*params=*/ValueRange{}, /*adjoint=*/false);
1234-1242: Use "PauliX" gate name for consistency with reverse conversion.Line 1237 also uses
"X"instead of"PauliX". Apply the same fix as for the pre-conjugation X gate to maintain round-trip consistency.Apply this diff:
// Post-conjugation X on qubit 0 to undo the pre X. auto xPost = rewriter.create<catalyst::quantum::CustomOp>( op.getLoc(), - /*gate=*/"X", + /*gate=*/"PauliX", /*in_qubits=*/ValueRange{isingxy.getOutQubits()[0]}, /*in_ctrl_qubits=*/rz2.getOutCtrlQubits(), /*in_ctrl_values=*/extracted.ctrlInfo.ctrlValues, /*params=*/ValueRange{}, /*adjoint=*/false);plugins/catalyst/README.md (5)
42-42: Add alt text to contributors image (MD045).The
<img>tag is missing analtattribute. Add meaningful alt text for accessibility and markdownlint compliance.- <img src="https://contrib.rocks/image?repo=munich-quantum-toolkit/core" /> + <img src="https://contrib.rocks/image?repo=munich-quantum-toolkit/core" alt="MQT Core contributors" />
61-61: Update LLVM commit hash to match Catalyst 0.12.0 (critical).Catalyst 0.12.0 pinned LLVM to commit a8513158, but the README specifies
f8cb7987c64dcffb72414a40560055cb717dbf74. This version mismatch will cause runtime errors and double-registration of options/passes, as highlighted in the PR feedback. Update to the correct hash immediately.-git checkout f8cb7987c64dcffb72414a40560055cb717dbf74 +git checkout a8513158Also update line 158 to reflect the correct hash:
-- [llvm/llvm-project](https://github.com/llvm/llvm-project): A toolkit for the construction of highly optimized compilers, optimizers, and run-time environments (specific revision: `f8cb7987c64dcffb72414a40560055cb717dbf74`). +- [llvm/llvm-project](https://github.com/llvm/llvm-project): A toolkit for the construction of highly optimized compilers, optimizers, and run-time environments (specific revision: `a8513158`).
120-121: Reverse pass decorator order for correct round-trip (critical).Decorators are applied bottom-to-top; the current order applies
mqtopt-to-catalystquantumfirst, which breaks the Catalyst→MQTOpt→Catalyst round-trip. Swap the decorators to applycatalystquantum-to-mqtoptfirst, thenmqtopt-to-catalystquantum.-@apply_pass("mqt.mqtopt-to-catalystquantum") -@apply_pass("mqt.catalystquantum-to-mqtopt") +@apply_pass("mqt.catalystquantum-to-mqtopt") +@apply_pass("mqt.mqtopt-to-catalystquantum") @qml.qnode(device)The PR feedback confirmed reversing the order is necessary for the round-trip to succeed.
141-148: Add missing import forqmlin alternative code snippet (critical).The alternative example uses
qmlwithout importing PennyLane. Add the import to make the snippet copy-pasteable and self-contained.```python3 +import pennylane as qml from mqt.core.plugins.catalyst import configure_device_for_mqt device = qml.device("lightning.qubit", wires=2) device = configure_device_for_mqt(device)--- `1-9`: **Address multiple markdown and configuration issues:** 1. **MD041 violation (lines 1–9)**: Move badges below the first top-level heading. Markdown documents must begin with a top-level heading (`#`). 2. **MD045 violation (line 42)**: Add `alt` attribute to contributors image: `<img src="https://contrib.rocks/image?repo=munich-quantum-toolkit/core" alt="Contributors" />` 3. **Line 61**: Update LLVM commit hash to `a8513158` (confirmed in Catalyst 0.12.0 release notes). Replace `f8cb7987c64dcffb72414a40560055cb717dbf74` with `a8513158`. 4. **Lines 120–121**: Reverse decorator order for correct round-trip semantics. The decorators should apply in reverse order (innermost first): ```diff -@apply_pass("mqt.mqtopt-to-catalystquantum") -@apply_pass("mqt.catalystquantum-to-mqtopt") +@apply_pass("mqt.catalystquantum-to-mqtopt") +@apply_pass("mqt.mqtopt-to-catalystquantum")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci_mlir_plugin.yml(1 hunks).gitignore(1 hunks)plugins/catalyst/README.md(1 hunks)plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-10-09T13:14:10.178Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:219-221
Timestamp: 2025-10-09T13:14:10.178Z
Learning: The MQT Core project (munich-quantum-toolkit/core repository) uses the C++20 standard, not C++17. C++20 features such as abbreviated function templates (e.g., `const auto&` parameters) are supported and valid in this codebase.
Applied to files:
plugins/catalyst/README.md
📚 Learning: 2025-10-08T15:58:03.774Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: docs/qdmi/qiskit_backend.md:24-35
Timestamp: 2025-10-08T15:58:03.774Z
Learning: In MQT Core documentation, console code blocks intentionally use `$` prompt symbols (e.g., `$ uv pip install ...` or `(.venv) $ python -m pip ...`). This is a project-wide documentation style that should not be flagged as an MD014 markdownlint violation.
Applied to files:
plugins/catalyst/README.md
📚 Learning: 2025-10-10T08:09:54.528Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.
Applied to files:
plugins/catalyst/README.md
📚 Learning: 2025-10-09T13:28:29.237Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 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:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/test/Dialect/MQTOpt/Transforms/lift-measurements.mlir:269-288
Timestamp: 2025-10-09T13:20:11.483Z
Learning: In the MQT MLIR dialect, the `rz` gate should not be included in the `DIAGONAL_GATES` set for the `ReplaceBasisStateControlsWithIfPattern` because its operator matrix does not have the required shape | 1 0 | / | 0 x | for the targets-as-controls optimization. It is only included in `LiftMeasurementsAboveGatesPatterns` where the matrix structure requirement differs.
Applied to files:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-11-01T15:57:31.153Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1283
File: src/qir/runtime/QIR.cpp:196-201
Timestamp: 2025-11-01T15:57:31.153Z
Learning: In the QIR runtime (src/qir/runtime/QIR.cpp), the PRX gate (__quantum__qis__prx__body) is an alias for the R gate (Phased X-Rotation) and should call runtime.apply<qc::R>(theta, phi, qubit), not runtime.apply<qc::RX>() which is a single-parameter rotation gate.
Applied to files:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 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:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
🧬 Code graph analysis (1)
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (1)
plugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cpp (17)
op(77-99)op(78-79)op(106-122)op(107-108)op(129-148)op(130-131)op(155-209)op(156-157)op(216-260)op(217-218)op(267-324)op(268-269)op(331-566)op(332-333)context(573-640)patterns(590-590)typeConverter(591-591)
🪛 LanguageTool
plugins/catalyst/README.md
[grammar] ~24-~24: Essayez « créer une »
Context: ...If you have any questions, feel free to create a [discussion](https://github.com/munic...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~24-~24: Essayez « créer une »
Context: ...have any questions, feel free to create a [discussion](https://github.com/munich-...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~24-~24: Essayez « ou un »
Context: ...unich-quantum-toolkit/core/discussions) or an [issue](https://github.com/munich-qu...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~24-~24: Essayez « ou un »
Context: ...ch-quantum-toolkit/core/discussions) or an [issue](https://github.com/munich-quant...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[uncategorized] ~24-~24: Hint : « issue » (French) means « exit » (English). Did you mean « problème|question » ?
Context: ...uantum-toolkit/core/discussions) or an [issue](https://github.com/munich-quantum-tool...
(ISSUE)
[grammar] ~24-~24: Le mot « sur » est plus probable.
Context: ...com/munich-quantum-toolkit/core/issues) on [GitHub](https://github.com/munich-quan...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_CONFUSION_ON_SUR)
[grammar] ~28-~28: Envisagez de supprimer « _[ »
Context: ...). ## Contributors and Supporters The _[Munich Quantum Toolkit (MQT)](https://m...
(QB_NEW_FR_OTHER_ERROR_IDS_UNNECESSARY_PUNCTUATION__OPEN_BRACKET)
[grammar] ~28-~28: Envisagez un remplacement
Context: ...ters The Munich Quantum Toolkit (MQT) is developed by the [Chair for Design A...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_PUNCTUATION_CLOSE_PARENTHESIS__CLOSE_PARENTHESIS)
[grammar] ~28-~28: Essayez « automation](https ://wwwwww.cda.cit.tum.de/ »
Context: ...oped by the Chair for Design Automation at the [Technical Univ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_NOUN_AUTOMATIONCLOSE_BRACKETOPEN_PARENTHESISHTTPSCOLONSLASHSLASH_W3PERIODCDAPERIODCITPERIODTUMPERIODDESLASH_AUTOMATIONCLOSE_BRACKETOPEN_PARENT)
[grammar] ~29-~29: Essayez « mqv)](https ://wwwwww.munich »
Context: ...part of the Munich Quantum Valley (MQV) initiative. ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_NOUN_MQVCLOSE_PARENTHESISCLOSE_BRACKETOPEN_PARENTHESISHTTPSCOLONSLASHSLASH_W3PERIODMUNICH_MQVCLOSE_PARENTHESISCLOSE_BRACKETOPEN_PARENTHESISHTT)
[grammar] ~38-~38: Typographie
Context: ...ibutors who have helped make MQT Core a reality!
mqt.core.catalyst is ...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[grammar] ~52-~52: Il y a peut-être une erreur ici
Context: ...d the exact LLVM/MLIR revision (locally) bash # Pick a workspace (optional) mkdir -p ~/dev && cd ~/dev # Clone the exact LLVM revision Catalyst expects git clone https://github.com/llvm/llvm-project.git cd llvm-project git checkout f8cb7987c64dcffb72414a40560055cb717dbf74 # Configure & build MLIR (Release is recommended) cmake -S llvm -B build_llvm -G Ninja \ -DLLVM_ENABLE_PROJECTS=mlir \ -DLLVM_BUILD_EXAMPLES=OFF \ -DLLVM_BUILD_TESTS=OFF \ -DLLVM_INCLUDE_TESTS=OFF \ -DLLVM_INCLUDE_EXAMPLES=OFF \ -DLLVM_ENABLE_ASSERTIONS=ON \ -DLLVM_ENABLE_ZLIB=FORCE_ON \ -DLLVM_ENABLE_ZSTD=OFF \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_CXX_VISIBILITY_PRESET=default cmake --build build_llvm --config Release # Export these for your shell/session export MLIR_DIR="$PWD/build_llvm/lib/cmake/mlir" export LLVM_DIR="$PWD/build_llvm/lib/cmake/llvm" ### 2) Create a local env and build the plug...
(QB_NEW_FR)
[grammar] ~103-~103: Il y a peut-être une erreur ici
Context: ... the MQT plugin with your PennyLane code The MQT plugin provides device configura...
(QB_NEW_FR)
[style] ~107-~107: L’espace doit être omise.
Context: ...ndtrip conversions. Important: Use get_device() from the MQT plugin instead of `qml.device()...
(PONCTUATION_DOUBLE)
[style] ~107-~107: L’espace doit être omise.
Context: ...evice()from the MQT plugin instead ofqml.device()` directly: ```python3 import catalyst import pen...
(PONCTUATION_DOUBLE)
[grammar] ~141-~141: Il y a peut-être une erreur ici
Context: ...on mlir_output = module.mlir_opt ``` Alternative: You can also configure an existing de...
(QB_NEW_FR)
[grammar] ~141-~141: Typographie
Context: ... = module.mlir_opt ``` Alternative: You can also configure an existing devic...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[grammar] ~141-~141: Essayez de remplacer cet élément par « :** »
Context: ...e:** You can also configure an existing device: python3 from mqt.core.plugins.catalyst import configure_device_for_mqt device = qml.device("lightning.qubit", wires=2) device = configure_device_for_mqt(device) ## System Requirements Building the MQT Co...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « sous »
Context: ...ng (and running) is continuously tested under Linux and macOS using the [latest avail...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « et »
Context: ...ing) is continuously tested under Linux and macOS using the [latest available syste...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « en utilisant les »
Context: ...ntinuously tested under Linux and macOS using the [latest available system versions f...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « en utilisant les »
Context: ...usly tested under Linux and macOS using the [latest available system versions for G...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...tested under Linux and macOS using the [latest available system versions for GitHub Ac...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...under Linux and macOS using the [latest available system versions for GitHub Actions](htt...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Essayez de remplacer cet élément par « dernières »
Context: ...x and macOS using the [latest available system versions for GitHub Actions](https://gi...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~153-~153: Il y a peut-être une erreur ici
Context: ...acOS using the [latest available system versions for GitHub Actions](https://github.com/...
(QB_NEW_FR)
[grammar] ~153-~153: Essayez de remplacer cet élément par « disponibles du système pour »
Context: ...g the [latest available system versions for GitHub Actions](https://github.com/acti...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~156-~156: Typographie
Context: ...Catalyst Plugin relies on some external dependencies: - [llvm/llvm-project](https://github.com/ll...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[typographical] ~158-~158: Pas de correspondance fermante ou ouvrante pour le caractère « [ »
Context: ...rnal dependencies: - llvm/llvm-project: ...
(UNPAIRED_BRACKETS)
[typographical] ~158-~158: Caractère d’apostrophe incorrect.
Context: ...n-time environments (specific revision: f8cb7987c64dcffb72414a40560055cb717dbf74). - [PennyLaneAI/catalyst](https://gith...
(APOS_INCORRECT)
[typographical] ~159-~159: Pas de correspondance fermante ou ouvrante pour le caractère « [ »
Context: ...055cb717dbf74`). - [PennyLaneAI/catalyst](https://github.com/PennyLaneAI/catalyst...
(UNPAIRED_BRACKETS)
[typographical] ~160-~160: Pas de correspondance fermante ou ouvrante pour le caractère « [ »
Context: ...ennyLane (version > 0.12.0). - [MQT Core](https://github.com/munich-quantum-toolk...
(UNPAIRED_BRACKETS)
[grammar] ~160-~160: Essayez de remplacer cet élément par « ) : fournit le »
Context: ...ithub.com/munich-quantum-toolkit/core): Provides the MQTOpt MLIR dialect and supporting ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~160-~160: Essayez de remplacer cet élément par « ) : fournit le »
Context: .../munich-quantum-toolkit/core): Provides the MQTOpt MLIR dialect and supporting infr...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~160-~160: Essayez « et l’ »
Context: ...core): Provides the MQTOpt MLIR dialect and supporting infrastructure. Note, both ...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~160-~160: Essayez « et l’ »
Context: ...): Provides the MQTOpt MLIR dialect and supporting infrastructure. Note, both LLVM/MLIR a...
(QB_NEW_FR_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~160-~160: Élément manquant
Context: ... the MQTOpt MLIR dialect and supporting infrastructure. Note, both LLVM/MLIR and Catalyst are...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_OTHER)
[grammar] ~166-~166: Il y a peut-être une erreur ici
Context: ...ns). ## Cite This If you want to cite MQT Core's MLIR Plugin, please use the foll...
(QB_NEW_FR)
[grammar] ~166-~166: Il y a peut-être une erreur ici
Context: ... ## Cite This If you want to cite MQT Core's MLIR Plugin, please use the following BibTeX entry:...
(QB_NEW_FR)
[grammar] ~166-~166: Typographie
Context: ...Plugin, please use the following BibTeX entry: bibtex @inproceedings{Hopf_2026, author = {Hopf, Patrick and Ochoa, Erick and Stade, Yannick and Rovara, Damian and Quetschlich, Nils and Florea, Ioan Albert and Izaac, Josh and Wille, Robert and Burgholzer, Lukas}, title = {Integrating Quantum Software Tools with(in) {MLIR}}, year = {2026}, publisher = {Association for Computing Machinery}, address = {New York, NY, USA}, url = {https://doi.org/10.1145/3773656.3773658}, doi = {10.1145/3773656.3773658}, booktitle = {Proceedings of the International Conference on High Performance Computing in Asia-Pacific Region}, keywords = {quantum software development, quantum compilation, intermediate representation, MLIR}, series = {HPCASIA '26} } --- ## Acknowledgements The Munich Quantum Too...
(QB_NEW_FR_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_SPACE)
[typographical] ~188-~188: « No. » (abréviation de numéro) est une expression fautive.
Context: ...and innovation program (grant agreement No. 101001318), the Bavarian State Ministry...
(NO)
[grammar] ~189-~189: Il y a peut-être une erreur ici
Context: ...rogram (grant agreement No. 101001318), the Bavarian State Ministry for Science and Arts through the Distinguished Professorship Program, as well as the Munich Quantum Valley, w...
(QB_NEW_FR)
[grammar] ~189-~189: Envisagez une suppression.
Context: ...ed Professorship Program, as well as the Munich Quantum Valley, which is supporte...
(QB_NEW_FR_OTHER_ERROR_IDS_UNNECESSARY_SPACE_
)
🪛 markdownlint-cli2 (0.18.1)
plugins/catalyst/README.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
42-42: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on ubuntu-24.04-arm from exact MLIR source
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on ubuntu-24.04 from exact MLIR source
- GitHub Check: 🧩 Test MLIR Catalyst Plugin on macos-14 from exact MLIR source
🔇 Additional comments (7)
.github/workflows/ci_mlir_plugin.yml (7)
1-18: Trigger and concurrency configuration looks good.The workflow appropriately triggers on plugin changes and includes merge group support. Concurrency group prevents duplicate runs. No concerns.
20-30: Job strategy and parallelism settings are well-configured.Matrix covers the expected platforms, and parallelism levels are reasonable. No concerns.
91-93: Plugin build step is straightforward.The uv build configuration looks appropriate. However, success depends on the MLIR_DIR/LLVM_DIR environment being correctly set and the runtime library paths being exported (see prior comment about missing LD_LIBRARY_PATH/DYLD_LIBRARY_PATH).
122-126: Good improvement: lit check and install.The addition of the lit availability check and conditional install is a solid improvement addressing prior feedback. This ensures lit is available before CMake configuration.
134-172: clang-tidy step is well-structured but depends on upstream fixes.The clang-tidy invocation includes proper checks for compile_commands.json and Catalyst includes, making it resilient. However, success depends on:
- Correct env vars from LLVM build (MLIR_DIR, LLVM_DIR, and library paths)
- Plugin build completing successfully
174-182: LIT test step is appropriate but may need library path handling.The step correctly uses ninja to invoke LIT tests. However, test execution may fail at runtime if LD_LIBRARY_PATH/DYLD_LIBRARY_PATH are not set (see prior comment about missing library path exports).
184-187: pytest step is straightforward and well-configured.Simple invocation with verbose and summary flags. Depends on plugin build and environment correctness from prior steps.
| BUILD_DIR=$(find build -type d -name Release | head -n1) | ||
| if [ -z "$BUILD_DIR" ]; then | ||
| BUILD_DIR="build" | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
BUILD_DIR detection logic is duplicated across three steps.
The same BUILD_DIR detection pattern (find Release subdir, fallback to "build") appears in three places. This is a code smell that violates DRY and makes future maintenance harder.
Consider extracting this to a reusable shell function or GitHub Actions composite action. For example, define a shell function early in the workflow:
detect_build_dir() {
local dir=$(find build -type d -name Release | head -n1)
echo "${dir:-build}"
}Then call it in each step:
- name: Reconfigure CMake for LIT
working-directory: plugins/catalyst
run: |
+ detect_build_dir() {
+ local dir=$(find build -type d -name Release | head -n1)
+ echo "${dir:-build}"
+ }
- BUILD_DIR=$(find build -type d -name Release | head -n1)
- if [ -z "$BUILD_DIR" ]; then
- BUILD_DIR="build"
- fi
+ BUILD_DIR=$(detect_build_dir)Repeat for the clang-tidy and LIT test steps.
Also applies to: 138-141, 178-181
🤖 Prompt for AI Agents
.github/workflows/ci_mlir_plugin.yml lines 117-120 (also similar blocks at
138-141 and 178-181): the BUILD_DIR detection logic is duplicated across three
steps; extract the logic into a single reusable function or composite action and
call it from each step instead of repeating the find/fallback block. Add a shell
function (e.g., detect_build_dir) defined once in the workflow's top-level run:
or a reusable composite action that returns the build dir, then replace each
duplicated block with a simple call that assigns BUILD_DIR=$(detect_build_dir)
(or uses the composite action output) so all three steps share the same
implementation and fallback behavior.
| @@ -0,0 +1,197 @@ | |||
| [](https://pypi.org/project/mqt.core/) | |||
|  | |||
There was a problem hiding this comment.
🧩 Analysis chain
Remove Windows from OS badge — not tested.
The badge claims Windows support, but line 153 clearly states: "Building (and running) is continuously tested under Linux and macOS." Past review feedback confirmed Windows is not tested. Update the badge to remove Windows.
-
+I'll verify the review comment's claims by examining the actual file content.
🏁 Script executed:
#!/bin/bash
# Check lines around 2 and 153 in plugins/catalyst/README.md
echo "=== Lines 1-5 (checking line 2 OS badge) ==="
sed -n '1,5p' plugins/catalyst/README.md
echo -e "\n=== Lines 150-156 (checking line 153 testing statement) ==="
sed -n '150,156p' plugins/catalyst/README.mdLength of output: 1262
Remove Windows from OS badge — not actually tested.
The OS badge claims Windows support, but line 153 explicitly documents that "Building (and running) is continuously tested under Linux and macOS." Update the badge to reflect the actual tested platforms:
-
+🤖 Prompt for AI Agents
In plugins/catalyst/README.md around line 2, the OS badge incorrectly lists
Windows even though the project is only tested on Linux and macOS; update the
badge to remove Windows and reflect only "linux | macos" (or use separate badges
for Linux and macOS) so the README matches documented testing; replace the
existing shield URL with one that omits "windows" and ensure the style and
wording remain consistent with the rest of the README.
| . .venv/bin/activate | ||
|
|
||
| # Install Catalyst and build the plugin | ||
| uv pip install pennylane-catalyst>0.12.0 |
There was a problem hiding this comment.
Pin Catalyst version more precisely to avoid incompatible releases.
The constraint pennylane-catalyst>0.12.0 allows future versions with potentially different LLVM requirements. Since the entire setup is pinned to Catalyst 0.12.0's LLVM commit, use >=0.12.0 or explicitly document which versions are tested, to prevent silent version mismatches.
-uv pip install pennylane-catalyst>0.12.0
+uv pip install pennylane-catalyst>=0.12.0Consider adding a note that the setup has been tested with Catalyst 0.12.0 specifically.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uv pip install pennylane-catalyst>0.12.0 | |
| uv pip install pennylane-catalyst>=0.12.0 |
🤖 Prompt for AI Agents
In plugins/catalyst/README.md around line 94, the pip constraint
`pennylane-catalyst>0.12.0` is too loose and may allow incompatible future
releases; update the requirement to either `pennylane-catalyst>=0.12.0` (if you
accept newer compatible releases) or pin to a tested release like
`pennylane-catalyst==0.12.0` (if you require exact reproducibility), and add a
one-line note stating which Catalyst version (e.g., 0.12.0) and LLVM commit the
setup was tested against.
|
|
||
| **Important:** Use `get_device()` from the MQT plugin instead of `qml.device()` directly: | ||
|
|
||
| ```python3 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use python instead of python3 for code fence language.
For better syntax highlighting compatibility, change the code fence marker from python3 to python.
-```python3
+```python🤖 Prompt for AI Agents
In plugins/catalyst/README.md at line 109, the fenced code block language is set
to "python3"; replace the opening fence marker from ```python3 to ```python to
ensure standard syntax highlighting compatibility (and update any other
instances of "```python3" in the file to "```python" if present), then save and
re-run your markdown linter/preview to confirm highlighting renders correctly.
…ntry point in pyproject.toml; fix linter workflow reference in CI configuration
…g file matching criteria
…and improve install directives
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (14)
.github/workflows/ci.yml (1)
158-158: Inconsistent workflow reference pinning breaks reproducibility.Line 158 uses a semantic version tag
@v1.17, while all other workflow references throughout the file (lines 17, 33, 53, 74, 98, 121, 139, 149, 184, 192, 206, 214, 222, 240) are pinned to specific commit hashes with the pattern@ea4fccbb432596cbb6840a9608d6376b6b2c8ff7 # v1.17.2. This inconsistency undermines reproducibility, as the tag can be moved or updated independently.Update line 158 to match the consistent pinning pattern used elsewhere in the file:
- uses: munich-quantum-toolkit/workflows/.github/workflows/[email protected] + uses: munich-quantum-toolkit/workflows/.github/workflows/reusable-cpp-linter.yml@ea4fccbb432596cbb6840a9608d6376b6b2c8ff7 # v1.17.2plugins/catalyst/test/Conversion/quantum_entangling.mlir (1)
54-57: Minor: Inconsistent spacing in quantum operations.There are extra spaces before the indices in the allocation and extraction operations (e.g.,
alloc( 3),extract %qreg[ 0]). While not breaking functionality, this creates visual inconsistency with other test files.Apply this diff for consistency:
- %qreg = quantum.alloc( 3) : !quantum.reg - %q0 = quantum.extract %qreg[ 0] : !quantum.reg -> !quantum.bit - %q1 = quantum.extract %qreg[ 1] : !quantum.reg -> !quantum.bit - %q2 = quantum.extract %qreg[ 2] : !quantum.reg -> !quantum.bit + %qreg = quantum.alloc(3) : !quantum.reg + %q0 = quantum.extract %qreg[0] : !quantum.reg -> !quantum.bit + %q1 = quantum.extract %qreg[1] : !quantum.reg -> !quantum.bit + %q2 = quantum.extract %qreg[2] : !quantum.reg -> !quantum.bitplugins/catalyst/test/Conversion/quantum_ising.mlir (1)
34-42: FileCheck variable reference syntax error persists.Lines 35 and 42 use
%[[PI:.*]]and%[[PI2:.*]]as references after their initial definition. FileCheck requires removing the capture syntax (:.*) when referencing a previously defined variable.Apply this fix:
// CHECK: %[[PI:.*]] = arith.constant 3.1415926535897931 : f64 - // CHECK: %[[XY:.*]]:2 = mqtopt.xx_plus_yy(%cst, %[[PI:.*]] static [] mask [false, false]) %[[Q0]], %[[Q1]] : !mqtopt.Qubit, !mqtopt.Qubit + // CHECK: %[[XY:.*]]:2 = mqtopt.xx_plus_yy(%cst, %[[PI]] static [] mask [false, false]) %[[Q0]], %[[Q1]] : !mqtopt.Qubit, !mqtopt.Qubit ... // CHECK: %[[PI2:.*]] = arith.constant 3.1415926535897931 : f64 - // CHECK: %[[CXY_T:.*]]:2, %[[CXY_C:.*]] = mqtopt.xx_plus_yy(%cst, %[[PI2:.*]] static [] mask [false, false]) %[[ZZ]]#0, %[[ZZ]]#1 ctrl %[[Q2]] : !mqtopt.Qubit, !mqtopt.Qubit ctrl !mqtopt.Qubit + // CHECK: %[[CXY_T:.*]]:2, %[[CXY_C:.*]] = mqtopt.xx_plus_yy(%cst, %[[PI2]] static [] mask [false, false]) %[[ZZ]]#0, %[[ZZ]]#1 ctrl %[[Q2]] : !mqtopt.Qubit, !mqtopt.Qubit ctrl !mqtopt.Qubitplugins/catalyst/test/Conversion/mqtopt_pauli.mlir (1)
9-13: RUN directive is functional but lacks graceful skip on missing dependencies.The test will fail rather than skip if Catalyst or the plugin path is unavailable. While functional, adding
// REQUIRES: catalystand// REQUIRES: mqt_plugin_pathwould improve CI robustness when the plugin isn't built.plugins/catalyst/pyproject.toml (4)
9-17:pennylane-catalystin[build-system].requiresmay cause PEP 517 isolation issues.Including a runtime dependency in the build-system requirements can cause resolution conflicts in isolated build environments (especially manylinux). Unless CMake configure genuinely imports Catalyst Python modules, consider removing it from build-system and relying solely on
[project.dependencies]and[dependency-groups].[build-system] requires = [ "scikit-build-core>=0.11.5", - "pennylane-catalyst~=0.13.0", # The following dependencies are needed whenever one wants to build with a pre-release version of CatalystIf Catalyst is needed at build time for MLIR_DIR discovery, document this via an environment variable setup in CI.
80-81: Entry point must reference the callable resolver function.The
catalyst.passes_resolutionentry point group expects a resolver function, not a module object. The current value"mqt.core.plugins.catalyst"will fail at runtime when Catalyst attempts to call it.[project.entry-points."catalyst.passes_resolution"] -"mqt.passes" = "mqt.core.plugins.catalyst" +"mqt.passes" = "mqt.core.plugins.catalyst:name2pass"
89-89: PyPI URL points to the wrong package.The URL references
mqt-corebut should point to this plugin's distribution page.-PyPI = "https://pypi.org/project/mqt-core/" +PyPI = "https://pypi.org/project/mqt-core-catalyst-plugin/"
186-192: Consider adding a plugin load smoke test to cibuildwheel.Given prior ABI/load errors across platforms, validating that the plugin loads successfully would catch compatibility issues early.
[tool.cibuildwheel] build = "cp3*" skip = "*-musllinux_* *-win_*" archs = "auto64" test-groups = ["test"] -test-command = "pytest {project}/test/python" +test-command = [ + "python -c 'from mqt.core.plugins.catalyst import get_catalyst_plugin_abs_path; print(get_catalyst_plugin_abs_path())'", + "pytest {project}/test/python", +] build-frontend = "build[uv]"plugins/catalyst/test/Conversion/mqtopt_clifford.mlir (1)
40-50: FileCheck variable redefinition issue persists.The variables
[[CST]]and[[NEG_CST]]are defined twice (lines 40 and 69 forCST, lines 46 and 75 forNEG_CST). FileCheck variables are single-assignment—reusing the capture syntax ([[VAR:.*]]) after initial definition causes a backref match that fails if the SSA names differ.Rename to unique names (e.g.,
[[CST1]]/[[CST2]]and[[NEG_CST1]]/[[NEG_CST2]]) and update all subsequent references.Also applies to: 69-79
plugins/catalyst/cmake/ExternalDependencies.cmake (4)
23-27: Treat incompatible Catalyst version as a fatal error.Per PR discussion, MLIR/LLVM version mismatches cause pass/option re-registration crashes at runtime ("LLVM ERROR: Option 'basic' already exists!"). A warning allows the build to succeed but runtime will fail. Fail fast to surface the issue early.
if(FOUND_CATALYST_VERSION VERSION_LESS ${CATALYST_VERSION}) message( - WARNING - "pennylane-catalyst version ${FOUND_CATALYST_VERSION} in python environment is not compatible." + FATAL_ERROR + "pennylane-catalyst ${FOUND_CATALYST_VERSION} is below required ${CATALYST_VERSION}." )
48-51: Remove duplicate include_directories calls.Lines 48-49 and 50-51 are identical duplicates.
include_directories("${CATALYST_INCLUDE_DIRS}") include_directories("${CATALYST_BUILD_INCLUDE_DIR}") - include_directories("${CATALYST_INCLUDE_DIRS}") - include_directories("${CATALYST_BUILD_INCLUDE_DIR}")
16-19: Consider capturing RESULT_VARIABLE for explicit error handling.If the Python import fails,
FOUND_CATALYST_VERSIONwill be empty, but you cannot distinguish "not installed" from other errors (e.g., Python crash). This is a minor robustness improvement.execute_process( COMMAND "${Python3_EXECUTABLE}" -c "import catalyst; print(catalyst.__version__)" RESULT_VARIABLE _catalyst_rc ERROR_VARIABLE _catalyst_err OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE FOUND_CATALYST_VERSION) if(NOT _catalyst_rc EQUAL 0) set(FOUND_CATALYST_VERSION "") endif()
13-14: MoveCATALYST_VERSIONoutside the conditional block.
CATALYST_VERSIONis defined inside theif(DEFINED Python3_EXECUTABLE...)block, but it's referenced at line 76 in theelsebranch where it will be undefined. This causesfind_package(Catalyst ${CATALYST_VERSION} QUIET)to run without a version constraint.Apply this diff:
include(FetchContent) +set(CATALYST_VERSION 0.13.0) + if(DEFINED Python3_EXECUTABLE AND Python3_EXECUTABLE) - set(CATALYST_VERSION 0.13.0) # Check if the pennylane-catalyst package is installed in the python environment.plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (1)
1061-1136: Consider refactoring to useFailureOr<StringRef>instead of empty string sentinel.Several
getGateNamespecializations return empty string for unsupported control counts (SWAP with 2+ controls, RX/RY/RZ/P with 2+ controls). While the callsite at lines 306-310 correctly checksgateName.empty()and emits an error, usingFailureOr<StringRef>would be more idiomatic MLIR style and make the failure path explicit in the type system.This is a stylistic improvement; the current implementation is functionally correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/ci.yml(2 hunks)plugins/catalyst/cmake/ExternalDependencies.cmake(1 hunks)plugins/catalyst/include/mlir/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.td(1 hunks)plugins/catalyst/lib/CMakeLists.txt(1 hunks)plugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cpp(1 hunks)plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp(1 hunks)plugins/catalyst/lib/mqt-plugin.cpp(1 hunks)plugins/catalyst/pyproject.toml(1 hunks)plugins/catalyst/python/mqt/core/plugins/catalyst/__init__.py(1 hunks)plugins/catalyst/python/mqt/core/plugins/catalyst/device.py(1 hunks)plugins/catalyst/test/Conversion/mqtopt_clifford.mlir(1 hunks)plugins/catalyst/test/Conversion/mqtopt_entangling.mlir(1 hunks)plugins/catalyst/test/Conversion/mqtopt_ising.mlir(1 hunks)plugins/catalyst/test/Conversion/mqtopt_param.mlir(1 hunks)plugins/catalyst/test/Conversion/mqtopt_pauli.mlir(1 hunks)plugins/catalyst/test/Conversion/quantum_clifford.mlir(1 hunks)plugins/catalyst/test/Conversion/quantum_entangling.mlir(1 hunks)plugins/catalyst/test/Conversion/quantum_ising.mlir(1 hunks)plugins/catalyst/test/Conversion/quantum_pauli.mlir(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 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:
plugins/catalyst/test/Conversion/mqtopt_pauli.mlirplugins/catalyst/test/Conversion/quantum_ising.mlirplugins/catalyst/test/Conversion/mqtopt_entangling.mlirplugins/catalyst/test/Conversion/mqtopt_ising.mlirplugins/catalyst/test/Conversion/quantum_pauli.mlirplugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cppplugins/catalyst/test/Conversion/mqtopt_clifford.mlirplugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cppplugins/catalyst/test/Conversion/quantum_entangling.mlirplugins/catalyst/test/Conversion/quantum_clifford.mlir
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/test/Dialect/MQTOpt/Transforms/lift-measurements.mlir:269-288
Timestamp: 2025-10-09T13:20:11.483Z
Learning: In the MQT MLIR dialect, the `rz` gate should not be included in the `DIAGONAL_GATES` set for the `ReplaceBasisStateControlsWithIfPattern` because its operator matrix does not have the required shape | 1 0 | / | 0 x | for the targets-as-controls optimization. It is only included in `LiftMeasurementsAboveGatesPatterns` where the matrix structure requirement differs.
Applied to files:
plugins/catalyst/test/Conversion/mqtopt_pauli.mlirplugins/catalyst/include/mlir/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.tdplugins/catalyst/test/Conversion/mqtopt_param.mlirplugins/catalyst/test/Conversion/quantum_ising.mlirplugins/catalyst/test/Conversion/mqtopt_entangling.mlirplugins/catalyst/test/Conversion/mqtopt_ising.mlirplugins/catalyst/test/Conversion/quantum_pauli.mlirplugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cppplugins/catalyst/test/Conversion/mqtopt_clifford.mlirplugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cppplugins/catalyst/test/Conversion/quantum_entangling.mlir
📚 Learning: 2025-11-01T15:57:31.153Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1283
File: src/qir/runtime/QIR.cpp:196-201
Timestamp: 2025-11-01T15:57:31.153Z
Learning: In the QIR runtime (src/qir/runtime/QIR.cpp), the PRX gate (__quantum__qis__prx__body) is an alias for the R gate (Phased X-Rotation) and should call runtime.apply<qc::R>(theta, phi, qubit), not runtime.apply<qc::RX>() which is a single-parameter rotation gate.
Applied to files:
plugins/catalyst/include/mlir/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.tdplugins/catalyst/test/Conversion/mqtopt_param.mlirplugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-10-09T13:28:29.237Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 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:
plugins/catalyst/include/mlir/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.tdplugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cppplugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.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:
plugins/catalyst/test/Conversion/mqtopt_entangling.mlir
📚 Learning: 2025-10-10T08:09:54.528Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.
Applied to files:
plugins/catalyst/lib/CMakeLists.txtplugins/catalyst/pyproject.tomlplugins/catalyst/cmake/ExternalDependencies.cmake
📚 Learning: 2025-10-10T08:10:16.394Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: test/python/na/test_na_fomac.py:35-0
Timestamp: 2025-10-10T08:10:16.394Z
Learning: In the munich-quantum-toolkit/core repository, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which means CMake `install()` commands with `DESTINATION <path>` install files relative to `mqt/core/` in the wheel, making them accessible via `files("mqt.core").joinpath("<path>")`.
Applied to files:
plugins/catalyst/lib/CMakeLists.txtplugins/catalyst/pyproject.tomlplugins/catalyst/cmake/ExternalDependencies.cmake
📚 Learning: 2025-10-09T13:14:10.178Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:219-221
Timestamp: 2025-10-09T13:14:10.178Z
Learning: The MQT Core project (munich-quantum-toolkit/core repository) uses the C++20 standard, not C++17. C++20 features such as abbreviated function templates (e.g., `const auto&` parameters) are supported and valid in this codebase.
Applied to files:
plugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cppplugins/catalyst/cmake/ExternalDependencies.cmake
📚 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:
plugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp
📚 Learning: 2025-11-03T23:09:26.881Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.
Applied to files:
plugins/catalyst/cmake/ExternalDependencies.cmake
📚 Learning: 2025-10-08T15:58:03.774Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: docs/qdmi/qiskit_backend.md:24-35
Timestamp: 2025-10-08T15:58:03.774Z
Learning: In MQT Core documentation, console code blocks intentionally use `$` prompt symbols (e.g., `$ uv pip install ...` or `(.venv) $ python -m pip ...`). This is a project-wide documentation style that should not be flagged as an MD014 markdownlint violation.
Applied to files:
plugins/catalyst/cmake/ExternalDependencies.cmake
🧬 Code graph analysis (2)
plugins/catalyst/python/mqt/core/plugins/catalyst/__init__.py (1)
plugins/catalyst/python/mqt/core/plugins/catalyst/device.py (2)
configure_device_for_mqt(23-79)get_device(82-107)
plugins/catalyst/python/mqt/core/plugins/catalyst/device.py (1)
python/mqt/core/fomac.pyi (1)
operations(107-108)
🔇 Additional comments (33)
.github/workflows/ci.yml (1)
167-167: Plugin linting exclusion properly configured.The
cpp-linter-ignore-extra: "plugins/**"parameter correctly excludes the plugins directory from linting, addressing the concern noted in the prior TODO comment. This aligns with the dedicated MLIR plugin CI workflow (ci_mlir_plugin.yml) handling plugin-specific linting.plugins/catalyst/test/Conversion/quantum_clifford.mlir (2)
1-85: LGTM!The test comprehensively covers Clifford+T gates (H, SX, S, T) and their controlled variants. The FileCheck patterns properly chain SSA values, and the allocation/extraction/reinsertion/deallocation lifecycle is correctly tested.
32-37: The adjoint attribute is not being converted for SX/S/T gates—this is a conversion pass bug, not a test issue.Looking at the conversion code (lines 367-378), the pass explicitly checks
op.getAdjoint()for ISWAP but omits this check for SX, S, and T gates. The MQTOpt dialect does have separatevdg,sdg,tdgoperations (confirmed inmqtopt_clifford.mlirlines 112, 114, 116), so the conversion should map adjoint variants to them, similar to ISWAP→iSWAPdg.The current CHECK patterns (lines 32–37) match the buggy conversion output, but they don't verify the adjoint distinction. Once the conversion is fixed to handle adjoint for SX/S/T, the CHECKs should be updated to verify
mqtopt.vdg(),mqtopt.sdg(),mqtopt.tdg()for the adjoint cases.The input file correctly uses
{adjoint}on lines 63, 65, 67 (and 73, 75, 77 for controlled variants), but the conversion silently drops this information instead of mapping to the dagger variants.The test CHECKs need updating once the conversion pass is fixed to properly handle adjoint SX/S/T gates.
⛔ Skipped due to learnings
Learnt from: DRovara Repo: munich-quantum-toolkit/core PR: 1108 File: mlir/test/Dialect/MQTOpt/Transforms/lift-measurements.mlir:269-288 Timestamp: 2025-10-09T13:20:11.483Z Learning: In the MQT MLIR dialect, the `rz` gate should not be included in the `DIAGONAL_GATES` set for the `ReplaceBasisStateControlsWithIfPattern` because its operator matrix does not have the required shape | 1 0 | / | 0 x | for the targets-as-controls optimization. It is only included in `LiftMeasurementsAboveGatesPatterns` where the matrix structure requirement differs.Learnt from: DRovara Repo: munich-quantum-toolkit/core PR: 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.Learnt from: DRovara Repo: munich-quantum-toolkit/core PR: 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.plugins/catalyst/test/Conversion/quantum_entangling.mlir (1)
32-51: LGTM on gate chaining and lifecycle checks.The uncontrolled gate sequence (SWAP→ISWAP→ISWAPDG→ECR) correctly chains outputs, and controlled gates properly propagate both target and control qubits. The reinsertion section now includes
memref.deallocverification.plugins/catalyst/test/Conversion/mqtopt_param.mlir (1)
53-78: LGTM on test input structure.The MQTOpt input correctly exercises parameterized gates (rx, ry, rz, p, gphase) with both uncontrolled and controlled variants, and properly chains qubit state through the operations.
plugins/catalyst/test/Conversion/mqtopt_entangling.mlir (1)
35-41: Good coverage of gate decomposition semantics.The test properly validates that:
mqtopt.dcxdecomposes to two sequentialCNOToperations (lines 40-41)- The operand order swap between the two CNOTs (
%[[ECR0]]#0, %[[ECR0]]#1then%[[DCX0]]#1, %[[DCX0]]#0) correctly implements DCX semanticsplugins/catalyst/test/Conversion/quantum_pauli.mlir (1)
21-68: Good resolution of FileCheck variable collisions.The FileCheck patterns now use distinct variable names to avoid redefinition errors:
C0-C3for allocation indicesC1_-C4_for single-qubit controlled gate outputsC_0-C_3for release indicesThis addresses the issues flagged in past reviews.
plugins/catalyst/test/Conversion/mqtopt_ising.mlir (2)
1-13: LGTM on copyright and RUN directive.The RUN directive correctly loads the plugin and applies the
mqtopt-to-catalystquantumpass with FileCheck validation.
84-116: Test input IR structure looks correct.The test properly allocates a 3-qubit memref, applies uncontrolled Ising gates (
xx_plus_yy,xx_minus_yy,rxx,ryy,rzz,rzx), then controlled variants, and finally reinserts and deallocates. This provides good coverage for the MQTOpt-to-Catalyst conversion of Ising-type gates.plugins/catalyst/test/Conversion/quantum_ising.mlir (1)
56-82: Test input IR structure is well-formed.The test correctly exercises the Catalyst-to-MQTOpt conversion for Ising gates with proper qubit allocation, extraction, gate application (both uncontrolled and controlled), reinsertion, and deallocation.
plugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cpp (5)
44-72: Type converter implementation is sound.The converter correctly establishes identity mappings for pass-through types and explicit conversions for Catalyst's
QubitType→ MQTOptQubitTypeandQuregType→ dynamicMemRefType. The target materialization returninginputs[0]is appropriate since the actual memref flows from alloc operations.
280-300: Control value inspection now correctly handles arith::ConstantOp.The code properly extracts the defining
arith::ConstantOpand checks forBoolAttrorIntegerAttrto determine positive vs negative controls, addressing the prior concern about usingmlir::Valueas a boolean.
470-510: Controlled gate handling assumes no pre-existing explicit controls.For gates like
CRX,CRY,CRZ, andControlledPhaseShift, the code addsinQubits[0]toinPosCtrlQubitsVec(which may already contain explicit controls frominCtrlQubits). If a CatalystCustomOpwith gate name "CRX" also has explicit controls, the result would have both the implicit control (from the gate name) and the explicit controls combined.Verify this is the intended behavior. If CRX should never have additional explicit controls in Catalyst IR, consider adding a diagnostic:
} else if (gateName == "CRX") { + if (!inCtrlQubits.empty()) { + return op.emitError("CRX gate has implicit control; explicit controls not expected"); + } // CRX gate: 1 control qubit + 1 target qubit inPosCtrlQubitsVec.emplace_back(inQubits[0]);
588-590: Unsupported gate diagnostic uses proper MLIR error emission.The fallback correctly uses
op.emitError()instead ofllvm::errs(), which integrates with MLIR's diagnostic infrastructure and pass failure mechanism.
600-672: Pass structure is well-organized with proper type conversion boilerplate.The pass correctly:
- Sets up legal/illegal dialects
- Marks specific Catalyst ops as legal that have no MQTOpt equivalent
- Wires in function signature, return, call, and branch type conversions
- Uses dynamic legality for flexible handling
The referenced article link (line 631) for MLIR dialect conversion is a helpful resource for maintainers.
plugins/catalyst/test/Conversion/mqtopt_pauli.mlir (2)
41-46: FileCheck patterns for ctrlvals are now robust.The
%[[TRUE:.*]]capture at line 41 is correctly referenced as%[[TRUE]]{{.*}}in subsequent checks, addressing the prior concern about literal%truebrittleness.
57-85: Test IR exercises comprehensive Pauli gate coverage.The test properly covers uncontrolled Pauli gates (X, Y, Z, I), single-control variants (via
ctrl), and multi-control (Toffoli-style with two controls). The reinsertion and deallocation pattern matches other test files in this suite.plugins/catalyst/lib/mqt-plugin.cpp (1)
1-43: LGTM!The plugin registration follows standard MLIR plugin patterns correctly. The license header conflict from the previous review has been resolved—only the MIT license is present. The dialect and pass registration hooks are properly implemented.
plugins/catalyst/include/mlir/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.td (1)
11-24: LGTM!The pass definition correctly declares all dependent dialects (QuantumDialect, ArithDialect, FuncDialect, MemRefDialect, MQTOptDialect) with consistent namespace qualifiers. The previous review concerns have been addressed.
plugins/catalyst/python/mqt/core/plugins/catalyst/__init__.py (1)
20-61: LGTM on the plugin discovery logic.The implementation properly handles:
- Explicit platform validation with clear error for unsupported systems
- Environment variable override with existence validation
- Prefix/suffix matching to avoid false positives
- Fallback to site-packages search
The previous review suggestions have been incorporated.
plugins/catalyst/python/mqt/core/plugins/catalyst/device.py (2)
23-79: Implementation correctly handles device configuration.The function properly:
- Loads capabilities from TOML with the "qjit" program name
- Removes QubitUnitary from operations
- Clears
_to_matrix_opsif present- Sets
qjit_capabilitieshookThe docstring at lines 31-34 now accurately describes the three techniques implemented (the past review comment about controllable/invertible appears to reference an older version of the docstring that has since been corrected).
82-107: LGTM!The convenience wrapper correctly creates a device and applies the MQT configuration. Lazy import of pennylane inside the function is appropriate for an optional dependency.
plugins/catalyst/cmake/ExternalDependencies.cmake (1)
85-119: LGTM!MQT Core configuration is well-structured:
- Version 3.3.3 with matching tag v3.3.3
FETCHCONTENT_SOURCE_DIR_MQT-COREcorrectly points to the parent repository for local buildsFIND_PACKAGE_ARGSprovides fallback discovery with minimum version constraintplugins/catalyst/lib/Conversion/MQTOptToCatalystQuantum/MQTOptToCatalystQuantum.cpp (10)
1-42: LGTM!Proper use of C++20
<numbers>header forstd::numbers::piinstead of non-portable M_PI macros. Header organization is clean.
44-106: LGTM!Helper functions correctly extract and concatenate control qubits with appropriate control values (1 for positive, 0 for negative). The template-based
extractOperandsreduces duplication across gate patterns.
108-128: LGTM!Type converter correctly maps:
MemRef<opt::QubitType>→catalyst::quantum::QuregTypeopt::QubitType→catalyst::quantum::QubitType- Identity for all other types
130-166: LGTM!The
nqubits_attrhandling has been addressed. Static memrefs now setnqubitsAttr(lines 151-153), while dynamic memrefs use the size operand. This enables proper round-trip conversion.
374-388: SX/SXdg handling has been corrected.The past review concern has been addressed:
opt::SXOpis now registered withConvertMQTOptSimpleGate(line 1455) andgetGateNamereturns "SX" (lines 1041-1045)opt::SXdgOpis registered withConvertMQTOptAdjointGate(line 1456) andgetGateInfocorrectly returns{"SX", true}This is now consistent with S/Sdg and T/Tdg patterns.
541-598: RZX decomposition has been corrected.Past review concerns addressed:
- H gates are now uncontrolled (lines 559-561, 583-584) - correct for RZX = H·RZZ·H decomposition
- Parameter bounds check added (lines 566-568)
- Only RZZ carries the controls (lines 573-574)
600-631: LGTM!GlobalPhase parameter bounds check has been added (lines 614-616). The conversion correctly creates output types for control qubits and handles the global phase operation.
1159-1270: LGTM!The XXminusYY decomposition correctly uses "PauliX" gate name (lines 1210, 1253), consistent with the reverse conversion. The decomposition logic using X-conjugation around XXplusYY is mathematically correct.
390-496: LGTM!V and V† gate decompositions are correct:
- V = RZ(π/2) · RY(π/2) · RZ(-π/2) using adjoint flag for the negative angle
- V† = RZ(π/2) · RY(-π/2) · RZ(-π/2)
The mixing of explicit negative angles and adjoint flags is intentional and mathematically equivalent.
1363-1508: LGTM!The pass implementation correctly:
- Configures legal/illegal dialects and dynamic legality for qubit-related memref ops
- Registers all gate conversion patterns including the specialized ones
- Handles function signature, call, return, and branch type conversions
- Uses
applyPartialConversionappropriately for the partial lowering
| # install directive for scikit-build-core | ||
| message(STATUS "Configuring mqt-core-catalyst-plugin CMakeLists.txt") | ||
| install( | ||
| TARGETS mqt-core-catalyst-plugin | ||
| LIBRARY DESTINATION "mqt/core/plugins/catalyst" | ||
| RUNTIME DESTINATION "mqt/core/plugins/catalyst" | ||
| ARCHIVE DESTINATION "mqt/core/plugins/catalyst" | ||
| ) No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for wheel.install-dir configuration in pyproject.toml files
echo "=== Checking for wheel.install-dir configuration ==="
rg -n 'wheel.install-dir|install-dir' --type toml .
echo ""
echo "=== Checking pyproject.toml structure ==="
if [ -f pyproject.toml ]; then
head -50 pyproject.toml
fi
echo ""
echo "=== Checking plugins/catalyst directory structure ==="
find plugins/catalyst -type f -name "*.toml" -o -name "CMakeLists.txt" | head -20Repository: munich-quantum-toolkit/core
Length of output: 2509
🏁 Script executed:
#!/bin/bash
# Read the plugins/catalyst/lib/CMakeLists.txt file
echo "=== plugins/catalyst/lib/CMakeLists.txt ==="
cat -n plugins/catalyst/lib/CMakeLists.txt
echo ""
echo "=== plugins/catalyst/pyproject.toml (relevant sections) ==="
rg -n 'cmake-build-dir|cmake-args|DESTINATION|install-dir' plugins/catalyst/pyproject.toml -A 3
echo ""
echo "=== Check for other install() directives in project ==="
rg -n 'DESTINATION.*mqt/core' --type cmake . | head -20Repository: munich-quantum-toolkit/core
Length of output: 2990
🏁 Script executed:
#!/bin/bash
# Check root CMakeLists.txt for install patterns
echo "=== Root CMakeLists.txt install patterns ==="
rg -n 'install\(' --type cmake -A 5 . | grep -A 5 'DESTINATION' | head -40
echo ""
echo "=== Check if there's a main library install ==="
rg -n 'TARGETS.*core' --type cmake -A 5 . | head -30
echo ""
echo "=== Look at plugins/catalyst/pyproject.toml completely ==="
cat -n plugins/catalyst/pyproject.toml | tail -30Repository: munich-quantum-toolkit/core
Length of output: 4154
🏁 Script executed:
#!/bin/bash
# Check the python structure within plugins/catalyst
echo "=== plugins/catalyst python structure ==="
find plugins/catalyst/python -type f 2>/dev/null | head -20
echo ""
echo "=== Check plugins/catalyst pyproject.toml wheel.packages ==="
rg -n 'wheel.packages|packages' plugins/catalyst/pyproject.toml -A 2
echo ""
echo "=== Check if there's documentation or other CMakeLists in plugin ==="
cat -n plugins/catalyst/CMakeLists.txt | head -50Repository: munich-quantum-toolkit/core
Length of output: 2334
🏁 Script executed:
#!/bin/bash
# Check if there are any references to where the plugin library should be loaded from
echo "=== Check Python code for plugin loading path ==="
rg -n 'plugin\|catalyst' plugins/catalyst/python/mqt/core/plugins/catalyst/ -l
echo ""
echo "=== Check device.py for plugin path ==="
cat plugins/catalyst/python/mqt/core/plugins/catalyst/device.py | head -50Repository: munich-quantum-toolkit/core
Length of output: 2017
Based on my verification, I've confirmed the issue exists but the proposed fix in the original review comment is incorrect. Let me provide the rewritten review:
Install destination uses incorrect relative path.
The plugin's pyproject.toml configures wheel.install-dir = "mqt/core/plugins/catalyst", which means CMake DESTINATION paths are prefixed with this value. The current destination "mqt/core/plugins/catalyst" results in "mqt/core/plugins/catalyst/mqt/core/plugins/catalyst" in the wheel.
Since wheel.install-dir already specifies the plugin's install location, the DESTINATION should be relative to it:
install(
TARGETS mqt-core-catalyst-plugin
- LIBRARY DESTINATION "mqt/core/plugins/catalyst"
- RUNTIME DESTINATION "mqt/core/plugins/catalyst"
- ARCHIVE DESTINATION "mqt/core/plugins/catalyst"
+ LIBRARY DESTINATION "."
+ RUNTIME DESTINATION "."
+ ARCHIVE DESTINATION "."
)This places the plugin library in the same directory as the Python package code (mqt/core/plugins/catalyst/).
🤖 Prompt for AI Agents
In plugins/catalyst/lib/CMakeLists.txt around lines 49 to 56, the install
DESTINATIONs are set to the full path "mqt/core/plugins/catalyst" which gets
prefixed by wheel.install-dir causing a duplicated path in the wheel; change the
DESTINATIONs to be relative to the wheel.install-dir (e.g., use "." or the
appropriate relative subdirectory) so the plugin library files are installed
directly into mqt/core/plugins/catalyst/ instead of
mqt/core/plugins/catalyst/mqt/core/plugins/catalyst/.
| } else if (gateName == "IsingXY") { | ||
| // PennyLane IsingXY has 1 parameter (phi), OpenQASM XXPlusYY needs 2 | ||
| // (theta, beta) Relationship: IsingXY(phi) = XXPlusYY(phi, pi) Add pi as | ||
| // second parameter | ||
| SmallVector<Value> isingxyParams(finalParamValues.begin(), | ||
| finalParamValues.end()); | ||
| auto piAttr = rewriter.getF64FloatAttr(std::numbers::pi); | ||
| isingxyParams.push_back( | ||
| rewriter.create<ConstantOp>(op.getLoc(), piAttr).getResult()); | ||
|
|
||
| SmallVector<double> isingxyStaticParams(staticParamsVec.begin(), | ||
| staticParamsVec.end()); | ||
| SmallVector<bool> isingxyParamsMask(paramsMaskVec.begin(), | ||
| paramsMaskVec.end()); | ||
| isingxyParamsMask.push_back(false); // pi is a dynamic constant | ||
|
|
||
| auto isingxyStaticParamsAttr = | ||
| DenseF64ArrayAttr::get(rewriter.getContext(), isingxyStaticParams); | ||
| auto isingxyParamsMaskAttr = | ||
| DenseBoolArrayAttr::get(rewriter.getContext(), isingxyParamsMask); | ||
|
|
||
| mqtoptOp = rewriter.create<opt::XXplusYYOp>( | ||
| op.getLoc(), inQubits.getTypes(), | ||
| ValueRange(inPosCtrlQubitsVec).getTypes(), | ||
| ValueRange(inNegCtrlQubitsVec).getTypes(), isingxyStaticParamsAttr, | ||
| isingxyParamsMaskAttr, isingxyParams, inQubits, inPosCtrlQubitsVec, | ||
| inNegCtrlQubitsVec); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
IsingXY to XXplusYY conversion correctly adds pi parameter.
The transformation from PennyLane's IsingXY(phi) to OpenQASM's XXPlusYY(theta, beta) with beta = pi is implemented correctly. The pi constant is created dynamically and appended to the parameter list with paramsMask[false] indicating it's not static.
Minor: The comment on line 524 says "pi is a dynamic constant" which is slightly contradictory wording. Consider "pi is passed as a runtime value" for clarity.
🤖 Prompt for AI Agents
In
plugins/catalyst/lib/Conversion/CatalystQuantumToMQTOpt/CatalystQuantumToMQTOpt.cpp
around lines 510 to 536, update the inline comment that currently reads "pi is a
dynamic constant" to clearer wording such as "pi is passed as a runtime value"
to avoid the contradictory phrase; make the change in-place on the comment only.
| cmake.verbose = true | ||
| logging.level = "INFO" | ||
| editable.mode = "inplace" |
There was a problem hiding this comment.
Stray configuration keys at end of file.
Lines 253-255 contain cmake.verbose, logging.level, and editable.mode without a preceding [tool.*] section header. These appear to belong under [tool.scikit-build] or a similar section.
+[tool.scikit-build.cmake]
+verbose = true
+
+[tool.scikit-build.logging]
+level = "INFO"
+
+[tool.scikit-build.editable]
+mode = "inplace"
-cmake.verbose = true
-logging.level = "INFO"
-editable.mode = "inplace"Alternatively, if these are scikit-build-core keys, move them under the existing [tool.scikit-build] section (lines 92-123).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cmake.verbose = true | |
| logging.level = "INFO" | |
| editable.mode = "inplace" | |
| [tool.scikit-build.cmake] | |
| verbose = true | |
| [tool.scikit-build.logging] | |
| level = "INFO" | |
| [tool.scikit-build.editable] | |
| mode = "inplace" |
🤖 Prompt for AI Agents
In plugins/catalyst/pyproject.toml around lines 253-255, there are stray keys
(cmake.verbose, logging.level, editable.mode) placed at the end of the file
without a [tool.*] section header; move these keys into the existing
[tool.scikit-build] section (lines 92-123) so they are valid scikit-build
configuration entries, preserving their values and ordering, and remove the
duplicate stray lines at the file end.
| # First check in the package resources | ||
| for file in resources.files("mqt.core.plugins.catalyst").iterdir(): | ||
| if file.name.startswith("mqt-core-catalyst-plugin") and file.name.endswith(ext): | ||
| return Path(str(file)).resolve() | ||
|
|
||
| # Then check in site-packages | ||
| import site | ||
|
|
||
| for site_dir in site.getsitepackages(): | ||
| site_path = Path(site_dir) / "mqt/core/plugins/catalyst" | ||
| if site_path.exists(): | ||
| for file in site_path.iterdir(): | ||
| if file.name.startswith("mqt-core-catalyst-plugin") and file.name.endswith(ext): | ||
| return file.resolve() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the catalyst plugin directory structure and CMakeLists.txt
find plugins/catalyst -name "CMakeLists.txt" -o -name "__init__.py" | head -20Repository: munich-quantum-toolkit/core
Length of output: 727
🏁 Script executed:
# Read the catalyst CMakeLists.txt to check install destination
fd CMakeLists.txt plugins/catalyst --exec cat {}Repository: munich-quantum-toolkit/core
Length of output: 4560
🏁 Script executed:
# Check the __init__.py file content around the discovery logic
cat -n plugins/catalyst/python/mqt/core/plugins/catalyst/__init__.py | sed -n '1,80p'Repository: munich-quantum-toolkit/core
Length of output: 3114
🏁 Script executed:
# Read the lib/CMakeLists.txt where the plugin library is likely installed
cat plugins/catalyst/lib/CMakeLists.txtRepository: munich-quantum-toolkit/core
Length of output: 2304
🏁 Script executed:
# Check pyproject.toml for wheel install configuration
cat pyproject.toml | grep -A 10 "wheel"Repository: munich-quantum-toolkit/core
Length of output: 1709
🏁 Script executed:
# Search for all install() directives in catalyst plugin
rg -n "install\(" plugins/catalyst --type cmake -A 3Repository: munich-quantum-toolkit/core
Length of output: 53
Based on my verification, I can now confirm the review comment's concern. Here's the rewritten review:
Plugin discovery will fail due to double-prefixed install destination.
The catalyst plugin's CMakeLists.txt specifies:
install(
TARGETS mqt-core-catalyst-plugin
LIBRARY DESTINATION "mqt/core/plugins/catalyst"
...
)Combined with pyproject.toml's wheel.install-dir = "mqt/core", scikit-build-core will install the plugin at mqt/core/mqt/core/plugins/catalyst in the wheel. However, the discovery logic at lines 46 and 54 searches for:
resources.files("mqt.core.plugins.catalyst")site-packages/mqt/core/plugins/catalyst
The plugin will not be found at either location.
Fix: Change the CMakeLists.txt install destination to DESTINATION "plugins/catalyst" (without the mqt/core prefix, since it's already added by scikit-build-core).
🤖 Prompt for AI Agents
In plugins/catalyst/python/mqt/core/plugins/catalyst/__init__.py around lines 45
to 58, plugin discovery fails because the built wheel installs the plugin under
mqt/core/mqt/core/plugins/catalyst (double-prefixed) due to CMakeLists using
LIBRARY DESTINATION "mqt/core/plugins/catalyst" while pyproject sets
wheel.install-dir = "mqt/core"; fix by updating the CMakeLists.txt to install
the target into a relative path that does not include the mqt/core prefix (e.g.,
LIBRARY DESTINATION "plugins/catalyst" or similar), rebuild the wheel, and
verify the plugin files end up at mqt/core/plugins/catalyst so the existing
discovery code (resources.files("mqt.core.plugins.catalyst") and
site-packages/mqt/core/plugins/catalyst) can find them.
| // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %c0 : index to i64 | ||
| // CHECK: quantum.insert %[[QREG]][%[[C0_FINAL]]], %[[CPERESDG_CNOT]]#0 : !quantum.reg, !quantum.bit | ||
| // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %c1 : index to i64 | ||
| // CHECK: quantum.insert %[[QREG]][%[[C1_FINAL]]], %[[CPERESDG_CNOT]]#1 : !quantum.reg, !quantum.bit | ||
| // CHECK: %[[C2_FINAL:.*]] = arith.index_cast %c2 : index to i64 | ||
| // CHECK: quantum.insert %[[QREG]][%[[C2_FINAL]]], %[[CPERESDG_CTRL]] : !quantum.reg, !quantum.bit |
There was a problem hiding this comment.
Use captured FileCheck variables instead of hardcoded names.
The reinsertion checks reference %c0, %c1, %c2 directly instead of using the captured variables %[[C0]], %[[C1]], %[[C2]] from lines 24-26. If MLIR produces different SSA names, these checks will fail.
Apply this diff:
- // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %c0 : index to i64
+ // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %[[C0]] : index to i64
// CHECK: quantum.insert %[[QREG]][%[[C0_FINAL]]], %[[CPERESDG_CNOT]]#0 : !quantum.reg, !quantum.bit
- // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %c1 : index to i64
+ // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %[[C1]] : index to i64
// CHECK: quantum.insert %[[QREG]][%[[C1_FINAL]]], %[[CPERESDG_CNOT]]#1 : !quantum.reg, !quantum.bit
- // CHECK: %[[C2_FINAL:.*]] = arith.index_cast %c2 : index to i64
+ // CHECK: %[[C2_FINAL:.*]] = arith.index_cast %[[C2]] : index to i64
// CHECK: quantum.insert %[[QREG]][%[[C2_FINAL]]], %[[CPERESDG_CTRL]] : !quantum.reg, !quantum.bit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %c0 : index to i64 | |
| // CHECK: quantum.insert %[[QREG]][%[[C0_FINAL]]], %[[CPERESDG_CNOT]]#0 : !quantum.reg, !quantum.bit | |
| // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %c1 : index to i64 | |
| // CHECK: quantum.insert %[[QREG]][%[[C1_FINAL]]], %[[CPERESDG_CNOT]]#1 : !quantum.reg, !quantum.bit | |
| // CHECK: %[[C2_FINAL:.*]] = arith.index_cast %c2 : index to i64 | |
| // CHECK: quantum.insert %[[QREG]][%[[C2_FINAL]]], %[[CPERESDG_CTRL]] : !quantum.reg, !quantum.bit | |
| // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %[[C0]] : index to i64 | |
| // CHECK: quantum.insert %[[QREG]][%[[C0_FINAL]]], %[[CPERESDG_CNOT]]#0 : !quantum.reg, !quantum.bit | |
| // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %[[C1]] : index to i64 | |
| // CHECK: quantum.insert %[[QREG]][%[[C1_FINAL]]], %[[CPERESDG_CNOT]]#1 : !quantum.reg, !quantum.bit | |
| // CHECK: %[[C2_FINAL:.*]] = arith.index_cast %[[C2]] : index to i64 | |
| // CHECK: quantum.insert %[[QREG]][%[[C2_FINAL]]], %[[CPERESDG_CTRL]] : !quantum.reg, !quantum.bit |
🤖 Prompt for AI Agents
In plugins/catalyst/test/Conversion/mqtopt_clifford.mlir around lines 91 to 96,
the FileCheck patterns use hardcoded SSA names %c0, %c1, %c2 for the reinsertion
checks which can break if MLIR gives different names; replace those literal
names with the previously captured variables %[[C0]], %[[C1]], %[[C2]] (or their
FINAL variants if you need distinct casts like %[[C0_FINAL]] etc.) so the checks
reference the captured identifiers consistently (e.g., use %[[C0_FINAL]] =
arith.index_cast %[[C0]] : index to i64 and then quantum.insert
%[[QREG]][%[[C0_FINAL]]], ...).
| // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %c0 : index to i64 | ||
| // CHECK: quantum.insert %[[QREG]][%[[C0_FINAL]]], %[[CPS_T]] : !quantum.reg, !quantum.bit | ||
| // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %c1 : index to i64 | ||
| // CHECK: quantum.insert %[[QREG]][%[[C1_FINAL]]], %[[CPS_C]] : !quantum.reg, !quantum.bit |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use captured variables instead of literals in reinsertion CHECKs.
Lines 47 and 49 use literal %c0 and %c1 instead of the previously captured %[[C0]] and %[[C1]]. This makes the checks fragile if the SSA naming changes.
Apply this diff:
// --- Reinsertion ---------------------------------------------------------------------------
- // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %c0 : index to i64
+ // CHECK: %[[C0_FINAL:.*]] = arith.index_cast %[[C0]] : index to i64
// CHECK: quantum.insert %[[QREG]][%[[C0_FINAL]]], %[[CPS_T]] : !quantum.reg, !quantum.bit
- // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %c1 : index to i64
+ // CHECK: %[[C1_FINAL:.*]] = arith.index_cast %[[C1]] : index to i64
// CHECK: quantum.insert %[[QREG]][%[[C1_FINAL]]], %[[CPS_C]] : !quantum.reg, !quantum.bit
// CHECK: quantum.dealloc %[[QREG]] : !quantum.reg🤖 Prompt for AI Agents
In plugins/catalyst/test/Conversion/mqtopt_param.mlir around lines 47 to 50, the
reinsertion CHECKs use literal indices %c0 and %c1 instead of the previously
captured SSA variables %[[C0]] and %[[C1]], which makes the tests brittle;
change the checks so the index_casts take %[[C0]] and %[[C1]] (e.g.
%[[C0_FINAL]] = arith.index_cast %[[C0]] : index to i64 and %[[C1_FINAL]] =
arith.index_cast %[[C1]] : index to i64) and then use those captured finals in
the quantum.insert checks (quantum.insert %[[QREG]][%[[C0_FINAL]]], %[[CPS_T]]
and quantum.insert %[[QREG]][%[[C1_FINAL]]], %[[CPS_C]]), ensuring you reference
the captured names exactly as used earlier.
| %qreg1 = quantum.insert %qreg[ 0], %q0_cecr : !quantum.reg, !quantum.bit | ||
| %qreg2 = quantum.insert %qreg1[ 1], %q1_cecr : !quantum.reg, !quantum.bit | ||
| %qreg3 = quantum.insert %qreg2[ 2], %q2_cecr : !quantum.reg, !quantum.bit |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Same spacing inconsistency in reinsertion.
- %qreg1 = quantum.insert %qreg[ 0], %q0_cecr : !quantum.reg, !quantum.bit
- %qreg2 = quantum.insert %qreg1[ 1], %q1_cecr : !quantum.reg, !quantum.bit
- %qreg3 = quantum.insert %qreg2[ 2], %q2_cecr : !quantum.reg, !quantum.bit
+ %qreg1 = quantum.insert %qreg[0], %q0_cecr : !quantum.reg, !quantum.bit
+ %qreg2 = quantum.insert %qreg1[1], %q1_cecr : !quantum.reg, !quantum.bit
+ %qreg3 = quantum.insert %qreg2[2], %q2_cecr : !quantum.reg, !quantum.bit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| %qreg1 = quantum.insert %qreg[ 0], %q0_cecr : !quantum.reg, !quantum.bit | |
| %qreg2 = quantum.insert %qreg1[ 1], %q1_cecr : !quantum.reg, !quantum.bit | |
| %qreg3 = quantum.insert %qreg2[ 2], %q2_cecr : !quantum.reg, !quantum.bit | |
| %qreg1 = quantum.insert %qreg[0], %q0_cecr : !quantum.reg, !quantum.bit | |
| %qreg2 = quantum.insert %qreg1[1], %q1_cecr : !quantum.reg, !quantum.bit | |
| %qreg3 = quantum.insert %qreg2[2], %q2_cecr : !quantum.reg, !quantum.bit |
🤖 Prompt for AI Agents
In plugins/catalyst/test/Conversion/quantum_entangling.mlir around lines 73 to
75, the quantum.insert operations use inconsistent spacing inside the register
index (e.g. "%qreg[ 0]" etc.); remove the extra space(s) inside the brackets so
the indices are consistently formatted like "%qreg[0]" for all three reinsertion
lines.
| %qreg = quantum.alloc( 4) : !quantum.reg | ||
| %q0 = quantum.extract %qreg[ 0] : !quantum.reg -> !quantum.bit | ||
| %q1 = quantum.extract %qreg[ 1] : !quantum.reg -> !quantum.bit | ||
| %q2 = quantum.extract %qreg[ 2] : !quantum.reg -> !quantum.bit | ||
| %q3 = quantum.extract %qreg[ 3] : !quantum.reg -> !quantum.bit |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Inconsistent spacing in quantum operations.
Extra spaces in allocation and extraction operations. This is a minor style consistency issue.
- %qreg = quantum.alloc( 4) : !quantum.reg
- %q0 = quantum.extract %qreg[ 0] : !quantum.reg -> !quantum.bit
- %q1 = quantum.extract %qreg[ 1] : !quantum.reg -> !quantum.bit
- %q2 = quantum.extract %qreg[ 2] : !quantum.reg -> !quantum.bit
- %q3 = quantum.extract %qreg[ 3] : !quantum.reg -> !quantum.bit
+ %qreg = quantum.alloc(4) : !quantum.reg
+ %q0 = quantum.extract %qreg[0] : !quantum.reg -> !quantum.bit
+ %q1 = quantum.extract %qreg[1] : !quantum.reg -> !quantum.bit
+ %q2 = quantum.extract %qreg[2] : !quantum.reg -> !quantum.bit
+ %q3 = quantum.extract %qreg[3] : !quantum.reg -> !quantum.bit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| %qreg = quantum.alloc( 4) : !quantum.reg | |
| %q0 = quantum.extract %qreg[ 0] : !quantum.reg -> !quantum.bit | |
| %q1 = quantum.extract %qreg[ 1] : !quantum.reg -> !quantum.bit | |
| %q2 = quantum.extract %qreg[ 2] : !quantum.reg -> !quantum.bit | |
| %q3 = quantum.extract %qreg[ 3] : !quantum.reg -> !quantum.bit | |
| %qreg = quantum.alloc(4) : !quantum.reg | |
| %q0 = quantum.extract %qreg[0] : !quantum.reg -> !quantum.bit | |
| %q1 = quantum.extract %qreg[1] : !quantum.reg -> !quantum.bit | |
| %q2 = quantum.extract %qreg[2] : !quantum.reg -> !quantum.bit | |
| %q3 = quantum.extract %qreg[3] : !quantum.reg -> !quantum.bit |
🤖 Prompt for AI Agents
In plugins/catalyst/test/Conversion/quantum_pauli.mlir around lines 71 to 75,
the quantum.alloc and quantum.extract ops contain extra spaces inside their
parentheses and brackets causing inconsistent spacing; normalize them by
removing the superfluous spaces so allocation reads e.g. quantum.alloc(4) and
extractions read e.g. quantum.extract %qreg[0] : !quantum.reg -> !quantum.bit to
match the project style.
| %q0_ctrlx, %q1_ctrlx = quantum.custom "PauliX"() %q0_i ctrls(%q1) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit | ||
| %q0_ctrly, %q1_ctrly = quantum.custom "PauliY"() %q0_ctrlx ctrls(%q1_ctrlx) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit | ||
| %q0_ctrlz, %q1_ctrlz = quantum.custom "PauliZ"() %q0_ctrly ctrls(%q1_ctrly) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit | ||
| %q0_ctrli, %q1_ctrli = quantum.custom "Identity"() %q0_ctrlz ctrls(%q1_ctrlz) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Missing space after colon in type annotations.
Lines 86-89 have :!quantum.bit instead of : !quantum.bit, which is inconsistent with other lines in this file and standard MLIR formatting.
- %q0_ctrlx, %q1_ctrlx = quantum.custom "PauliX"() %q0_i ctrls(%q1) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit
- %q0_ctrly, %q1_ctrly = quantum.custom "PauliY"() %q0_ctrlx ctrls(%q1_ctrlx) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit
- %q0_ctrlz, %q1_ctrlz = quantum.custom "PauliZ"() %q0_ctrly ctrls(%q1_ctrly) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit
- %q0_ctrli, %q1_ctrli = quantum.custom "Identity"() %q0_ctrlz ctrls(%q1_ctrlz) ctrlvals(%true) :!quantum.bit ctrls !quantum.bit
+ %q0_ctrlx, %q1_ctrlx = quantum.custom "PauliX"() %q0_i ctrls(%q1) ctrlvals(%true) : !quantum.bit ctrls !quantum.bit
+ %q0_ctrly, %q1_ctrly = quantum.custom "PauliY"() %q0_ctrlx ctrls(%q1_ctrlx) ctrlvals(%true) : !quantum.bit ctrls !quantum.bit
+ %q0_ctrlz, %q1_ctrlz = quantum.custom "PauliZ"() %q0_ctrly ctrls(%q1_ctrly) ctrlvals(%true) : !quantum.bit ctrls !quantum.bit
+ %q0_ctrli, %q1_ctrli = quantum.custom "Identity"() %q0_ctrlz ctrls(%q1_ctrlz) ctrlvals(%true) : !quantum.bit ctrls !quantum.bit🤖 Prompt for AI Agents
In plugins/catalyst/test/Conversion/quantum_pauli.mlir around lines 86 to 89,
the type annotations use `:!quantum.bit` without a space after the colon; update
each occurrence to include a space so they read `: !quantum.bit` to match MLIR
formatting and the rest of the file.
| %q0_ccx, %q1_ccx, %q2_ccx = quantum.custom "CNOT"() %q0_ct, %q1_ct ctrls(%q2_ct) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit | ||
| %q0_ccy, %q1_ccy, %q2_ccy = quantum.custom "CY"() %q0_ccx, %q1_ccx ctrls(%q2_ccx) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit | ||
| %q0_ccz, %q1_ccz, %q2_ccz = quantum.custom "CZ"() %q0_ccy, %q1_ccy ctrls(%q2_ccy) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit | ||
| %q0_cccx, %q1_cccx, %q2_cccx, %q3_cccx = quantum.custom "Toffoli"() %q0_ccz, %q1_ccz, %q2_ccz ctrls(%q3) ctrlvals(%true) :!quantum.bit, !quantum.bit, !quantum.bit ctrls !quantum.bit |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Same spacing issue in controlled-C gates section.
- %q0_ccx, %q1_ccx, %q2_ccx = quantum.custom "CNOT"() %q0_ct, %q1_ct ctrls(%q2_ct) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit
- %q0_ccy, %q1_ccy, %q2_ccy = quantum.custom "CY"() %q0_ccx, %q1_ccx ctrls(%q2_ccx) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit
- %q0_ccz, %q1_ccz, %q2_ccz = quantum.custom "CZ"() %q0_ccy, %q1_ccy ctrls(%q2_ccy) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit
- %q0_cccx, %q1_cccx, %q2_cccx, %q3_cccx = quantum.custom "Toffoli"() %q0_ccz, %q1_ccz, %q2_ccz ctrls(%q3) ctrlvals(%true) :!quantum.bit, !quantum.bit, !quantum.bit ctrls !quantum.bit
+ %q0_ccx, %q1_ccx, %q2_ccx = quantum.custom "CNOT"() %q0_ct, %q1_ct ctrls(%q2_ct) ctrlvals(%true) : !quantum.bit, !quantum.bit ctrls !quantum.bit
+ %q0_ccy, %q1_ccy, %q2_ccy = quantum.custom "CY"() %q0_ccx, %q1_ccx ctrls(%q2_ccx) ctrlvals(%true) : !quantum.bit, !quantum.bit ctrls !quantum.bit
+ %q0_ccz, %q1_ccz, %q2_ccz = quantum.custom "CZ"() %q0_ccy, %q1_ccy ctrls(%q2_ccy) ctrlvals(%true) : !quantum.bit, !quantum.bit ctrls !quantum.bit
+ %q0_cccx, %q1_cccx, %q2_cccx, %q3_cccx = quantum.custom "Toffoli"() %q0_ccz, %q1_ccz, %q2_ccz ctrls(%q3) ctrlvals(%true) : !quantum.bit, !quantum.bit, !quantum.bit ctrls !quantum.bit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| %q0_ccx, %q1_ccx, %q2_ccx = quantum.custom "CNOT"() %q0_ct, %q1_ct ctrls(%q2_ct) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit | |
| %q0_ccy, %q1_ccy, %q2_ccy = quantum.custom "CY"() %q0_ccx, %q1_ccx ctrls(%q2_ccx) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit | |
| %q0_ccz, %q1_ccz, %q2_ccz = quantum.custom "CZ"() %q0_ccy, %q1_ccy ctrls(%q2_ccy) ctrlvals(%true) :!quantum.bit, !quantum.bit ctrls !quantum.bit | |
| %q0_cccx, %q1_cccx, %q2_cccx, %q3_cccx = quantum.custom "Toffoli"() %q0_ccz, %q1_ccz, %q2_ccz ctrls(%q3) ctrlvals(%true) :!quantum.bit, !quantum.bit, !quantum.bit ctrls !quantum.bit | |
| %q0_ccx, %q1_ccx, %q2_ccx = quantum.custom "CNOT"() %q0_ct, %q1_ct ctrls(%q2_ct) ctrlvals(%true) : !quantum.bit, !quantum.bit ctrls !quantum.bit | |
| %q0_ccy, %q1_ccy, %q2_ccy = quantum.custom "CY"() %q0_ccx, %q1_ccx ctrls(%q2_ccx) ctrlvals(%true) : !quantum.bit, !quantum.bit ctrls !quantum.bit | |
| %q0_ccz, %q1_ccz, %q2_ccz = quantum.custom "CZ"() %q0_ccy, %q1_ccy ctrls(%q2_ccy) ctrlvals(%true) : !quantum.bit, !quantum.bit ctrls !quantum.bit | |
| %q0_cccx, %q1_cccx, %q2_cccx, %q3_cccx = quantum.custom "Toffoli"() %q0_ccz, %q1_ccz, %q2_ccz ctrls(%q3) ctrlvals(%true) : !quantum.bit, !quantum.bit, !quantum.bit ctrls !quantum.bit |
🤖 Prompt for AI Agents
In plugins/catalyst/test/Conversion/quantum_pauli.mlir around lines 98 to 101,
the controlled-C gates block has inconsistent spacing (e.g., missing/extra
spaces around commas, between operands and attributes like ctrls(...) and
ctrlvals(...), and before the type colon). Normalize the spacing to match the
file style: ensure single spaces after commas, a single space between operand
list and attribute groups (ctrls/ctrlvals), and a single space before the ":"
that precedes the type annotations so the four quantum.custom lines are
consistently formatted.
Description
This PR introduces a MLIR plguin such that the round-trip pass can be executed with Pennylane Catalyst.
It additionally provides the plugin as a wheel.
Checklist: