✨ Build target environments from QDMI payloads - #2227
Conversation
burgholzer
added
feature
ee2c0b4 to
f19a838
Compare
000f1a7 to
119e8dd
Compare
119e8dd to
5aefae1
Compare
f19a838 to
b9a5363
Compare
5aefae1 to
f6ccb12
Compare
b9a5363 to
31d6f2d
Compare
c463a53 to
4289979
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
fcf7027 to
0c598df
Compare
31d6f2d to
b7d1f2a
Compare
0c598df to
6d85e61
Compare
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
This largely looks good to me. I one simplification request, one lifted constraint and likely some better bindings. I have the feeling that changes in the lower layers of the PR stack will trigger a few more changes here.
I am also wondering whether there is a bit too much overlap/duplication between the MLIR and the QDMI concepts and whether some of that could be simplified while still maintaining the proper separation between the two.
@denialhaag @simon1hofmann same question here as in the other PRs: Do you also see some things that need changes?
| /** | ||
| * @brief Snapshot a QDMI device and one accepted payload as a target | ||
| * environment. | ||
| * | ||
| * @details The adapter preserves the exact program format, groups feature | ||
| * records with the same ID and value, and adds the normative baseline of a | ||
| * standard payload. Unknown optional feature metadata remains unknown. | ||
| */ | ||
| [[nodiscard]] llvm::Expected<TargetEnvironment> | ||
| targetEnvironmentFromDevice(const qdmi::Device& device, | ||
| const QDMI_Program_Format& format); | ||
|
|
||
| /** | ||
| * @brief Open a registered QDMI device and snapshot one accepted payload. | ||
| * | ||
| * @details This adapter contains exceptions from the QDMI C++ API and returns | ||
| * them as LLVM errors. The returned environment owns all queried metadata. | ||
| */ | ||
| [[nodiscard]] llvm::Expected<TargetEnvironment> | ||
| targetEnvironmentFromDeviceId(std::string_view deviceId, | ||
| const QDMI_Program_Format& format); | ||
|
|
There was a problem hiding this comment.
This is just me trying to be pragmatic: Most devices will expose a single payload format that they support. In these cases, it would be quite convenient to define a shortcut that either just uses the device or the ID without an explicit program format in the call.
The only thing to choose would be what to do with devices that offer multiple formats. One plausible option would be to default to the first supported format, which, by definition, is the preferred format by the device.
It should likely be pointed out explicitly that there is an overload for the convenience shortcut that allows specifying an explicit format.
As a follow-up, all code that does not explicitly need it otherwise and all docs may be moved to the convenience function.
| if (format.id != "qir" || format.version != "2.1.0" || | ||
| format.profile != "adaptive") { | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
The 2.1.0 feels a bit more restrictive than this likely needs to be.
|
|
||
| import qiskit.circuit | ||
|
|
||
| import mqt.core.qdmi |
There was a problem hiding this comment.
This line feels odd and I am pretty sure that we can get rid of that by better writing the bindings.
simon1hofmann
left a comment
simon1hofmann
left a comment
There was a problem hiding this comment.
Only have a couple of comments about the docs, but besides that this PR also looks really good!
| return copyValue(DEVICE_NAME.data(), DEVICE_NAME.size() + 1U, size, value, | ||
| sizeRet); | ||
| } | ||
| case QDMI_DEVICE_PROPERTY_QUBITSNUM: { |
There was a problem hiding this comment.
Was this the reason for calling it qubits_num in the other PR?
There was a problem hiding this comment.
Yeah. Mainly. But we use the other wording practically everywhere else. So I would be fine with the syntactic break going from C to C++.
| metadata are unknown unless the caller states them: | ||
|
|
||
| ```python | ||
| target = CompilerTarget( |
There was a problem hiding this comment.
CompilerTarget is not imported anymore (deleted in line 18).
There was a problem hiding this comment.
Ah. We need to be careful here while the docs in CI are failing. They should still all pass locally for all of the changes here and produce correct output.
| environment = TargetEnvironment(target, payload) | ||
| environment = TargetEnvironment.from_device_id( | ||
| "mqt.sc.iqm.garnet", | ||
| ProgramFormat.QIR21_BASE_BINARY, |
There was a problem hiding this comment.
The SC provider does not advertise any program formats right?
Same for the example in lines 111-112.
There was a problem hiding this comment.
It does not. But maybe it should.
Maybe the configuration for the device should include which formats the device claims to accept/support. Would allow a more faithful model of an IQM machine.
denialhaag
left a comment
denialhaag
left a comment
There was a problem hiding this comment.
This also LGTM! 🙂 I just have one nitpick and a comment on the documentation:
| #include <nanobind/stl/string_view.h> // NOLINT(misc-include-cleaner) | ||
| #include <nanobind/stl/variant.h> // NOLINT(misc-include-cleaner) | ||
| #include <nanobind/stl/vector.h> // NOLINT(misc-include-cleaner) | ||
| #include <qdmi/constants.h> |
There was a problem hiding this comment.
We still need to streamline our includes when we have fewer PRs open, but this would comply with the current style:
| #include <qdmi/constants.h> | |
| #include "qdmi/constants.h" |
There was a problem hiding this comment.
Yeah. You are right. Still have this wired wrongly in my brain. We did device at some point to only treat stdlib includes with angle brackets, right?
Because this is still an external header pulled in as a dependency.
There was a problem hiding this comment.
Yeah, but then we changed our minds again, following the discussion in FullStaQD/qcc#37. I still have it on my list to apply this across the MQT. Maybe just before the v4 release is a good time, since we hopefully won't have too many open PRs at that point. 🤔
There was a problem hiding this comment.
Yeah. Before the release makes sense.
We may even want to consider that as a point where we may use an "mqt" namespace for our code to differentiate it a tad bit more from plain mlir. Similar to how this is done in FullStaQD with qcc. I was initially opposed to that but I am starting to see the potential benefits.
| The payload specification identifies the exact representation selected for the | ||
| device. MQT Core derives the compiler output from that specification and uses | ||
| The QDMI adapter checks that the device accepts the exact program format. It | ||
| groups program-feature records by ID and value, adds the selected format's | ||
| normative baseline, and preserves whether the optional feature list is known. | ||
| MQT Core derives the compiler output from this payload specification and uses | ||
| the canonical QCO pipeline. The targeted overload therefore accepts one | ||
| `TargetEnvironment` and no independent output or custom pipeline. MQT Core's | ||
| QDMI adapter does not yet translate QDMI program-format and feature metadata, so | ||
| callers must construct the payload specification from the device documentation. | ||
|
|
||
| The example has no reported execution capabilities. A producer must add every | ||
| effective capability, including the selected format's baseline. Set | ||
| `optional_capabilities_known=True` only when the producer also knows that the | ||
| list contains every optional device capability. | ||
| `TargetEnvironment` and no independent output or custom pipeline. |
There was a problem hiding this comment.
This might just be me, but this feels overly technical and not user-facing. Again, this might just be me, but my brain switched off while I was reading. 🤠
There was a problem hiding this comment.
This entire page needs a revamp once the implementation is consolidated.
And some of the terminology might still need bike shedding. As pointed out in several of my own review comments, I am not quite happy yet with how some of the things are framed.
b7d1f2a to
2ff9471
Compare
6d85e61 to
7d372d0
Compare
eaedc54 to
2849167
Compare
7d372d0 to
3290c27
Compare
Snapshot one exact device-supported descriptor, its grouped optional features, and its normative baseline into an owning TargetEnvironment. Expose matching C++ and Python factories. Assisted-by: GPT-5.6 Sol via Codex
Exercise constrained and malformed feature groups, invalid descriptors, snapshot propagation, and exception containment through the public adapter entry points. Remove duplicate validation branches that cannot run after the QDMI C++ client validates provider records and encodings. Assisted-by: GPT-5.6 Sol via Codex
2849167 to
44062e0
Compare
3290c27 to
f109042
Compare
3 participants
🤖 AI text below 🤖
Lead: @simon1hofmann. Cross-repository coordination: @burgholzer.
Description
Bridge the independently developed compiler model (#2219) and QDMI runtime prototype (#2226). Snapshot an open device and one exact accepted program format into an owning
TargetEnvironment, preserving grouped optional features, encoding and the prototype's standard-format baseline.C++ and Python factories share the existing target inference contract: unknown topology or gate-set facts fail early, and DDSIM's arbitrary controlled-operation families and zero-arity global phase remain supported. Python device factories reuse configuration validation at the extension boundary. Examples use DDSIM when they need an executable QIR format; the model-only SC device is not presented as an execution provider.
Integration-only base
The base branch
codex/qdmi-capability-integrationtemporarily merges #2219 and #2226 only to build and test this adapter. It is not a new PR or serial dependency. Neither foundation depends on the other, and #2162 is not included. Once both foundations are available inv4.1, retarget this existing PR there and remove the temporary base. The compiler foundation targets Core 4.0 onmain; the QDMI runtime foundation targets Core 4.1 onv4.1.Native multi-program adoption (#2373), SDK batching consumers, metadata removal, replaceable drivers, and scheduler integration remain independent.
Design and release gate
This remains a draft, non-blocking Core 4.1 / QDMI 1.4 candidate. It must not enter Core 4.0. QDMI #523 and Core #2365 own the unresolved program-capability contract; passing prototype tests does not settle that design. Use released dependency pins before release artifacts are published.
Refs #2365, Munich-Quantum-Software-Stack/QDMI#523.
Validation
AI assistance was used for rebasing, integration adaptations, tests, documentation and this description.
Adapter acceptance
The adapter must reflect accepted capabilities for the selected device and format, without inventing missing topology, operations, or classical features. Validate both Qiskit and PennyLane compiler-entry cases through #2365. Keep service compilation guarantees distinct from serialization and compiler target facts; the underlying QDMI design remains a non-blocking release priority.
Checklist
If PR contains AI-assisted content:
🤖 *AI text below* 🤖(titles are exempt).