✨ Instantiate Device from dynamic QDMI device library
#1381
Conversation
marcelwa
added
usability
There was a problem hiding this comment.
Pull request overview
This PR refactors the dynamic device library loading API to return the newly created Device object instead of void, improving usability by allowing callers to immediately obtain a device handle for backend creation.
Key changes:
- Modified
addDynamicDeviceLibraryto returnQDMI_Deviceinstead ofvoid - Added
Session::Device::fromQDMIDevicefactory method for creating Device objects from raw handles - Updated Python bindings to return
Deviceobjects with improved documentation and usage examples
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
include/mqt-core/qdmi/Driver.hpp |
Updated addDynamicDeviceLibrary signature to return QDMI_Device and added documentation for the return value |
src/qdmi/Driver.cpp |
Implemented return of newly created device pointer from addDynamicDeviceLibrary |
include/mqt-core/fomac/FoMaC.hpp |
Added fromQDMIDevice static factory method to enable Device construction from raw handles in bindings |
bindings/fomac/fomac.cpp |
Updated Python binding to capture and return Device object using the new factory method |
python/mqt/core/fomac.pyi |
Updated type stub with new return type and enhanced documentation with usage examples |
test/qdmi/test_driver.cpp |
Added test verifying the function returns a valid, usable device pointer |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe pull request modifies Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe return type of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
bindings/fomac/fomac.cpp(2 hunks)include/mqt-core/fomac/FoMaC.hpp(1 hunks)include/mqt-core/qdmi/Driver.hpp(1 hunks)python/mqt/core/fomac.pyi(2 hunks)src/qdmi/Driver.cpp(1 hunks)test/qdmi/test_driver.cpp(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.820Z
Learning: In the munich-quantum-toolkit/core repository, duplication of QDMI-related macros (such as IS_INVALID_ARGUMENT) across device implementations (e.g., in src/qdmi/sc/Device.cpp and src/qdmi/dd/Device.cpp) is acceptable as a temporary measure. The preferred long-term solution is to upstream these macros to the QDMI repository rather than creating local shared headers, so they can be reused across all dependent projects.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/error_handling_test.cpp:118-194
Timestamp: 2025-11-05T07:42:45.507Z
Learning: In the munich-quantum-toolkit/core QDMI device API, session parameters can only be set before calling `device_session_init()`. Once a session is initialized, any attempt to set a parameter returns `QDMI_ERROR_BADSTATE`. Since `SessionGuard` (in test/qdmi/dd/helpers/test_utils.hpp) automatically initializes the session in its constructor, tests that need to verify session parameter setting behavior before initialization must allocate a separate uninitialized session rather than reusing the `SessionGuard`'s session.
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/test_qdmi_qiskit_backend.py:0-0
Timestamp: 2025-11-04T14:28:32.371Z
Learning: In the munich-quantum-toolkit/core repository, at least one FoMaC device is always available during testing, so skip logic for missing devices in QDMI Qiskit backend tests is not necessary.
📚 Learning: 2025-12-07T09:10:31.820Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.820Z
Learning: In the munich-quantum-toolkit/core repository, duplication of QDMI-related macros (such as IS_INVALID_ARGUMENT) across device implementations (e.g., in src/qdmi/sc/Device.cpp and src/qdmi/dd/Device.cpp) is acceptable as a temporary measure. The preferred long-term solution is to upstream these macros to the QDMI repository rather than creating local shared headers, so they can be reused across all dependent projects.
Applied to files:
include/mqt-core/fomac/FoMaC.hppinclude/mqt-core/qdmi/Driver.hpp
📚 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:
include/mqt-core/fomac/FoMaC.hppinclude/mqt-core/qdmi/Driver.hppsrc/qdmi/Driver.cppbindings/fomac/fomac.cpptest/qdmi/test_driver.cpp
📚 Learning: 2025-10-17T11:09:50.147Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/test_qdmi_qiskit_site_handling.py:162-173
Timestamp: 2025-10-17T11:09:50.147Z
Learning: For QDMI tests that reference a specific named device via fixture (e.g., "MQT NA Default QDMI Device"), it is acceptable to hard-code device-specific values such as zone counts and indices, as these tests are intentionally coupled to that device's structure and serve as structural invariant checks.
Applied to files:
include/mqt-core/fomac/FoMaC.hpptest/qdmi/test_driver.cpp
📚 Learning: 2025-11-04T14:28:32.371Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/test_qdmi_qiskit_backend.py:0-0
Timestamp: 2025-11-04T14:28:32.371Z
Learning: In the munich-quantum-toolkit/core repository, at least one FoMaC device is always available during testing, so skip logic for missing devices in QDMI Qiskit backend tests is not necessary.
Applied to files:
include/mqt-core/fomac/FoMaC.hpp
📚 Learning: 2025-11-05T07:42:45.507Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/error_handling_test.cpp:118-194
Timestamp: 2025-11-05T07:42:45.507Z
Learning: In the munich-quantum-toolkit/core QDMI device API, session parameters can only be set before calling `device_session_init()`. Once a session is initialized, any attempt to set a parameter returns `QDMI_ERROR_BADSTATE`. Since `SessionGuard` (in test/qdmi/dd/helpers/test_utils.hpp) automatically initializes the session in its constructor, tests that need to verify session parameter setting behavior before initialization must allocate a separate uninitialized session rather than reusing the `SessionGuard`'s session.
Applied to files:
include/mqt-core/qdmi/Driver.hppbindings/fomac/fomac.cpptest/qdmi/test_driver.cpp
🧬 Code graph analysis (5)
python/mqt/core/fomac.pyi (5)
include/mqt-core/fomac/FoMaC.hpp (1)
Device(614-614)src/qdmi/dd/Device.cpp (1)
Device(149-151)include/mqt-core/na/fomac/Device.hpp (1)
Device(88-88)src/na/fomac/Device.cpp (1)
Device(579-580)python/mqt/core/na/fomac.pyi (1)
Device(17-114)
include/mqt-core/qdmi/Driver.hpp (1)
test/qdmi/test_driver.cpp (1)
config(52-60)
src/qdmi/Driver.cpp (2)
include/mqt-core/qdmi/Driver.hpp (1)
libName(447-450)test/qdmi/test_driver.cpp (1)
config(52-60)
bindings/fomac/fomac.cpp (2)
src/qdmi/dd/Device.cpp (2)
get(152-158)get(152-152)test/qdmi/test_driver.cpp (1)
config(52-60)
test/qdmi/test_driver.cpp (2)
src/qdmi/sc/Device.cpp (2)
get(50-56)get(50-50)src/qdmi/Driver.cpp (2)
QDMI_device_query_device_property(513-521)QDMI_device_query_device_property(513-516)
🪛 Cppcheck (2.18.0)
src/qdmi/Driver.cpp
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
⏰ 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). (2)
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
🔇 Additional comments (5)
include/mqt-core/fomac/FoMaC.hpp (1)
615-624: LGTM! Factory method provides clean binding-oriented construction.The static factory correctly uses the existing private constructor pattern. Consider whether a null-check on the
deviceparameter is needed here or if it's intentionally left to callers/downstream code.src/qdmi/Driver.cpp (1)
388-395: LGTM! Clean implementation returning the new device handle.The implementation correctly returns the raw pointer from the newly emplaced device. The lifetime is properly managed by the
Driversingleton'sdevices_vector.include/mqt-core/qdmi/Driver.hpp (1)
440-450: LGTM! API change is well-documented.The updated signature and documentation clearly communicate the new return value. Existing callers can safely ignore the return value if they don't need it, maintaining backward compatibility.
python/mqt/core/fomac.pyi (1)
300-341: LGTM! Type stub and documentation properly updated.The return type, docstring, and example all correctly reflect the new behavior of returning a
Deviceobject that can be used directly to create backends.bindings/fomac/fomac.cpp (1)
243-258: LGTM! Binding correctly wraps the QDMI device handle.The implementation properly captures the returned
QDMI_Deviceand wraps it using the newfromQDMIDevicefactory. The lack of null-check is acceptable sinceaddDynamicDeviceLibrarythrowsstd::runtime_erroron failure (per the documented contract inDriver.hpp), ensuringqdmiDeviceis non-null on successful return.
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
This looks great to me.
A Changelog entry would probably be justified.
Description
This pull request refactors the dynamic device library loading API to return the newly created device object, improving usability and clarity in both C++ and Python bindings. The changes ensure that when a dynamic device library is loaded, the caller immediately receives a usable device handle, simplifying subsequent backend creation and session management.
API and Binding Improvements
addDynamicDeviceLibraryinqdmi::Driverwas changed fromvoidtoQDMI_Device, so it now returns a pointer to the newly loaded device instead of just performing the action. (include/mqt-core/qdmi/Driver.hpp,src/qdmi/Driver.cpp) [1] [2]add_dynamic_device_librarywas updated to return aDeviceobject, with documentation and usage examples reflecting the new return value for easier backend creation. (bindings/fomac/fomac.cpp,python/mqt/core/fomac.pyi) [1] [2] [3] [4]Device Construction and Usability
fromQDMIDeviceto theSession::Deviceclass, allowing creation of aDeviceobject from a raw device handle, which is necessary for bindings where direct token construction is not accessible. (include/mqt-core/fomac/FoMaC.hpp)Testing
addDynamicDeviceLibraryreturns a valid device pointer and that the device is usable, improving reliability and regression coverage. (test/qdmi/test_driver.cpp)Checklist: