🎨 Reorganize QDMI Codebase #1444
Conversation
ystade
added
refactor
|
@coderabbitai Could you please update the PR description while keeping the checklist at the bottom. Please mention that this pR extracts the best practices from #1347. Finally, please close #1347 with a reference to this PR. Thanks! |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@ystade I'll help you update the PR description and close #1347. Let me first gather information about PR #1347 to understand the best practices being extracted. 🏁 Script executed: Length of output: 2229 🏁 Script executed: Length of output: 164 @ystade I've completed all the requested tasks: ✅ Updated PR #1444 description - Added a description mentioning that this PR reorganizes the QDMI codebase and extracts the best practices from #1347, with the checklist maintained at the bottom. ✅ Closed PR #1347 - Closed with a comment referencing this PR (#1444). You can view the updated description at #1444 🧠 Learnings used |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughReorganizes QDMI into subdirectories (common, devices, driver); adds a reusable Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 5
🤖 Fix all issues with AI agents
In @bindings/fomac/fomac.cpp:
- Line 13: The include uses a relative filesystem path; update the include
directive to use the include-directory-relative form used elsewhere by replacing
"../../include/mqt-core/qdmi/driver/Driver.hpp" with "qdmi/driver/Driver.hpp" so
the file imports Driver.hpp via the project include root (affecting the include
at the top of bindings/fomac/fomac.cpp that references Driver.hpp).
In @include/mqt-core/qdmi/devices/common/Common.hpp:
- Around line 35-38: The docstrings incorrectly refer to "Device class" for the
generic Singleton template; update the comments associated with the Singleton
destructor (~Singleton) and the instance accessor to use generic wording (e.g.,
"the Singleton instance" or "the singleton instance of the templated type")
instead of "Device class", ensuring both the destructor comment and the
"@returns the singleton instance..." comment reference the templated Singleton
rather than a Device-specific class.
- Around line 28-30: The Singleton currently defines move constructor and move
assignment (Singleton(Singleton&&) and Singleton& operator=(Singleton&&)) which
is semantically unsafe for a singleton; delete both move operations (set them
=delete) just like the copy constructor and copy assignment are deleted so the
singleton instance cannot be moved or invalidated; update the Singleton class
declaration to remove/mark the move ctor and move-assignment as deleted, leaving
get() and Concrete construction unchanged.
In @src/fomac/FoMaC.cpp:
- Line 13: Replace the inconsistent relative include in FoMaC.cpp with the
project's clean include path; specifically change the include directive that
references "../../include/mqt-core/qdmi/devices/common/Common.hpp" to use
"qdmi/devices/common/Common.hpp" so it matches other sources that link the same
QDMI targets (locate the include in FoMaC.cpp and update the path accordingly).
In @test/fomac/test_fomac.cpp:
- Line 11: The include directive is a fragile filesystem-relative path; replace
the include string "../../include/mqt-core/qdmi/devices/common/Common.hpp" with
the project-standard include "qdmi/devices/common/Common.hpp" for the FoMaC
module occurrences (the include in FoMaC.cpp and the include in test_fomac.cpp)
so the test and source use the same include-directory-relative pattern as the
rest of the codebase.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (59)
bindings/fomac/fomac.cppbindings/na/register_fomac.cppinclude/mqt-core/fomac/FoMaC.hppinclude/mqt-core/na/fomac/Device.hppinclude/mqt-core/qdmi/devices/common/Common.hppinclude/mqt-core/qdmi/devices/dd/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/na/Generator.hppinclude/mqt-core/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/sc/Generator.hppinclude/mqt-core/qdmi/driver/Driver.hppsrc/fomac/FoMaC.cppsrc/na/fomac/Device.cppsrc/qdmi/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txtsrc/qdmi/devices/common/CMakeLists.txtsrc/qdmi/devices/common/Common.cppsrc/qdmi/devices/dd/CMakeLists.txtsrc/qdmi/devices/dd/Device.cppsrc/qdmi/devices/na/App.cppsrc/qdmi/devices/na/CMakeLists.txtsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/na/DynDevice.cppsrc/qdmi/devices/na/Generator.cppsrc/qdmi/devices/sc/App.cppsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/sc/Device.cppsrc/qdmi/devices/sc/DynDevice.cppsrc/qdmi/devices/sc/Generator.cppsrc/qdmi/driver/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/fomac/test_fomac.cpptest/na/CMakeLists.txttest/qdmi/CMakeLists.txttest/qdmi/devices/CMakeLists.txttest/qdmi/devices/dd/CMakeLists.txttest/qdmi/devices/dd/concurrency_test.cpptest/qdmi/devices/dd/device_properties_test.cpptest/qdmi/devices/dd/device_status_test.cpptest/qdmi/devices/dd/error_handling_test.cpptest/qdmi/devices/dd/helpers/circuits.hpptest/qdmi/devices/dd/helpers/test_utils.cpptest/qdmi/devices/dd/helpers/test_utils.hpptest/qdmi/devices/dd/job_lifecycle_test.cpptest/qdmi/devices/dd/job_parameters_test.cpptest/qdmi/devices/dd/results_probabilities_test.cpptest/qdmi/devices/dd/results_sampling_test.cpptest/qdmi/devices/dd/results_statevector_test.cpptest/qdmi/devices/dd/session_lifecycle_test.cpptest/qdmi/devices/na/CMakeLists.txttest/qdmi/devices/na/test_app.cpptest/qdmi/devices/na/test_device.cpptest/qdmi/devices/na/test_generator.cpptest/qdmi/devices/sc/CMakeLists.txttest/qdmi/devices/sc/test_app.cpptest/qdmi/devices/sc/test_device.cpptest/qdmi/devices/sc/test_generator.cpptest/qdmi/driver/CMakeLists.txttest/qdmi/driver/test_driver.cpp
💤 Files with no reviewable changes (1)
- test/na/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (18)
📓 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.836Z
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/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.
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.
📚 Learning: 2025-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
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.hppbindings/fomac/fomac.cppsrc/qdmi/devices/na/CMakeLists.txttest/fomac/test_fomac.cppinclude/mqt-core/na/fomac/Device.hppsrc/qdmi/devices/dd/Device.cppbindings/na/register_fomac.cppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/common/Common.cppsrc/qdmi/devices/common/CMakeLists.txtsrc/qdmi/devices/sc/App.cppsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txttest/qdmi/CMakeLists.txtsrc/qdmi/devices/sc/Device.cpptest/qdmi/driver/test_driver.cpptest/qdmi/devices/sc/test_generator.cppsrc/qdmi/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/qdmi/devices/na/test_device.cpptest/qdmi/devices/CMakeLists.txtsrc/qdmi/devices/dd/CMakeLists.txtinclude/mqt-core/qdmi/devices/common/Common.hppsrc/na/fomac/Device.cppsrc/qdmi/devices/na/App.cpptest/qdmi/driver/CMakeLists.txtinclude/mqt-core/qdmi/devices/dd/Device.hppsrc/qdmi/devices/na/Generator.cppsrc/fomac/FoMaC.cppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/sc/Device.hppsrc/qdmi/devices/sc/Generator.cpptest/qdmi/devices/na/test_generator.cppsrc/qdmi/driver/CMakeLists.txt
📚 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.hppbindings/fomac/fomac.cppsrc/qdmi/devices/na/CMakeLists.txttest/fomac/test_fomac.cppinclude/mqt-core/na/fomac/Device.hppsrc/qdmi/devices/dd/Device.cppbindings/na/register_fomac.cppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/common/Common.cppsrc/qdmi/devices/common/CMakeLists.txtsrc/qdmi/devices/sc/App.cppsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txttest/qdmi/CMakeLists.txtsrc/qdmi/devices/sc/Device.cpptest/qdmi/driver/test_driver.cpptest/qdmi/devices/sc/test_generator.cppsrc/qdmi/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/qdmi/devices/na/test_device.cpptest/qdmi/devices/CMakeLists.txtsrc/qdmi/devices/dd/CMakeLists.txtsrc/na/fomac/Device.cppsrc/qdmi/devices/na/App.cpptest/qdmi/driver/CMakeLists.txtsrc/qdmi/devices/na/Generator.cppsrc/fomac/FoMaC.cppsrc/qdmi/devices/sc/Generator.cpptest/qdmi/devices/na/test_generator.cppsrc/qdmi/driver/CMakeLists.txt
📚 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.hppsrc/qdmi/devices/na/CMakeLists.txttest/fomac/test_fomac.cppsrc/qdmi/devices/dd/Device.cppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/common/Common.cpptest/qdmi/CMakeLists.txtsrc/qdmi/devices/sc/Device.cpptest/qdmi/devices/sc/test_generator.cppsrc/qdmi/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/qdmi/devices/na/test_device.cpptest/qdmi/devices/CMakeLists.txtsrc/qdmi/devices/dd/CMakeLists.txttest/qdmi/driver/CMakeLists.txtinclude/mqt-core/qdmi/devices/dd/Device.hppsrc/qdmi/devices/na/Generator.cppsrc/fomac/FoMaC.cppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/sc/Device.hpptest/qdmi/devices/na/test_generator.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:
bindings/fomac/fomac.cpptest/fomac/test_fomac.cppsrc/qdmi/devices/dd/Device.cppbindings/na/register_fomac.cppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/sc/Device.cpptest/qdmi/driver/CMakeLists.txtsrc/fomac/FoMaC.cpp
📚 Learning: 2025-12-15T01:54:18.630Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/dd/register_matrix_dds.cpp:64-109
Timestamp: 2025-12-15T01:54:18.630Z
Learning: In munich-quantum-toolkit/core, for C++ binding files under the bindings directory (any nested path), add Python docstrings directly in the binding code using the R"pb(...)pb" syntax. Do not maintain docstrings in stub files (.pyi); instead, keep stubs auto-generated by the script: bindings/generate-stubs.sh. This replaces the previous pybind11 approach where docstrings were kept in stubs. Ensure the binding code reflects accurate Python docstrings and that the stub generation script runs to refresh the .pyi files.
Applied to files:
bindings/fomac/fomac.cppbindings/na/register_fomac.cpp
📚 Learning: 2025-12-19T00:05:41.711Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:111-116
Timestamp: 2025-12-19T00:05:41.711Z
Learning: In nanobind bindings (munich-quantum-toolkit/core), lifetime management differs from pybind11. After migrating to nanobind, nb::keep_alive<nurse, patient>() does not exist. For methods that return objects which reference the parent’s internal state (e.g., Session::getDevices returning Device objects dependent on the Session), use nb::rv_policy::reference_internal when binding those methods. This ensures the parent (Session) stays alive while its children (Device) exist. Apply this pattern in binding code under bindings/ (e.g., bindings/fomac/fomac.cpp and similar) wherever return values depend on the parent.
Applied to files:
bindings/fomac/fomac.cppbindings/na/register_fomac.cpp
📚 Learning: 2025-12-21T22:35:00.998Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:348-364
Timestamp: 2025-12-21T22:35:00.998Z
Learning: In munich-quantum-toolkit/core, for nanobind bindings, when a parameter has a mutable default (e.g., a vector or container default), use .sig("...") on that parameter (e.g., some_arg.sig("...") = std::vector<...>{}) to prevent exposing mutable defaults in the Python API. This is preferred over removing .sig(...) since removing it would still show the default in the stub. Apply this pattern to similar binding parameters across the repository (bindings/**/, not just this file) to keep Python bindings safe from linter warnings and maintainable.
Applied to files:
bindings/fomac/fomac.cppbindings/na/register_fomac.cpp
📚 Learning: 2025-12-22T01:25:14.123Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:14.123Z
Learning: Prefer calling nanobind iterator factory functions like make_key_iterator and make_iterator without the nb:: qualifier in code within this project. Rely on Argument-Dependent Lookup (ADL) as suggested by clang-tidy to resolve these unqualified calls, which improves readability and aligns with the repository's style. Ensure that the calls are discoverable via ADL and that the relevant headers are included so the unqualified names are found without explicit namespace prefixes.
Applied to files:
bindings/fomac/fomac.cppbindings/na/register_fomac.cpp
📚 Learning: 2025-12-28T17:14:53.890Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: src/qdmi/na/CMakeLists.txt:31-38
Timestamp: 2025-12-28T17:14:53.890Z
Learning: In the munich-quantum-toolkit/core repository, the NA device generator target (mqt-core-qdmi-na-device-gen) is intentionally propagated to MQT_CORE_TARGETS via list(APPEND) because it's publicly linked to the NA device library (the NA device uses a public function from the generator). The SC device generator is not propagated because it has no such dependency with the SC device library.
Applied to files:
src/qdmi/devices/na/CMakeLists.txtinclude/mqt-core/na/fomac/Device.hppsrc/qdmi/devices/dd/Device.cppbindings/na/register_fomac.cppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/common/CMakeLists.txtsrc/qdmi/devices/sc/App.cppsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txttest/qdmi/CMakeLists.txtsrc/qdmi/devices/sc/Device.cpptest/qdmi/devices/sc/test_generator.cppsrc/qdmi/CMakeLists.txttest/qdmi/devices/na/test_device.cpptest/qdmi/devices/CMakeLists.txtsrc/qdmi/devices/dd/CMakeLists.txtsrc/na/fomac/Device.cppsrc/qdmi/devices/na/App.cpptest/qdmi/driver/CMakeLists.txtsrc/qdmi/devices/na/Generator.cppinclude/mqt-core/qdmi/devices/na/Device.hppsrc/qdmi/devices/sc/Generator.cpptest/qdmi/devices/na/test_generator.cppsrc/qdmi/driver/CMakeLists.txt
📚 Learning: 2026-01-04T23:54:33.540Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: src/qdmi/na/CMakeLists.txt:179-184
Timestamp: 2026-01-04T23:54:33.540Z
Learning: In the munich-quantum-toolkit/core repository, MSVC link-time code generation (LTCG) must be disabled for the NA device dynamic library target (mqt-core-qdmi-na-device-dyn) on Windows to avoid link errors and missing symbols. This issue affects both release-only builds and debug/release mixed builds, not just debug/release mixing scenarios.
Applied to files:
src/qdmi/devices/na/CMakeLists.txtsrc/qdmi/devices/common/CMakeLists.txtsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txttest/qdmi/CMakeLists.txtsrc/qdmi/CMakeLists.txttest/qdmi/devices/na/test_device.cppsrc/qdmi/devices/dd/CMakeLists.txttest/qdmi/driver/CMakeLists.txtsrc/qdmi/devices/na/Generator.cppsrc/qdmi/driver/CMakeLists.txt
📚 Learning: 2025-12-05T15:57:39.701Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 3
File: lib/Conversion/CatalystQuantumToMQTOpt/CMakeLists.txt:22-25
Timestamp: 2025-12-05T15:57:39.701Z
Learning: The munich-quantum-toolkit projects (core and core-plugins-catalyst) use `file(GLOB_RECURSE ...)` patterns in CMakeLists.txt files to collect header files, following an established convention in the parent repository for consistency across the codebase.
Applied to files:
src/qdmi/devices/na/CMakeLists.txtsrc/qdmi/devices/common/CMakeLists.txtsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txtsrc/qdmi/CMakeLists.txtsrc/qdmi/devices/dd/CMakeLists.txt
📚 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:
src/qdmi/devices/dd/Device.cppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/sc/Device.cppinclude/mqt-core/qdmi/devices/dd/Device.hpp
📚 Learning: 2025-11-05T09:23:46.540Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: src/qdmi/dd/Device.cpp:492-521
Timestamp: 2025-11-05T09:23:46.540Z
Learning: In the munich-quantum-toolkit/core repository, the `qasm3::Importer::imports()` function is backwards compatible with OpenQASM 2 programs. Therefore, it can be used to parse both QASM2 and QASM3 program formats without requiring separate importers for each version.
Applied to files:
src/qdmi/devices/dd/Device.cpp
📚 Learning: 2025-11-04T14:26:25.420Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/conftest.py:11-19
Timestamp: 2025-11-04T14:26:25.420Z
Learning: In the munich-quantum-toolkit/core repository, Qiskit is always available as a dependency during testing, so import guards for qiskit-dependent imports in test files (e.g., test/python/qdmi/qiskit/*.py) are not necessary.
Applied to files:
src/qdmi/devices/dd/Device.cpp
📚 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:
src/qdmi/devices/CMakeLists.txtsrc/qdmi/CMakeLists.txt
📚 Learning: 2025-12-28T17:13:36.900Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: pyproject.toml:98-102
Timestamp: 2025-12-28T17:13:36.900Z
Learning: In the munich-quantum-toolkit/core project, scikit-build-core is intelligent enough to skip build targets listed in pyproject.toml that don't exist for a given platform, so platform-specific targets (like `-dyn` targets conditioned on `NOT WIN32`) can be unconditionally listed in `build.targets` without causing Windows build failures.
Applied to files:
src/qdmi/devices/CMakeLists.txt
📚 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:
src/qdmi/devices/CMakeLists.txt
🧬 Code graph analysis (3)
include/mqt-core/qdmi/devices/common/Common.hpp (1)
include/mqt-core/qdmi/driver/Driver.hpp (1)
instance(421-424)
include/mqt-core/qdmi/devices/dd/Device.hpp (2)
include/mqt-core/qdmi/devices/na/Device.hpp (2)
Device(72-72)Device(76-76)include/mqt-core/qdmi/devices/sc/Device.hpp (2)
Device(54-54)Device(58-58)
include/mqt-core/qdmi/devices/sc/Device.hpp (3)
src/qdmi/devices/dd/Device.cpp (1)
Device(149-151)src/qdmi/devices/na/Device.cpp (1)
Device(35-46)src/qdmi/devices/sc/Device.cpp (2)
Device(36-45)Device(46-49)
🪛 Cppcheck (2.19.0)
bindings/fomac/fomac.cpp
[information] 13-13: Include file
(missingIncludeSystem)
src/qdmi/devices/dd/Device.cpp
[information] 15-15: Include file
(missingInclude)
[information] 25-25: Include file
(missingInclude)
bindings/na/register_fomac.cpp
[information] 13-13: Include file
(missingInclude)
src/qdmi/devices/na/Device.cpp
[information] 15-15: Include file
(missingInclude)
[information] 17-17: Include file
(missingInclude)
[information] 18-18: Include file
(missingInclude)
src/qdmi/devices/common/Common.cpp
[information] 15-15: Include file
(missingInclude)
src/qdmi/devices/sc/App.cpp
[information] 11-11: Include file
(missingInclude)
src/qdmi/devices/sc/Device.cpp
[information] 15-15: Include file
(missingInclude)
[information] 17-17: Include file
(missingInclude)
[information] 18-18: Include file
(missingInclude)
test/qdmi/driver/test_driver.cpp
[information] 11-11: Include file
(missingInclude)
test/qdmi/devices/sc/test_generator.cpp
[information] 11-11: Include file
(missingInclude)
src/qdmi/driver/Driver.cpp
[information] 11-11: Include file
(missingInclude)
[information] 13-13: Include file
(missingInclude)
[information] 14-14: Include file
(missingInclude)
[information] 15-15: Include file
(missingInclude)
[information] 16-16: Include file
(missingInclude)
test/qdmi/devices/na/test_device.cpp
[information] 12-12: Include file
(missingInclude)
src/na/fomac/Device.cpp
[information] 15-15: Include file
(missingInclude)
src/qdmi/devices/na/App.cpp
[information] 11-11: Include file
(missingInclude)
src/qdmi/devices/na/Generator.cpp
[information] 15-15: Include file
(missingInclude)
src/qdmi/devices/sc/Generator.cpp
[information] 15-15: Include file
(missingInclude)
test/qdmi/devices/na/test_generator.cpp
[information] 11-11: Include file
(missingInclude)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Debug) / 🍎 macos-15 clang Debug
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (macos-15) / 🐍 macos-15
- GitHub Check: 🐍 Test (windows-2025) / 🐍 windows-2025
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Release) / 🍎 macos-15 clang Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🐍 Lint / 🚨 Lint
- GitHub Check: 🇨 Lint / 🚨 Lint
- GitHub Check: 🇨 Coverage / 📈 Coverage
🔇 Additional comments (43)
src/qdmi/devices/sc/App.cpp (1)
11-11: LGTM!Include path correctly updated to reflect the new
qdmi/devices/sc/directory structure.include/mqt-core/na/fomac/Device.hpp (1)
14-14: LGTM!Include path correctly updated to align with the new
qdmi/devices/na/directory structure.include/mqt-core/qdmi/devices/common/Common.hpp (1)
39-46: LGTM - intentional leak pattern for static deinitialization safety.The heap allocation with intentional leak is a well-known pattern to avoid the static deinitialization order fiasco. The comment clearly documents the rationale.
Note: This differs from
Driver::get()inDriver.hpp(line 420-423) which usesstatic Driver instance;. Consider whether consistent singleton patterns across the codebase would be beneficial.src/qdmi/devices/common/Common.cpp (1)
15-15: LGTM!Include path correctly updated to the new
qdmi/devices/common/location using a clean include path.include/mqt-core/fomac/FoMaC.hpp (1)
13-13: LGTM!Include path correctly updated using a relative path within the include tree.
bindings/na/register_fomac.cpp (1)
13-13: LGTM!Include path correctly updated to reflect the new
qdmi/devices/na/directory structure.src/qdmi/devices/sc/Generator.cpp (1)
15-15: LGTM!The include path update correctly reflects the new directory structure under
qdmi/devices/sc/.test/qdmi/devices/na/test_device.cpp (1)
12-12: LGTM!The include path update correctly aligns with the new header location under
qdmi/devices/na/.test/qdmi/driver/test_driver.cpp (1)
11-11: LGTM!The include path update correctly reflects the new header location under
qdmi/driver/.src/qdmi/devices/na/App.cpp (1)
11-11: LGTM!The include path update correctly reflects the new header location under
qdmi/devices/na/.src/qdmi/devices/CMakeLists.txt (1)
1-16: LGTM!The new CMakeLists.txt correctly:
- Includes the proper MIT license header
- Adds subdirectories for the reorganized device components (
common,dd,sc,na)- Propagates
MQT_CORE_TARGETSto the parent scope, consistent with the repository's target management patternBased on learnings, this structure aligns with how device targets are organized—the NA device generator is propagated via
MQT_CORE_TARGETSdue to its public linkage with the NA device library, while the SC generator is not propagated since it has no such dependency.test/qdmi/devices/sc/test_generator.cpp (1)
11-11: LGTM!The include path update to
"qdmi/devices/sc/Generator.hpp"correctly reflects the header reorganization into thedevices/subdirectory structure.src/qdmi/driver/Driver.cpp (1)
11-11: LGTM!The include path updates correctly reflect the header reorganization:
"qdmi/driver/Driver.hpp"for the Driver header moved to the driver subdirectory"qdmi/devices/common/Common.hpp"for the Common header moved to devices/commonThese are proper include-directory-relative paths consistent with the codebase structure.
Also applies to: 16-16
test/qdmi/devices/CMakeLists.txt (1)
9-11: No action needed – CMakeLists.txt is correct as written.The
test/qdmi/devices/common/subdirectory does not exist. The CMakeLists.txt file correctly adds all three existing subdirectories (dd, na, sc). The discrepancy with the AI summary is due to that summary being inaccurate, not an issue with this code.Likely an incorrect or invalid review comment.
src/na/fomac/Device.cpp (1)
15-15: LGTM!Include path correctly updated to reflect the new directory structure under
qdmi/devices/na/.test/qdmi/devices/na/test_generator.cpp (1)
11-11: LGTM!Include path correctly updated to match the relocated Generator header.
test/qdmi/CMakeLists.txt (1)
9-10: LGTM!Clean restructuring that delegates test target definitions to the appropriate subdirectories, consistent with the modular reorganization of the QDMI codebase.
src/qdmi/devices/na/Generator.cpp (1)
15-15: LGTM!Include path correctly updated to reflect the relocated Generator header under
qdmi/devices/na/.src/qdmi/devices/dd/Device.cpp (1)
15-15: LGTM!Include paths correctly updated to reflect the new directory structure:
- Device header relocated to
qdmi/devices/dd/- Common utilities relocated to
qdmi/devices/common/The
Device::get()calls throughout the file will now use the inheritedSingleton<Device>::get()method, which is consistent with the singleton pattern extraction in this PR.Also applies to: 25-25
src/qdmi/devices/na/Device.cpp (1)
15-18: LGTM! Header paths correctly updated to new directory structure.The include paths have been properly updated to reflect the reorganized codebase structure (
qdmi/devices/na/Device.hppandqdmi/devices/common/Common.hpp). TheDevice::get()calls throughout the file now correctly resolve to the inherited method fromSingleton<Device>.src/qdmi/devices/sc/Device.cpp (1)
15-18: LGTM! Header paths correctly updated to match the new directory structure.The include paths are properly updated to the reorganized locations. The
Device::get()accessor is now inherited fromSingleton<Device>viaCommon.hpp, maintaining the same functionality with a cleaner design.src/qdmi/devices/dd/CMakeLists.txt (2)
37-37: Header path correctly updated.
51-63: LGTM! CMake target handling improved with consistent pattern.The change from the previous
set()pattern tolist(APPEND)followed byset(...PARENT_SCOPE)outside theifblock is cleaner and consistent with the other device CMakeLists.txt files in this PR. This ensures proper target propagation regardless of whether the target was already defined.src/qdmi/devices/na/CMakeLists.txt (3)
22-24: Header path correctly updated for the generator target.
120-120: Header path correctly updated for the static device library.
164-164: Header path correctly updated for the dynamic device library.All header path updates in this file are consistent with the reorganized directory structure (
qdmi/devices/na/). The MSVC LTCG workaround (lines 180-185) is correctly preserved as expected. Based on learnings, the NA device generator target propagation vialist(APPEND)is intentional since the NA device publicly links to the generator.test/qdmi/driver/CMakeLists.txt (1)
9-19: The test configuration is well-structured with proper guards and dependencies. TheDYN_DEV_LIBScompile definition correctly embeds file paths from generator expressions, and Windows path handling works as expected.LoadLibraryA()(used on Windows) natively supports both forward and backward slashes in file paths, and the paths are properly embedded as string literals in the compile definition.src/qdmi/CMakeLists.txt (1)
9-14: Clean modular CMake structure.The delegation to subdirectories (
devicesanddriver) withPARENT_SCOPEpropagation ofMQT_CORE_TARGETSis a clean approach that aligns well with the reorganization goals. This allows each subdirectory to manage its own target definitions while ensuring they're properly exported.src/qdmi/devices/sc/CMakeLists.txt (3)
22-24: Header path updated consistently.The path change from
qdmi/sc/Generator.hpptoqdmi/devices/sc/Generator.hppaligns with the new directory structure.
117-118: Device header path updated for static library target.Path consistently updated to
qdmi/devices/sc/Device.hpp.
161-162: Device header path updated for dynamic library target.Path consistently updated to match the static library target.
src/qdmi/devices/common/CMakeLists.txt (1)
9-35: Well-structured common library target.The new
QDMICommontarget follows the established patterns in the codebase:
- Proper target guard with
if(NOT TARGET ...)- Clear separation of public (
qdmi::qdmi) and private (qdmi::qdmi_project_warnings) dependencies- Consistent
PARENT_SCOPEpropagation ofMQT_CORE_TARGETSThis provides a clean home for the extracted Singleton pattern and other shared utilities.
include/mqt-core/qdmi/devices/dd/Device.hpp (4)
29-29: Common header included for Singleton base.This include provides the
Singleton<T>template that theDeviceclass now inherits from.
35-37: Singleton pattern correctly applied.The class now inherits from
Singleton<Device>and declaresSingletonas a friend, allowing the base class to access the private constructor while maintaining the singleton guarantee.
71-71: Destructor correctly marked as override.Since
Singleton<Device>presumably has a virtual destructor, theoverridespecifier is appropriate here.
242-244: TheSingleton<Device>template correctly provides the staticget()method, andDeviceinherits it asDevice final : public Singleton<Device>. The constructor's call toqdmi::dd::Device::get().generateUniqueID()will work correctly sinceget()returns a reference to the Device singleton instance.include/mqt-core/qdmi/devices/na/Device.hpp (3)
23-23: Common header included for Singleton base.Consistent with other device headers.
31-33: Singleton pattern applied consistently with DD device.The inheritance from
Singleton<Device>and friend declaration follow the same pattern asqdmi::dd::Device.
76-76: Virtual destructor override defaulted.Appropriate for a class that doesn't need custom cleanup logic.
src/qdmi/driver/CMakeLists.txt (1)
9-35: Driver library target properly configured.The target configuration follows established patterns:
- Public link to
qdmi::qdmiandMQT::CoreQDMICommonexposes necessary interfaces- Private links to device libraries (NA, SC, DD) keep implementation details hidden
${CMAKE_DL_LIBS}correctly included for dynamic loading supportinclude/mqt-core/qdmi/devices/sc/Device.hpp (3)
22-22: Common header included for Singleton base.Consistent with other device headers.
30-32: Singleton pattern applied consistently with other devices.The inheritance and friend declaration follow the established pattern.
58-58: Non-defaulted destructor override is appropriate.Unlike the DD and NA devices which use
= default, the SC device has a custom destructor implementation (as seen insrc/qdmi/devices/sc/Device.cpp) that explicitly clears sessions to avoid segfaults during destruction. Theoverridespecifier correctly indicates this overrides the virtual destructor fromSingleton<Device>.
|
Apparently, codecov cannot include code lines in destructors in its analysis. Currently, codecov only complains about lines in destructors, which I cannot cover with tests. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
bindings/fomac/fomac.cppinclude/mqt-core/qdmi/devices/common/Common.hppsrc/fomac/FoMaC.cpptest/fomac/test_fomac.cpp
🧰 Additional context used
🧠 Learnings (9)
📓 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.836Z
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/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.
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.
📚 Learning: 2025-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
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:
src/fomac/FoMaC.cpptest/fomac/test_fomac.cppbindings/fomac/fomac.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:
src/fomac/FoMaC.cpptest/fomac/test_fomac.cppbindings/fomac/fomac.cpp
📚 Learning: 2025-12-05T15:57:39.701Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 3
File: lib/Conversion/CatalystQuantumToMQTOpt/CMakeLists.txt:22-25
Timestamp: 2025-12-05T15:57:39.701Z
Learning: The munich-quantum-toolkit projects (core and core-plugins-catalyst) use `file(GLOB_RECURSE ...)` patterns in CMakeLists.txt files to collect header files, following an established convention in the parent repository for consistency across the codebase.
Applied to files:
src/fomac/FoMaC.cpptest/fomac/test_fomac.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:
src/fomac/FoMaC.cpptest/fomac/test_fomac.cpp
📚 Learning: 2025-12-22T01:25:14.123Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:14.123Z
Learning: Prefer calling nanobind iterator factory functions like make_key_iterator and make_iterator without the nb:: qualifier in code within this project. Rely on Argument-Dependent Lookup (ADL) as suggested by clang-tidy to resolve these unqualified calls, which improves readability and aligns with the repository's style. Ensure that the calls are discoverable via ADL and that the relevant headers are included so the unqualified names are found without explicit namespace prefixes.
Applied to files:
bindings/fomac/fomac.cpp
📚 Learning: 2025-12-15T01:54:18.630Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/dd/register_matrix_dds.cpp:64-109
Timestamp: 2025-12-15T01:54:18.630Z
Learning: In munich-quantum-toolkit/core, for C++ binding files under the bindings directory (any nested path), add Python docstrings directly in the binding code using the R"pb(...)pb" syntax. Do not maintain docstrings in stub files (.pyi); instead, keep stubs auto-generated by the script: bindings/generate-stubs.sh. This replaces the previous pybind11 approach where docstrings were kept in stubs. Ensure the binding code reflects accurate Python docstrings and that the stub generation script runs to refresh the .pyi files.
Applied to files:
bindings/fomac/fomac.cpp
📚 Learning: 2025-12-19T00:05:41.711Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:111-116
Timestamp: 2025-12-19T00:05:41.711Z
Learning: In nanobind bindings (munich-quantum-toolkit/core), lifetime management differs from pybind11. After migrating to nanobind, nb::keep_alive<nurse, patient>() does not exist. For methods that return objects which reference the parent’s internal state (e.g., Session::getDevices returning Device objects dependent on the Session), use nb::rv_policy::reference_internal when binding those methods. This ensures the parent (Session) stays alive while its children (Device) exist. Apply this pattern in binding code under bindings/ (e.g., bindings/fomac/fomac.cpp and similar) wherever return values depend on the parent.
Applied to files:
bindings/fomac/fomac.cpp
📚 Learning: 2025-12-21T22:35:00.998Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:348-364
Timestamp: 2025-12-21T22:35:00.998Z
Learning: In munich-quantum-toolkit/core, for nanobind bindings, when a parameter has a mutable default (e.g., a vector or container default), use .sig("...") on that parameter (e.g., some_arg.sig("...") = std::vector<...>{}) to prevent exposing mutable defaults in the Python API. This is preferred over removing .sig(...) since removing it would still show the default in the stub. Apply this pattern to similar binding parameters across the repository (bindings/**/, not just this file) to keep Python bindings safe from linter warnings and maintainable.
Applied to files:
bindings/fomac/fomac.cpp
🧬 Code graph analysis (1)
include/mqt-core/qdmi/devices/common/Common.hpp (1)
include/mqt-core/qdmi/driver/Driver.hpp (1)
instance(421-424)
🪛 Cppcheck (2.19.0)
src/fomac/FoMaC.cpp
[information] 13-13: Include file
(missingInclude)
test/fomac/test_fomac.cpp
[information] 12-12: Include file
(missingInclude)
bindings/fomac/fomac.cpp
[information] 13-13: Include file
(missingInclude)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🐍 Test (windows-2025) / 🐍 windows-2025
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🐍 Test (macos-15) / 🐍 macos-15
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Lint / 🚨 Lint
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🇨 Lint / 🚨 Lint
🔇 Additional comments (4)
include/mqt-core/qdmi/devices/common/Common.hpp (1)
53-65: Documentation improvement noted.The docstring now correctly uses "compiler-specific extensions" (hyphenated compound adjective). The
unreachable()function implementation is correct.test/fomac/test_fomac.cpp (1)
12-12: Include path update aligns with the codebase reorganization.The updated include path
"qdmi/devices/common/Common.hpp"is consistent with the new header structure introduced in this PR. The CppcheckmissingIncludehint is a false positive—the header exists at the new location.bindings/fomac/fomac.cpp (1)
13-13: Include path correctly updated toqdmi/driver/Driver.hpp.This include is necessary for the
qdmi::Driver::get().addDynamicDeviceLibrary()call on line 450. The CppcheckmissingIncludehint is a false positive.Note: The AI summary mentioned this file includes
Common.hpp, but the actual change is toDriver.hpp, which is correct for the functionality used in this file.src/fomac/FoMaC.cpp (1)
13-13: Include path update is correct and consistent.The updated path
"qdmi/devices/common/Common.hpp"aligns with the codebase reorganization. This file correctly uses utilities fromCommon.hppsuch asqdmi::throwIfError,qdmi::unreachable, andqdmi::toString. The CppcheckmissingIncludehint is a false positive.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
include/mqt-core/qdmi/common/Common.hpp (2)
17-51: Missing required headers for C APIs used in this header (compile-break risk).This header uses
strlen,strncpy(_s), andmemcpybut doesn’t include<cstring>(and if you add anstd::abort()fallback, you’ll need<cstdlib>too). SinceCommon.hppis now widely included, this can surface as a hard build failure.Proposed fix
#pragma once #include <qdmi/client.h> #include <qdmi/constants.h> +#include <cstring> +#include <cstdlib> #include <string>
59-65:unreachable()should have a safe fallback on non-GNU/MSVC compilers.An empty
[[noreturn]]function can trigger warnings/errors and is brittle if another compiler is used.Proposed fix
[[noreturn]] inline void unreachable() { #ifdef __GNUC__ // GCC, Clang, ICC __builtin_unreachable(); #elif defined(_MSC_VER) // MSVC __assume(false); +#else + std::abort(); #endif }include/mqt-core/qdmi/devices/na/Device.hpp (1)
71-76: Resource cleanup won't occur due to intentional singleton leak.The
Singleton<Concrete>::get()pattern allocates instances withnewand intentionally leaks them (as documented in Common.hpp) to avoid static deinitialization order issues. Consequently,~Device()will never execute, and cleanup of thestd::unique_ptrmembers (sites_,operations_,sessions_) won't happen. This is a deliberate tradeoff for safety, but developers should be aware that any cleanup expected in the destructor will not run during normal program shutdown.
🤖 Fix all issues with AI agents
In @include/mqt-core/qdmi/devices/dd/Device.hpp:
- Around line 35-37: Replace the ill-formed friend declaration that currently
names just Singleton with a proper friend for the Singleton specialization so
Singleton<Device> can access Device() — e.g., change the friend declaration to
explicitly friend the Singleton<Device> specialization (or alternatively add a
template friend declaration template<typename> friend class Singleton) so the
Device constructor is accessible to Singleton<Device>.
In @include/mqt-core/qdmi/devices/na/Device.hpp:
- Around line 31-33: The friend declaration 'friend Singleton;' is invalid and
won’t grant Singleton<Device> access to Device’s private constructor; replace it
with a proper friend of the specialization or template (either friend class
Singleton<Device> to friend only the Device specialization, or template<typename
T> friend class Singleton to friend the whole template) so that
Singleton<Device>::get() can call Device::Device().
In @include/mqt-core/qdmi/devices/sc/Device.hpp:
- Around line 30-32: The friend declaration `friend Singleton;` is ill-formed —
make the Singleton friendship explicit for this instantiation by changing it to
`friend class Singleton<Device>;` (or `friend Singleton<Device>;`) so the Device
class correctly befriends the Singleton template instantiation; locate the
declaration in class Device and replace the existing `friend Singleton;` line
with the explicit instantiation.
- Around line 56-59: Device::~Device is never run because Singleton::get()
intentionally leaks the instance, so sessions_.clear() in the destructor never
executes; add an explicit cleanup path instead of relying on the destructor:
implement a public method like Device::shutdown() or Device::cleanupSessions()
that performs sessions_.clear() and any other teardown, and call that from the
application shutdown sequence (or add a Singleton::destroyInstance()/release()
that invokes shutdown before freeing or marking the singleton for destruction).
Alternatively, if you prefer automatic cleanup, change the singleton ownership
so it is not intentionally leaked (e.g., manage it with a static unique_ptr and
a controlled destroy function) and ensure Device::~Device then runs; update call
sites to use the new shutdown/destroy method accordingly.
In @include/mqt-core/qdmi/driver/Driver.hpp:
- Around line 397-399: The class currently declares "friend Singleton;" which
doesn't match the template specialization and prevents Singleton<Driver>::get()
from accessing Driver's private constructor; update the friendship to either
friend class Singleton<Driver>; or make the template friend with
"template<typename> friend class Singleton;" so that Singleton<Driver>::get()
can access Driver()—locate the Driver class declaration and replace the
incorrect friend line accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
include/mqt-core/fomac/FoMaC.hppinclude/mqt-core/qdmi/common/Common.hppinclude/mqt-core/qdmi/devices/dd/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/driver/Driver.hppsrc/fomac/FoMaC.cppsrc/qdmi/CMakeLists.txtsrc/qdmi/common/CMakeLists.txtsrc/qdmi/common/Common.cppsrc/qdmi/devices/CMakeLists.txtsrc/qdmi/devices/dd/Device.cppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/sc/Device.cppsrc/qdmi/driver/Driver.cpptest/fomac/test_fomac.cpp
🧰 Additional context used
🧠 Learnings (16)
📓 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.836Z
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/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.
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.
📚 Learning: 2025-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
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/qdmi/devices/sc/Device.hppsrc/qdmi/common/CMakeLists.txtinclude/mqt-core/fomac/FoMaC.hppsrc/qdmi/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/fomac/test_fomac.cppsrc/qdmi/devices/CMakeLists.txtinclude/mqt-core/qdmi/devices/na/Device.hppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/dd/Device.cppsrc/qdmi/common/Common.cppinclude/mqt-core/qdmi/driver/Driver.hppsrc/fomac/FoMaC.cppinclude/mqt-core/qdmi/devices/dd/Device.hppsrc/qdmi/devices/sc/Device.cppinclude/mqt-core/qdmi/common/Common.hpp
📚 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/qdmi/devices/sc/Device.hppsrc/qdmi/driver/Driver.cppinclude/mqt-core/qdmi/devices/na/Device.hppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/dd/Device.cppsrc/qdmi/common/Common.cppinclude/mqt-core/qdmi/driver/Driver.hppinclude/mqt-core/qdmi/devices/dd/Device.hppsrc/qdmi/devices/sc/Device.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:
include/mqt-core/qdmi/devices/sc/Device.hppsrc/qdmi/common/CMakeLists.txtinclude/mqt-core/fomac/FoMaC.hppsrc/qdmi/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/fomac/test_fomac.cppsrc/qdmi/devices/CMakeLists.txtinclude/mqt-core/qdmi/devices/na/Device.hppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/dd/Device.cppsrc/qdmi/common/Common.cppinclude/mqt-core/qdmi/driver/Driver.hppsrc/fomac/FoMaC.cppinclude/mqt-core/qdmi/devices/dd/Device.hppsrc/qdmi/devices/sc/Device.cpp
📚 Learning: 2025-12-28T17:14:53.890Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: src/qdmi/na/CMakeLists.txt:31-38
Timestamp: 2025-12-28T17:14:53.890Z
Learning: In the munich-quantum-toolkit/core repository, the NA device generator target (mqt-core-qdmi-na-device-gen) is intentionally propagated to MQT_CORE_TARGETS via list(APPEND) because it's publicly linked to the NA device library (the NA device uses a public function from the generator). The SC device generator is not propagated because it has no such dependency with the SC device library.
Applied to files:
include/mqt-core/qdmi/devices/sc/Device.hppsrc/qdmi/common/CMakeLists.txtsrc/qdmi/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txtinclude/mqt-core/qdmi/devices/na/Device.hppsrc/qdmi/devices/na/Device.cppsrc/qdmi/devices/dd/Device.cppsrc/qdmi/devices/sc/Device.cpp
📚 Learning: 2025-11-04T19:50:14.489Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/helpers/test_utils.cpp:103-118
Timestamp: 2025-11-04T19:50:14.489Z
Learning: In the munich-quantum-toolkit/core QDMI device tests, when QDMI APIs return a string result size `ks` that includes a null terminator, the pattern `std::string keys(ks - 1, '\0'); ... device_job_get_results(..., ks, keys.data(), nullptr)` is correct and safe. The string allocates `ks` bytes total (ks-1 content + implicit null), and the API writing `ks` bytes (including trailing null) fits exactly into this space without buffer overrun.
Applied to files:
include/mqt-core/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/driver/Driver.hpp
📚 Learning: 2026-01-04T23:54:33.540Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: src/qdmi/na/CMakeLists.txt:179-184
Timestamp: 2026-01-04T23:54:33.540Z
Learning: In the munich-quantum-toolkit/core repository, MSVC link-time code generation (LTCG) must be disabled for the NA device dynamic library target (mqt-core-qdmi-na-device-dyn) on Windows to avoid link errors and missing symbols. This issue affects both release-only builds and debug/release mixed builds, not just debug/release mixing scenarios.
Applied to files:
src/qdmi/common/CMakeLists.txtsrc/qdmi/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txt
📚 Learning: 2025-12-05T15:57:39.701Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 3
File: lib/Conversion/CatalystQuantumToMQTOpt/CMakeLists.txt:22-25
Timestamp: 2025-12-05T15:57:39.701Z
Learning: The munich-quantum-toolkit projects (core and core-plugins-catalyst) use `file(GLOB_RECURSE ...)` patterns in CMakeLists.txt files to collect header files, following an established convention in the parent repository for consistency across the codebase.
Applied to files:
src/qdmi/common/CMakeLists.txtsrc/qdmi/CMakeLists.txttest/fomac/test_fomac.cppsrc/qdmi/devices/CMakeLists.txtsrc/fomac/FoMaC.cpp
📚 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:
src/qdmi/CMakeLists.txtsrc/qdmi/devices/CMakeLists.txt
📚 Learning: 2025-12-28T17:13:36.900Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: pyproject.toml:98-102
Timestamp: 2025-12-28T17:13:36.900Z
Learning: In the munich-quantum-toolkit/core project, scikit-build-core is intelligent enough to skip build targets listed in pyproject.toml that don't exist for a given platform, so platform-specific targets (like `-dyn` targets conditioned on `NOT WIN32`) can be unconditionally listed in `build.targets` without causing Windows build failures.
Applied to files:
src/qdmi/devices/CMakeLists.txt
📚 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:
src/qdmi/devices/CMakeLists.txt
📚 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:
src/qdmi/devices/na/Device.cppsrc/qdmi/devices/dd/Device.cppinclude/mqt-core/qdmi/driver/Driver.hppsrc/qdmi/devices/sc/Device.cpp
📚 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:
src/qdmi/devices/na/Device.cppsrc/qdmi/devices/dd/Device.cppinclude/mqt-core/qdmi/devices/dd/Device.hppsrc/qdmi/devices/sc/Device.cpp
📚 Learning: 2025-11-05T09:23:46.540Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: src/qdmi/dd/Device.cpp:492-521
Timestamp: 2025-11-05T09:23:46.540Z
Learning: In the munich-quantum-toolkit/core repository, the `qasm3::Importer::imports()` function is backwards compatible with OpenQASM 2 programs. Therefore, it can be used to parse both QASM2 and QASM3 program formats without requiring separate importers for each version.
Applied to files:
src/qdmi/devices/dd/Device.cpp
📚 Learning: 2025-11-04T14:26:25.420Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/conftest.py:11-19
Timestamp: 2025-11-04T14:26:25.420Z
Learning: In the munich-quantum-toolkit/core repository, Qiskit is always available as a dependency during testing, so import guards for qiskit-dependent imports in test files (e.g., test/python/qdmi/qiskit/*.py) are not necessary.
Applied to files:
src/qdmi/devices/dd/Device.cpp
📚 Learning: 2026-01-04T23:01:47.734Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: mlir/CMakeLists.txt:13-15
Timestamp: 2026-01-04T23:01:47.734Z
Learning: In the munich-quantum-toolkit/core repository's MLIR directory (mlir/CMakeLists.txt), the MSVC compile definition `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` is necessary to suppress warnings triggered by MLIR/LLVM internal headers, not because the project code uses non-floating complex types. This suppression is needed to avoid warnings from third-party MLIR/LLVM header includes.
Applied to files:
src/fomac/FoMaC.cpp
🧬 Code graph analysis (4)
include/mqt-core/qdmi/devices/sc/Device.hpp (2)
include/mqt-core/qdmi/devices/dd/Device.hpp (2)
Device(67-67)Device(71-71)include/mqt-core/qdmi/devices/na/Device.hpp (2)
Device(72-72)Device(76-76)
include/mqt-core/qdmi/driver/Driver.hpp (1)
src/qdmi/driver/Driver.cpp (2)
Driver(388-395)Driver(397-400)
include/mqt-core/qdmi/devices/dd/Device.hpp (2)
src/qdmi/devices/dd/Device.cpp (1)
Device(149-151)src/qdmi/devices/sc/Device.cpp (2)
Device(36-45)Device(46-49)
include/mqt-core/qdmi/common/Common.hpp (3)
include/mqt-core/fomac/FoMaC.hpp (9)
nodiscard(235-235)nodiscard(326-358)nodiscard(369-369)nodiscard(413-474)nodiscard(486-488)nodiscard(559-607)nodiscard(622-624)nodiscard(626-626)nodiscard(692-706)src/qdmi/driver/Driver.cpp (2)
nodiscard(267-279)nodiscard(295-309)test/fomac/test_fomac.cpp (2)
nodiscard(76-85)nodiscard(94-102)
🪛 Cppcheck (2.19.0)
src/qdmi/driver/Driver.cpp
[information] 11-11: Include file
(missingInclude)
[information] 13-13: Include file
(missingInclude)
[information] 14-14: Include file
(missingInclude)
[information] 15-15: Include file
(missingInclude)
[information] 16-16: Include file
(missingInclude)
test/fomac/test_fomac.cpp
[information] 12-12: Include file
(missingInclude)
src/qdmi/devices/na/Device.cpp
[information] 15-15: Include file
(missingInclude)
[information] 17-17: Include file
(missingInclude)
[information] 18-18: Include file
(missingInclude)
src/qdmi/devices/dd/Device.cpp
[information] 15-15: Include file
(missingInclude)
[information] 25-25: Include file
(missingInclude)
src/qdmi/common/Common.cpp
[information] 15-15: Include file
(missingInclude)
src/fomac/FoMaC.cpp
[information] 13-13: Include file
(missingInclude)
src/qdmi/devices/sc/Device.cpp
[information] 15-15: Include file
(missingInclude)
[information] 17-17: Include file
(missingInclude)
[information] 18-18: Include file
(missingInclude)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🐍 Test (macos-15) / 🐍 macos-15
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🐍 Test (windows-2025) / 🐍 windows-2025
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🐍 Lint / 🚨 Lint
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🇨 Lint / 🚨 Lint
🔇 Additional comments (13)
test/fomac/test_fomac.cpp (1)
12-12: Include path update is fine; ensure test target has the right include dirs (Cppcheck missingInclude).
Given Cppcheck reports(missingInclude)at Line 12, double-check the test’s include paths match the newqdmi/common/...layout (especially in non-default configurations).src/qdmi/common/Common.cpp (1)
15-15: LGTM—keep include path consistent with installed header layout; fix build include dirs if needed.
Cppcheck’s(missingInclude)at Line 15 is likely configuration-related, but it’s worth ensuring the qdmi-common target exports the include directories so this compiles in all build modes.include/mqt-core/fomac/FoMaC.hpp (1)
13-13: Public header include path change looks correct—confirm install/export keepsqdmi/common/Common.hppavailable.
This is part of the public interface surface; ensure the install/export step still ships the header at that exact include prefix.src/fomac/FoMaC.cpp (1)
13-13: > Likely an incorrect or invalid review comment.src/qdmi/common/CMakeLists.txt (1)
9-34: File_SET and MQT_CORE_TARGETS propagation are correctly implemented. FILE_SET is fully supported by the project's CMake 3.24 minimum requirement, and headers are properly staged ininclude/mqt-core/. Theif(NOT TARGET)guard withlist(APPEND MQT_CORE_TARGETS)inside andset(MQT_CORE_TARGETS ... PARENT_SCOPE)outside follows the established pattern used for other QDMI targets (NA device generator, SC device, DD device). No changes required.src/qdmi/devices/CMakeLists.txt (1)
9-15: CMake subdirectory delegation +MQT_CORE_TARGETSpropagation looks consistent.Please double-check that
src/qdmi/devices/dd/CMakeLists.txt,src/qdmi/devices/sc/CMakeLists.txt, andsrc/qdmi/devices/na/CMakeLists.txteach push any new targets intoMQT_CORE_TARGETSin this directory scope (viaPARENT_SCOPE), otherwise Line 13-15 won’t export anything new.src/qdmi/devices/dd/Device.cpp (1)
15-25: Header path adjustments are coherent with the newdevices/+common/layout.src/qdmi/devices/sc/Device.cpp (1)
15-18: Include path updates match the reorg; no functional concerns here.src/qdmi/devices/na/Device.cpp (1)
15-19: Include path updates are consistent with the new directory structure.src/qdmi/driver/Driver.cpp (1)
11-16: Include path updates are correct—no stale includes found.Verified that no remaining references to the old include paths (
qdmi/Driver.hpp,qdmi/Common.hpp,qdmi/dd/Device.hpp,qdmi/sc/Device.hpp,qdmi/na/Device.hpp,qdmi/dd/Generator.hpp,qdmi/sc/Generator.hpp,qdmi/na/Generator.hpp) exist in the repository. The reorganization is complete and consistent throughout the codebase.src/qdmi/CMakeLists.txt (1)
9-15: Verify moved subdirectory CMake preserved prior special-case behavior.This file now delegates everything; please ensure the new
src/qdmi/common|devices|driver/CMakeLists.txtkeep:
- correct
MQT_CORE_TARGETSappends/propagation, and- Windows/MSVC special-casing for NA dynamic library (LTCG disable), and NA generator target propagation behavior.
Based on learnings, these details have caused regressions before.
include/mqt-core/qdmi/common/Common.hpp (1)
22-50: Remove this comment—the design concern is based on incorrect assumptions.No derived classes attempt to hide or override
get(). All classes (Driver, Device instances) publicly inherit fromSingleton<Concrete>, makingget()part of their public API by design. The intentional memory leak is already documented in the code as a deliberate choice to avoid static initialization order issues. If this design pattern is a concern, it should be addressed at a higher architectural level rather than flagged as an API exposure problem.Likely an incorrect or invalid review comment.
include/mqt-core/qdmi/devices/dd/Device.hpp (1)
35-37: No changes needed. The code correctly implements the singleton pattern with public inheritance andfriend Singleton;. TheDevice::get()method is appropriately public—this is the intended design. The Singleton template provides a public staticget()method, and public inheritance makes this method accessible on the Device class, which is exactly how the pattern should work. Thefrienddeclaration is necessary because Singleton'sget()method instantiates the derived class using its (intentionally private) constructor. The extensive usage ofDevice::get()throughout the codebase confirms this is the intended public API.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @include/mqt-core/qdmi/driver/Driver.hpp:
- Line 18: The include in Driver.hpp uses angle brackets for an internal header;
change #include <qdmi/common/Common.hpp> to use quotes (#include
"qdmi/common/Common.hpp") to match the other device headers (e.g., na::Device,
sc::Device, dd::Device) and maintain consistent internal include style.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
include/mqt-core/qdmi/devices/dd/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/driver/Driver.hpp
🧰 Additional context used
🧠 Learnings (9)
📓 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.836Z
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/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.
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.
📚 Learning: 2025-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
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/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/dd/Device.hppinclude/mqt-core/qdmi/driver/Driver.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/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/dd/Device.hppinclude/mqt-core/qdmi/driver/Driver.hpp
📚 Learning: 2025-12-19T00:05:54.428Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:111-116
Timestamp: 2025-12-19T00:05:54.428Z
Learning: In the munich-quantum-toolkit/core repository after migrating to nanobind, lifetime management differs from pybind11: `nb::keep_alive<nurse, patient>()` does not exist in nanobind. Instead, use `nb::rv_policy::reference_internal` when binding methods that return objects referencing internal state of the parent object (e.g., Session::getDevices returning Device objects that depend on the Session). This tells nanobind to keep the parent alive while children exist.
Applied to files:
include/mqt-core/qdmi/devices/sc/Device.hpp
📚 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/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/dd/Device.hppinclude/mqt-core/qdmi/driver/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/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/devices/dd/Device.hppinclude/mqt-core/qdmi/driver/Driver.hpp
📚 Learning: 2025-12-28T17:14:53.890Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: src/qdmi/na/CMakeLists.txt:31-38
Timestamp: 2025-12-28T17:14:53.890Z
Learning: In the munich-quantum-toolkit/core repository, the NA device generator target (mqt-core-qdmi-na-device-gen) is intentionally propagated to MQT_CORE_TARGETS via list(APPEND) because it's publicly linked to the NA device library (the NA device uses a public function from the generator). The SC device generator is not propagated because it has no such dependency with the SC device library.
Applied to files:
include/mqt-core/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hpp
📚 Learning: 2025-11-04T19:50:14.489Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/helpers/test_utils.cpp:103-118
Timestamp: 2025-11-04T19:50:14.489Z
Learning: In the munich-quantum-toolkit/core QDMI device tests, when QDMI APIs return a string result size `ks` that includes a null terminator, the pattern `std::string keys(ks - 1, '\0'); ... device_job_get_results(..., ks, keys.data(), nullptr)` is correct and safe. The string allocates `ks` bytes total (ks-1 content + implicit null), and the API writing `ks` bytes (including trailing null) fits exactly into this space without buffer overrun.
Applied to files:
include/mqt-core/qdmi/devices/sc/Device.hppinclude/mqt-core/qdmi/devices/na/Device.hppinclude/mqt-core/qdmi/driver/Driver.hpp
📚 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/qdmi/driver/Driver.hpp
🧬 Code graph analysis (3)
include/mqt-core/qdmi/devices/sc/Device.hpp (1)
src/qdmi/devices/sc/Device.cpp (2)
Device(36-45)Device(46-49)
include/mqt-core/qdmi/devices/na/Device.hpp (1)
src/qdmi/devices/na/Device.cpp (1)
Device(35-46)
include/mqt-core/qdmi/driver/Driver.hpp (1)
src/qdmi/driver/Driver.cpp (2)
Driver(388-395)Driver(397-400)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🐍 Test (windows-2025) / 🐍 windows-2025
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🐍 Lint / 🚨 Lint
- GitHub Check: 🇨 Lint / 🚨 Lint
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Debug) / 🍎 macos-15 clang Debug
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Release) / 🍎 macos-15 clang Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
🔇 Additional comments (5)
include/mqt-core/qdmi/devices/na/Device.hpp (1)
18-18: LGTM! Singleton pattern correctly implemented.The class correctly inherits from
Singleton<Device>, declares the friend class to allow the base class access to the private constructor, and properly overrides the destructor. The= defaultdestructor is appropriate here since no explicit cleanup is needed.Also applies to: 31-32, 76-76
include/mqt-core/qdmi/devices/sc/Device.hpp (1)
18-18: LGTM! Singleton pattern correctly implemented with appropriate destructor.The non-defaulted destructor declaration is correct here, as the implementation (in
src/qdmi/devices/sc/Device.cpplines 45-48) explicitly clearssessions_before destruction to avoid segfaults. This differs appropriately from NA/DD devices which use= default.Also applies to: 30-32, 58-58
include/mqt-core/qdmi/driver/Driver.hpp (1)
397-401: LGTM! Singleton pattern correctly implemented.The Driver class properly inherits from
Singleton<Driver>, declares the friend relationship, keeps the constructor private, and correctly overrides the destructor for explicit cleanup of sessions and devices.Also applies to: 417-417
include/mqt-core/qdmi/devices/dd/Device.hpp (2)
20-20: LGTM! Singleton pattern correctly implemented.The class correctly inherits from
Singleton<Device>, declares the friend class, and usesoverride = defaultfor the destructor. The thread-safety members (mutexes, atomics) are appropriately preserved.Also applies to: 35-37, 71-71
242-244: LGTM! Correct usage of Singleton accessor.The
MQT_DDSIM_QDMI_Device_Job_impl_dconstructor correctly usesqdmi::dd::Device::get().generateUniqueID(), leveraging the staticget()method inherited from theSingleton<Device>base class.
|
@burgholzer, I am aware of the open conversations. Could you please check them during your review as well as I would value your input there. |
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
# Conflicts: # CHANGELOG.md
|
@Mergifyio backport v3.x |
✅ Backports have been createdDetails
|
## Description This PR reorganizes the QDMI codebase to improve its structure and maintainability. It extracts and incorporates the best practices from #1347 (Refactor QDMI Devices and Fix Singleton Handling). ## 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. - [ ] 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: burgholzer <[email protected]> (cherry picked from commit 42fb76c) # Conflicts: # CHANGELOG.md
## Description This PR reorganizes the QDMI codebase to improve its structure and maintainability. It extracts and incorporates the best practices from #1347 (Refactor QDMI Devices and Fix Singleton Handling). ## Checklist: - [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. - [ ] 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.<hr>This is an automatic backport of pull request #1444 done by [Mergify](https://mergify.com). --------- Signed-off-by: burgholzer <[email protected]> Co-authored-by: Yannick Stade <[email protected]> Co-authored-by: burgholzer <[email protected]>
Description
This PR reorganizes the QDMI codebase to improve its structure and maintainability. It extracts and incorporates the best practices from #1347 (Refactor QDMI Devices and Fix Singleton Handling).
Checklist: