♻️ Redesign QDMI device management

[burgholzer]

🤖 AI text below 🤖

Summary

• replace the former FoMaC/driver/client layering with the public qdmi::DeviceRegistry, DeviceManager, Device, Job, Site, and Operation object model
• support the selected QDMI dependency through one private DeviceApi that owns the loaded library and stores the exact upstream function-pointer types with decltype(QDMI_...); there is no generic function table, virtual adapter hierarchy, or version-dispatch machinery
• use QDMI's QDMI_Device_Status, QDMI_Job_Status, and QDMI_Program_Format types directly in the C++ API instead of redefining equivalent MQT enums
• provide side-effect-free JSON/TOML discovery, layered configuration, relocatable built-in manifests, per-device session parameters, and lazy device opening
• retain independent sessions while reusing live provider libraries process-wide through a canonical-path weak cache; devices, children, sites, operations, and jobs own the shared state they require and may outlive their manager
• provide ID-keyed openAll()/open_all() bulk opening with successful devices and isolated per-device errors
• use device_id in Python bindings while retaining id in configuration and DeviceDefinition::id in C++
• document the complete public Python binding surface directly in bindings/ and regenerate the generated stubs
• centralize common device-target setup in mqt_configure_qdmi_device and migrate Python, Qiskit, neutral-atom helpers, tests, stubs, and documentation to the final QDMI API
• remove all legacy FoMaC paths, names, targets, the Driver singleton, the public client-interface implementation, global session configuration, and compatibility aliases
• vendor the current upstream toml++ single header at commit a43ad3787293f4a46b1d70c0924b5a25d10e79fc; its preamble embeds the MIT license
• maintain the implementation and its validation history in the checkout-independent ExecPlan .agent/plans/qdmi-integration-redesign.md

Motivation and impact

Device discovery is side-effect-free: parsing configuration does not execute provider code. A native library is loaded only when a caller explicitly opens a stable device ID. Authentication, base URLs, and custom QDMI session values are configured per device rather than process-wide.

MQT Core supports the one QDMI revision selected in cmake/ExternalDependencies.cmake. The private loader mirrors the QDMI C API directly and requires its complete symbol set. When the dependency evolves, this implementation evolves with it instead of preserving parallel ABI adapters or exposing a configuration dispatch marker.

DeviceManager remains the application-side definition/session factory. It is intentionally not called an orchestrator because QDMI orchestration has broader provider traversal and job-routing responsibilities. Registry access returns stable snapshots, concurrent opens are synchronized, and simultaneously live managers reuse provider libraries without sharing sessions. Operation queries also validate that every supplied site belongs to the same device state before passing handles to provider code.

Breaking changes

• C++ consumers use qdmi objects from qdmi/Device.hpp, qdmi/DeviceRegistry.hpp, and qdmi/DeviceManager.hpp, linking MQT::CoreQDMI.
• C++ status and format values are the QDMI-defined QDMI_Device_Status, QDMI_Job_Status, and QDMI_Program_Format types and constants. Python retains the convenient Device.Status, Job.Status, and ProgramFormat names for bindings of those QDMI enums.
• Python consumers use mqt.core.qdmi; neutral-atom conversion lives in mqt.core.na.qdmi.
• Python DeviceDefinition construction and property access use device_id. Configuration and C++ continue to use id.
• Devices can be opened individually with open(device_id, session_overrides) or in bulk with open_all(). Bulk opening returns successful devices and errors keyed by stable ID.
• Per-device session values belong in the definition or one open() override. Provider-specific values use QDMI custom session slots rather than a global project_id field.
• The configuration abi field is no longer needed because MQT Core supports one selected QDMI dependency.
• The former FoMaC namespace/module/target, Driver API, dynamic-library registration method, and public QDMI client-session ABI are removed without compatibility aliases for v4.

The expanded UPGRADING.md includes before/after C++ and Python examples, direct QDMI enum usage, bulk and selective opening, configuration precedence, per-device authentication, object lifetime behavior, and static/relocatable packaging guidance. Historical release entries continue to use the FoMaC name that applied when they were published.

Mainline integration

The branch is rebased onto main through #1909, including the MLIR Python bindings and DDSIM QIR execution support from #1815 and the ExecPlan process refinements from #1907 and #1908. The QIR execution test now lives under the renamed QDMI test hierarchy, and the centralized bindings/patterns.txt retains both the MLIR and QDMI stub-generation rules. The later #1904 overlap was limited to adjacent changelog link definitions and preserves both entries.

Scope

This PR completes the configuration/registration foundation and the driver/client/FoMaC object-model merge for this stage. Runtime-owned configurable SC and NA device models remain separate work tracked by #1226 and #1331; the optional resource broker is also outside this core stage.

Verification

• complete debug native build
• full native suite: 1,464 tests passed (two expected unsupported-property skips)
• full Python suite: 358 passed, 3 expected Qiskit-version skips
• latest QDMI specification selection: 64 tests passed, with two expected unsupported-property skips
• focused object-model suite: 165 tests passed, covering exact QDMI function-pointer signatures, child/session/job lifetime, cleanup, malformed values, direct enum forwarding, process-wide canonical library reuse, and cross-device site rejection
• focused manager/registry suite: 20 tests passed, covering configuration validation, definition snapshots, concurrent access, independent sessions, incomplete symbol sets, disabled runtime replacement, and partial bulk-open failures
• focused Python QDMI and estimator suite: 171 tests passed, including integer custom-job parameters
• all translation units from the first clang-tidy report pass the reported checks with Clang 22.1.8 and warnings treated as errors
• generated Python stubs via uvx nox -s stubs
• warning-as-error documentation build
• uvx nox -s lint
• moved-install-tree C++ consumer successfully discovered and opened mqt.ddsim.default, mqt.sc.default, and mqt.na.default
• git diff --check, naming/wording audits, and tracked/ignored artifact audits
• fresh-context critical review followed by a second implementation and validation pass

Related issues

Fixes #1358

Fixes #1362

Fixes #1363

[codecov]

Codecov Report

❌ Patch coverage is 93.92789% with 64 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/qdmi/DeviceRegistry.cpp	93.0%	21 Missing ⚠️
src/qdmi/Device.cpp	96.2%	17 Missing ⚠️
src/qdmi/DeviceState.cpp	89.6%	8 Missing ⚠️
include/mqt-core/na/qdmi/Device.hpp	71.4%	6 Missing ⚠️
src/qdmi/DeviceManager.cpp	89.2%	6 Missing ⚠️
include/mqt-core/qdmi/common/Common.hpp	0.0%	4 Missing ⚠️
python/mqt/core/plugins/qiskit/provider.py	85.7%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[MatthiasReumann]

This is a great start into the QDMI rewrite! 🚀

I've gone through the code a couple of times now and the following comments are the result of that.

However, currently this can't construct devices at runtime, right? As of now this simply loads dynamic libraries, I think 🤔.

[MatthiasReumann]

Something is fishy here.

impl_->libraries[key] is a std::weak_ptr - a non-owning reference to a shared_ptr. The DeviceManager (or the DeviceManager::Impl) class should be owning the device shared_ptr. .lock()ing and assigning the newly created shared_ptr only works (as far as I can tell) because the Device class below holds the same shared_ptr. I am pretty sure this function wouldn't work anymore if the Device takes a const pointer instead of a smart pointer.

[MatthiasReumann]

The map in DeviceManager::Impl should probably store the unique_ptr mentioned above (or below).

[MatthiasReumann]

I am also not really sure if I like the mutex here. I get where this is coming from - thread safety - but I think one really has to think this through if this would be enough.

[MatthiasReumann]

@coderabbitai Could you please evaluate why a weak_ptr is used here and if this is correct?

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
fd -HI "DeviceManager.hpp" 
fd -HI "Device.hpp" src/qdmi || true
fd -HI "DeviceApi.hpp" || true
echo "---DeviceManager.hpp---"
fd "DeviceManager.hpp" -x cat -n {}
echo "---Device.hpp (qdmi)---"
fd "Device.hpp" -p "qdmi" -x cat -n {}
echo "---DeviceApi.hpp---"
fd "DeviceApi.hpp" -x cat -n {}

Length of output: 4767

---

🏁 Script executed:

#!/bin/bash
set -e
fd "Device.hpp" -x cat -n {}
echo "---DeviceApi.hpp---"
cat -n src/qdmi/DeviceApi.hpp
echo "---DeviceApi.cpp (if exists)---"
fd "DeviceApi.cpp" -x cat -n {}

Length of output: 89512

---

@MatthiasReumann Good catch — let me walk through what's actually happening and where I agree/disagree.

Why weak_ptr is used (the intent)

impl_->libraries is meant to be a non-owning cache keyed by library path + prefix, so that:

• Multiple open() calls for the same underlying .so/.dll (e.g., two sessions of the same device, or child devices sharing a parent library) reuse a single dlopen'd DeviceApi instance instead of loading it multiple times.
• The library is automatically unloaded (dlclose) once nothing needs it anymore — DeviceManager doesn't want to force-keep every library loaded forever just because it was opened once.

The real owner of the shared_ptr<DeviceApi> is not DeviceManager at all — it's detail::DeviceState::api (see DeviceApi.hpp Line 97), which is reached via Device::state_ → DeviceState. So the chain is:

Device --(shared_ptr)--> DeviceState --(shared_ptr)--> DeviceApi

DeviceManager::Impl::libraries[key] only ever holds a weak_ptr, so as soon as the last Device/Job using that library is destroyed, DeviceState's destructor releases its shared_ptr<DeviceApi>, the refcount hits zero, the library is dlclose'd, and the cache entry silently becomes expired (lazily replaced on the next open()).

Is this correct?

Today, yes — mechanically it works because DeviceState is the sole strong owner. But your instinct is right that this is fragile: correctness here depends entirely on an invariant that isn't enforced or even documented anywhere near DeviceManager.cpp — it just happens to hold because of how DeviceState/Device are implemented elsewhere. If someone later changes Device/DeviceState to not hold the shared_ptr (e.g., a raw pointer optimization, or if DeviceState::api were made a weak_ptr too "for symmetry"), this cache would break with no compile-time signal — library in open() would be constructed, immediately go out of scope with no other owner, and be unloaded from under the returned Device. I'd suggest adding a comment above the libraries member (in DeviceManager::Impl) stating explicitly: "non-owning cache; DeviceState/Device must hold the real shared_ptr<DeviceApi>." That at least makes the contract discoverable for future refactors.

A more concrete, separate bug your mutex comment surfaced

The mutex in DeviceManager::Impl only guards libraries. It does not guard registry (DeviceRegistry::definitions_, a plain std::vector):

• DeviceManager::registerDevice / unregisterDevice call straight into impl_->registry.registerDevice/unregisterDevice with no lock at all.
• DeviceManager::open reads impl_->registry.definitions() unguarded (the scoped_lock is taken only afterward, around the libraries map access).

So if one thread calls open() while another calls registerDevice()/unregisterDevice() on the same DeviceManager, that's an unguarded concurrent read/write on std::vector, which is a real data race/UB, not just a theoretical concern. If DeviceManager is meant to be safely usable from multiple threads (which the presence of a mutex implies), this needs to be fixed too, e.g.:

const std::vector<DeviceDefinition>& DeviceManager::definitions() const {
  const std::scoped_lock lock(impl_->mutex);
  return impl_->registry.definitions();  // NB: returning a ref out of a locked scope is itself unsafe under mutation
}

void DeviceManager::registerDevice(DeviceDefinition definition, const bool replace) {
  const std::scoped_lock lock(impl_->mutex);
  impl_->registry.registerDevice(std::move(definition), replace);
}

Note that even this isn't quite enough for definitions()/open(), since returning a reference to (or iterating) a mutable vector after releasing the lock is unsafe if another thread can mutate it concurrently — you'd likely want open() to copy out the matching DeviceDefinition while holding the lock, rather than holding an iterator/reference into definitions_ past the lock's scope.

So, to directly answer your question: the weak_ptr choice for the library cache is intentional and works today, but it's an implicit, undocumented invariant rather than a locally-verifiable one — worth a comment at minimum. The mutex, as currently scoped, doesn't make DeviceManager thread-safe overall since it never protects registry/definitions_, which is the concrete issue underlying your third comment.

Want me to open a follow-up issue for the registry/definitions_ locking gap, or would you rather fold it into this PR?

---

✏️ Learnings added

Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1901
File: src/qdmi/DeviceManager.cpp:659-667
Timestamp: 2026-07-15T08:07:16.502Z
Learning: In munich-quantum-toolkit/core, `src/qdmi/DeviceManager.cpp`'s `DeviceManager::Impl::libraries` map intentionally stores `std::weak_ptr<detail::DeviceApi>` as a non-owning cache keyed by (library path, prefix); the true strong owner of the loaded library is `detail::DeviceState::api` (a `std::shared_ptr<DeviceApi>`, defined in `src/qdmi/DeviceApi.hpp`), reached transitively via `Device::state_` -> `DeviceState`. This lets the library auto-unload via `dlclose`/`FreeLibrary` once the last `Device`/`Job` referencing it is destroyed. This is a fragile, non-local invariant: if `DeviceState`/`Device` ever stop holding a strong `shared_ptr<DeviceApi>`, the weak cache in `DeviceManager` would break silently.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[MatthiasReumann]

@burgholzer I think this is quite interesting. Maybe something to think about whether the DeviceManager should be the only instance managing devices. Personally, I am not 100% sure if I would like to keep the shared_ptr; although the arguments are somewhat convincing. In the end it's a design decision.

[burgholzer]

I'll add some more reasoning from my iteration later. Right now, I am rather convinced this is a good design as it is.

[coderabbitai]

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

[burgholzer]

@coderabbitai @MatthiasReumann The weak ownership is intentional: the cache is only a reuse index, while live device and job state owns the loaded library. This lets multiple sessions reuse an already loaded provider without forcing the library to remain loaded after the final live object disappears.

I did find a real problem in the original implementation: the cache was manager-local, so separate managers could initialize the same library independently. It is now process-wide, keyed by canonical library path and symbol prefix, and guarded during lookup and construction. If no live object remains, the weak reference expires and reopening the provider is the expected behavior. A regression test covers reuse across simultaneously live managers and equivalent library paths.

Following the lifetime reasoning above, I did not put manager-owned unique_ptrs into this map. Doing so would couple library lifetime to one manager and conflict with the requirement that returned devices and jobs can outlive it.

The map has instead moved out of DeviceManager into the process-wide library loader and contains weak references. Strong ownership remains exclusively with live device and job state, so the final such object controls unloading.

Agreed that the original mutex was not a complete synchronization design. The responsibilities are now separated:

• a manager-local registry mutex protects definition snapshots, registration, unregistration, and lookup;

• definitions() returns a value snapshot instead of exposing a reference;

• a separate process-wide cache mutex protects canonical library lookup and initialization across all managers.

Session creation remains independent after the shared provider library has been obtained. Tests cover concurrent opens, concurrent registry access, independent sessions, and process-wide library reuse. This defines the manager’s guarantees without attempting to serialize all calls made through independent provider sessions.

[MatthiasReumann]

I think the rationale is getting clearer for me now, thanks! I have some question / thoughts though:

The weak ownership is intentional: the cache is only a reuse index, while live device and job state owns the loaded library.

Now I get why the DeviceApi is being used as a shared_ptr - this makes totally sense!

I did find a real problem in the original implementation: the cache was manager-local, so separate managers could initialize the same library independently. It is now process-wide, keyed by canonical library path and symbol prefix, and guarded during lookup and construction. If no live object remains, the weak reference expires and reopening the provider is the expected behavior. A regression test covers reuse across simultaneously live managers and equivalent library paths.

Just to make sure that I understand you fully here: Alternatively we could have made the DeviceManager a singleton, right? Probably depends on if we ever want to create DeviceManager's using different registries.

Following the lifetime reasoning above, I did not put manager-owned unique_ptrs into this map. Doing so would couple library lifetime to one manager and conflict with the requirement that returned devices and jobs can outlive it.

I think we should then probably find a more suitable name than DeviceManager. I don't know, something like DeviceExplorer? Following we should document how to best use the DeviceManager. Currently, I imagine something like

auto findAllDevices(....) {
  DeviceManager m(...); 
  return m.openAll(...);
}

[burgholzer]

@MatthiasReumann I am through with another iteration here. Feel free to take another look.
Note that this does not yet touch the SC and NA devices and makes them configurable. That would be a follow-up.
I still believe this is shaping up quite nicely.

I may try to split this into two changes, one that is v3.x compatible but adds the configurability and a separate one that contains the breaking change of dropping the client interface and combining driver and FoMaC.
That also might be a nice separation of concerns.

Also pinging @marcelwa @flowerthrower @ystade here for awareness.
If you want a very quick overview of the changes, read https://github.com/munich-quantum-toolkit/core/blob/agent/qdmi-integration-redesign/UPGRADING.md#qdmi-device-management-has-been-redesigned.

[MatthiasReumann]

Solid improvements! 🚀

I've left two minor requests and some questions / thoughts in the open discussion above.

Otherwise, I've nothing to complain than we should probably add a bit more documentation on the caching, shared_ptr, lifetime logic to the respective classes. I am pretty sure Codex can help with that ;)

[MatthiasReumann]

.hpp file in src folder

[MatthiasReumann]

This file still uses trailing return types.