🚸 Improve QDMI Integration#1694
Open
burgholzer wants to merge 14 commits into
Open
Conversation
burgholzer
added
refactor
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
burgholzer
force-pushed
the
smoother-qdmi-integration
branch
from
14740d4 to
5e96b16
Compare
This was referenced May 11, 2026
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR refactors the QDMI device architecture to build all devices as shared libraries, removing static library wrappers and implementing Windows-compatible runtime dynamic loading. It enhances Qiskit backend support for multi-controlled gates, fixes a DD simulation bug with measurements, consolidates mock device testing infrastructure, and updates both C++ and Python test suites to use real device discovery via FoMaC instead of hardcoded mocks. ChangesQDMI Architecture Refactor
Qiskit Backend Multi-Controlled Gate Support
Bug Fixes & Modernization
Test Infrastructure Refactoring
Documentation & Release Notes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/python/plugins/qiskit/test_sampler.py (1)
56-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale comment: this is no longer a mock backend.
This test now executes against a real DDSIM device, so the "mock backend returns random distribution" comment is misleading. With DDSIM, the result of
|00⟩ + |11⟩measured in the computational basis should only ever yield"00"or"11"— you can additionally assert that the distribution is restricted to those two outcomes to make the test more meaningful now that results are deterministic.📝 Suggested fix
- # Check that we got some counts (mock backend returns random distribution) - counts = bit_array.get_counts() - assert sum(counts.values()) == 100 + # Bell state: only "00" and "11" outcomes should appear + counts = bit_array.get_counts() + assert sum(counts.values()) == 100 + assert set(counts).issubset({"00", "11"})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/python/plugins/qiskit/test_sampler.py` around lines 56 - 58, The comment is stale because the test now runs on a real DDSIM backend; update the test around bit_array.get_counts() by removing the "mock backend" phrasing and add an assertion that the observed outcome keys are only "00" and "11" (e.g., assert set(counts.keys()) <= {"00","11"}) in addition to the existing total-count check so the test verifies the expected deterministic Bell-state outcomes; locate this change in the test function using bit_array.get_counts() and the counts variable.test/python/plugins/qiskit/test_estimator.py (1)
36-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale "mock backend" comment; tighten the assertion now that DDSIM is deterministic.
The comment on line 47 is no longer accurate — DDSIM is a real simulator, not a mock returning random values. For the simple
|0⟩state,⟨Z⟩should be1.0and⟨X⟩should be0.0, and the test callsrun(..., precision=None). You can additionally assert the expected values (withpytest.approxto account for sampling noise) to actually validate behavior rather than only shape.📝 Suggested fix
- # Values are simulated by mock backend (random), so we just check structure - evs = pub_result.data["evs"] + # For |0>: <Z> = 1.0 and <X> = 0.0 + evs = pub_result.data["evs"] stds = pub_result.data["stds"] assert evs.shape == (2,) # 2 observables assert stds.shape == (2,) + assert evs[0] == pytest.approx(1.0, abs=0.1) + assert evs[1] == pytest.approx(0.0, abs=0.1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/python/plugins/qiskit/test_estimator.py` around lines 36 - 51, The test_estimator_run_simple_observable has a stale comment about a "mock backend" and only asserts shapes; update it to assert the actual expected expectation values from the deterministic DDSIM run: remove or update the comment and add assertions on pub_result.data["evs"] using pytest.approx to check evs[0] == approx(1.0) and evs[1] == approx(0.0) (and keep the shape asserts if desired); reference the test function test_estimator_run_simple_observable, the estimator.run(...) call and the pub_result.data["evs"] / ["stds"] fields when making the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/qdmi/driver/CMakeLists.txt`:
- Around line 55-60: The rpath entries are currently set with
target_link_options(... INTERFACE ...), which only propagates to consumers and
not to the driver's own binary (so dlopen() inside the driver won't find device
libs); replace that with set_target_properties(${TARGET_NAME} PROPERTIES
BUILD_RPATH "<list>" (or INSTALL_RPATH if you want it for install) using the
same generator expressions $<TARGET_FILE_DIR:MQT::CoreQDMINaDevice>,
$<TARGET_FILE_DIR:MQT::CoreQDMIScDevice>,
$<TARGET_FILE_DIR:MQT::CoreQDMI_DDSIM_Device> so the rpath is embedded into the
driver itself; remove the INTERFACE target_link_options entries or keep only
non-INTERFACE options if needed.
In `@src/qdmi/driver/Driver.cpp`:
- Around line 44-81: Add Doxygen-style comments for the two Windows loader
helpers so their behavior is clear; above getDriverDirectory() document what it
does (returns the directory of the current module), note it is [[nodiscard]],
describe failure behavior (returns empty path), and summarize the Windows APIs
used (GetModuleHandleExW/GetModuleFileNameW) and why a loop/resizing is
necessary; above loadDeviceLibrary(const std::string& libName) document the
parameter (libName), explain path resolution (if libName has a parent path use
it, otherwise resolve relative to getDriverDirectory()), describe the return
value (HMODULE or null on failure), and list that it uses LoadLibraryExW with
LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS; include
`@param`, `@return` and brief one-line description for each function following the
project Doxygen style.
- Around line 383-386: Driver::Driver() currently calls addDynamicDeviceLibrary
for each entry in DYN_DEV_LIBS and allows any dlopen/LoadLibrary exception to
bubble out (breaking Driver::get() and C entrypoints). Wrap each
addDynamicDeviceLibrary(lib, prefix) invocation in a try/catch that catches
std::exception (and a catch(...) as fallback), log a clear error including the
failing lib/prefix and the exception.what() (or a generic message for non-std
exceptions), and continue to the next library so missing builtin DLLs are
skipped instead of aborting startup.
In `@test/python/plugins/qiskit/test_backend.py`:
- Around line 68-92: Add Google-style docstrings to the four helper functions
_single_qubit_circuit, _two_qubit_circuit, _two_qubit_barrier_circuit, and
_two_qubit_circuit_without_measurements: for each function include a one-line
summary, a short description of what the circuit contains, a Args section (none
or describe omitted params if any), and a Returns section specifying that a
QuantumCircuit is returned; place the docstring immediately below the def line
in triple quotes following Google-style formatting.
In `@test/python/plugins/qiskit/test_estimator.py`:
- Around line 142-143: Update the stale test comment in test_estimator.py:
replace "mock random results" with text stating the test runs against the DDSIM
simulator (deterministic up to sampling noise) and mention that for the
Bell-state circuit the expected expectation values are known (II=1, XX=1, YY=-1,
ZZ=1, IX=0, YI=0) and can be asserted with pytest.approx; optionally add those
pytest.approx assertions against the estimator outputs to make the test
deterministic instead of relying on a no-crash check.
- Around line 24-33: The estimator fixture duplicates DDSIM device discovery
logic found in test_sampler.py; extract that discovery into a shared fixture
named ddsim_backend in test/python/plugins/qiskit/conftest.py (use the same
logic that creates QDMIBackend(device=device, provider=None) from fomac.Session
devices) and then change def estimator() to def estimator(ddsim_backend) ->
QDMIEstimator: return QDMIEstimator(ddsim_backend), replacing the repeated
fomac.Session/QDMIBackend code; reference the existing estimator fixture,
QDMIEstimator, QDMIBackend, and the sampler fixture in test_sampler.py when
implementing the shared ddsim_backend fixture.
In `@test/python/plugins/qiskit/test_mock_backend.py`:
- Around line 317-378: Tests repeat the same mock_get_devices closure and
monkeypatch.setattr(fomac.Session, "get_devices", ...) in multiple tests;
extract this into a small helper (e.g., _patch_session_devices) that accepts
monkeypatch and a list of MockQDMIDevice and installs a single closure returning
that list via monkeypatch.setattr, then replace the per-test mock_get_devices
definitions and monkeypatch.setattr calls with calls to the new helper in
test_backend_warns_on_unmappable_operation,
test_backend_warns_on_missing_measurement_operation, and
test_backend_validation_uses_inverse_mapping.
In `@test/python/plugins/qiskit/test_sampler.py`:
- Around line 24-33: Extract the DDSIM discovery loop into a shared pytest
fixture (e.g., ddsim_backend) in test/python/plugins/qiskit/conftest.py that
creates and returns a QDMIBackend for the device whose device.name() contains
"DDSIM" or calls pytest.skip("DDSIM device not available") if none found; then
update the sampler fixture in test_sampler.py to accept that ddsim_backend
fixture and return QDMISampler(ddsim_backend) instead of repeating the loop, and
likewise change the estimator fixture (and the similar code in test_backend.py)
to depend on ddsim_backend so all device selection and skip behavior is
centralized.
---
Outside diff comments:
In `@test/python/plugins/qiskit/test_estimator.py`:
- Around line 36-51: The test_estimator_run_simple_observable has a stale
comment about a "mock backend" and only asserts shapes; update it to assert the
actual expected expectation values from the deterministic DDSIM run: remove or
update the comment and add assertions on pub_result.data["evs"] using
pytest.approx to check evs[0] == approx(1.0) and evs[1] == approx(0.0) (and keep
the shape asserts if desired); reference the test function
test_estimator_run_simple_observable, the estimator.run(...) call and the
pub_result.data["evs"] / ["stds"] fields when making the assertions.
In `@test/python/plugins/qiskit/test_sampler.py`:
- Around line 56-58: The comment is stale because the test now runs on a real
DDSIM backend; update the test around bit_array.get_counts() by removing the
"mock backend" phrasing and add an assertion that the observed outcome keys are
only "00" and "11" (e.g., assert set(counts.keys()) <= {"00","11"}) in addition
to the existing total-count check so the test verifies the expected
deterministic Bell-state outcomes; locate this change in the test function using
bit_array.get_counts() and the counts variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b068b742-fe63-4d9e-9f37-f941d7d6ec5b
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
CHANGELOG.mdUPGRADING.mdcmake/AddMQTCoreLibrary.cmakecmake/PackageAddTest.cmakedocs/qdmi/driver.mdinclude/mqt-core/qdmi/driver/Driver.hppjson/sc/device.jsonpyproject.tomlpython/mqt/core/plugins/qiskit/backend.pysrc/dd/Simulation.cppsrc/ir/QuantumComputation.cppsrc/qdmi/devices/dd/CMakeLists.txtsrc/qdmi/devices/dd/Device.cppsrc/qdmi/devices/na/CMakeLists.txtsrc/qdmi/devices/na/DynDevice.cppsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/sc/DynDevice.cppsrc/qdmi/driver/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/fomac/CMakeLists.txttest/na/fomac/CMakeLists.txttest/python/plugins/qiskit/conftest.pytest/python/plugins/qiskit/test_backend.pytest/python/plugins/qiskit/test_converters.pytest/python/plugins/qiskit/test_estimator.pytest/python/plugins/qiskit/test_mock_backend.pytest/python/plugins/qiskit/test_provider.pytest/python/plugins/qiskit/test_sampler.pytest/qdmi/devices/dd/CMakeLists.txttest/qdmi/devices/na/CMakeLists.txttest/qdmi/devices/sc/CMakeLists.txttest/qdmi/driver/CMakeLists.txttest/qdmi/driver/test_driver.cpp
💤 Files with no reviewable changes (7)
- test/python/plugins/qiskit/test_provider.py
- src/qdmi/devices/na/DynDevice.cpp
- src/qdmi/devices/dd/Device.cpp
- test/python/plugins/qiskit/conftest.py
- include/mqt-core/qdmi/driver/Driver.hpp
- src/qdmi/devices/sc/DynDevice.cpp
- test/python/plugins/qiskit/test_converters.py
…ary wrappers Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
…onverter Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
…sent Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
burgholzer
force-pushed
the
smoother-qdmi-integration
branch
from
5e96b16 to
c86bd14
Compare
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
python/mqt/core/plugins/qiskit/backend.py (2)
230-239:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize operation names before alias remapping/dedup.
Use lowercase once before
_QDMI_TO_QISKIT_GATE_MAPlookup. Current case-sensitive lookup can miss aliases and produce duplicate class-gate names with different casing.Suggested change
- op_name = op.name() + op_name = op.name().lower() @@ - if op_name in self._QDMI_TO_QISKIT_GATE_MAP: - op_name = self._QDMI_TO_QISKIT_GATE_MAP[op_name] + op_name = self._QDMI_TO_QISKIT_GATE_MAP.get(op_name, op_name)Also applies to: 251-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/mqt/core/plugins/qiskit/backend.py` around lines 230 - 239, Normalize op_name to lowercase immediately after retrieving it from op.name() and before any further checks or the alias lookup so the case-insensitive mapping in _QDMI_TO_QISKIT_GATE_MAP works and deduping doesn't produce different-cased duplicates; specifically, update the block that sets op_name = op.name() and the subsequent lookup against _QDMI_TO_QISKIT_GATE_MAP to use op_name = op.name().lower(), and apply the same normalization to the later occurrence that currently performs a lookup/alias remapping (the lines around where op_name is remapped again) so all comparisons and remaps use the lowercased op_name consistently.
63-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the gate map types to include instruction classes.
The map stores both instruction instances and classes (e.g.,
MCXGate,MCPhaseGatefrom lines 80–82), but type annotations declare onlyInstructioninstances. This will triggertydiagnostics.Suggested change
-def _build_gate_mappings_for_backend( +def _build_gate_mappings_for_backend( gate_aliases: dict[str, set[str]], -) -> tuple[dict[str, set[str]], dict[str, Instruction]]: +) -> tuple[dict[str, set[str]], dict[str, Instruction | type[Instruction]]]: @@ - operation_to_gate: dict[str, Instruction] = {} + operation_to_gate: dict[str, Instruction | type[Instruction]] = {} @@ - _OPERATION_TO_GATE_MAP: ClassVar[dict[str, Instruction]] + _OPERATION_TO_GATE_MAP: ClassVar[dict[str, Instruction | type[Instruction]]] @@ - def _map_operation_to_gate(op_name: str) -> Instruction | None: + def _map_operation_to_gate(op_name: str) -> Instruction | type[Instruction] | None:Also applies to lines 86–87, 158–159, and 324–325.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/mqt/core/plugins/qiskit/backend.py` around lines 63 - 64, The type annotations for the Qiskit↔QDMI mappings currently only allow Instruction instances but the maps also store instruction classes (e.g., MCXGate, MCPhaseGate); update the annotations to include both classes and instances by importing typing.Type and typing.Union and changing occurrences like dict[str, Instruction] or set[Instruction] to dict[str, Union[Instruction, Type[Instruction]]] or set[Union[Instruction, Type[Instruction]]] (or use Type[Instruction] where only classes are stored); apply this change to the function return annotation that currently reads ") -> tuple[dict[str, set[str]], dict[str, Instruction]]" and to the other map/type hints referenced around the same module (the maps defined near lines ~86–87, ~158–159, and ~324–325) so all places that can hold classes or instances accept Type[Instruction] as well as Instruction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Around line 52-53: The docs dependency group is allowing
qiskit[qasm3-import]>=1.0.0 which mismatches the other groups; update the
`dependency-groups.docs` entry for `qiskit[qasm3-import]` to use the same lower
bound `>=1.1.0` so it matches `project.optional-dependencies.qiskit` and
`dependency-groups.test` (edit the docs group line that currently mentions
`qiskit[qasm3-import]` and change the version specifier to `>=1.1.0`).
In `@test/na/fomac/CMakeLists.txt`:
- Around line 18-35: Change the add_custom_command invocations that copy device
DLLs to use POST_BUILD instead of PRE_BUILD and add per-target existence guards
before referencing each device target; specifically, wrap the copy command for
each device target (MQT::CoreQDMIScDevice, MQT::CoreQDMINaDevice,
MQT::CoreQDMI_DDSIM_Device) in an if(TARGET <device>) ... endif() block and
replace PRE_BUILD with POST_BUILD while keeping the same COMMAND and destination
using $<TARGET_FILE_DIR:${TARGET_NAME}> so the commands only run when the device
targets exist and run after the test target is built.
In `@test/python/plugins/qiskit/test_mock_backend.py`:
- Around line 176-178: The outcome generation incorrectly encodes zero classical
bits because format(0, "00b") yields "0"; update the logic in the block that
computes num_outcomes and outcomes (using self._num_clbits and the outcomes
list) to special-case self._num_clbits == 0 so that outcomes becomes [""] (and
num_outcomes remains 1) instead of producing a phantom "0" bit string; adjust
only the outcome generation around num_outcomes and outcomes to return an
empty-string key for no-measurement programs.
In `@test/qdmi/devices/dd/CMakeLists.txt`:
- Around line 36-43: The custom command that copies the Windows DLL uses
PRE_BUILD which is unreliable across non-Visual-Studio generators; change the
add_custom_command for TARGET ${TARGET_NAME} to use POST_BUILD instead of
PRE_BUILD so the copy runs after the target is built (staging
$<TARGET_FILE:MQT::CoreQDMI_DDSIM_Device> into
$<TARGET_FILE_DIR:${TARGET_NAME}>). Ensure the command still uses the same
TARGET ${TARGET_NAME} and copy_if_different arguments but with POST_BUILD to
avoid timing issues with Ninja and other generators.
In `@test/qdmi/driver/CMakeLists.txt`:
- Around line 19-36: The add_custom_command calls for the test target (TARGET
${TARGET_NAME}) use PRE_BUILD which is generator-dependent; change each
PRE_BUILD to POST_BUILD and make the copy commands depend explicitly on the test
target's build so they run after the target is produced, i.e. update the three
add_custom_command invocations that reference
$<TARGET_FILE:MQT::CoreQDMIScDevice>, $<TARGET_FILE:MQT::CoreQDMINaDevice>, and
$<TARGET_FILE:MQT::CoreQDMI_DDSIM_Device> to use POST_BUILD and ensure the
target dependency is the ${TARGET_NAME} (or add add_dependencies(${TARGET_NAME}
<device-target>) if needed) so the device DLLs are copied deterministically
across generators.
---
Outside diff comments:
In `@python/mqt/core/plugins/qiskit/backend.py`:
- Around line 230-239: Normalize op_name to lowercase immediately after
retrieving it from op.name() and before any further checks or the alias lookup
so the case-insensitive mapping in _QDMI_TO_QISKIT_GATE_MAP works and deduping
doesn't produce different-cased duplicates; specifically, update the block that
sets op_name = op.name() and the subsequent lookup against
_QDMI_TO_QISKIT_GATE_MAP to use op_name = op.name().lower(), and apply the same
normalization to the later occurrence that currently performs a lookup/alias
remapping (the lines around where op_name is remapped again) so all comparisons
and remaps use the lowercased op_name consistently.
- Around line 63-64: The type annotations for the Qiskit↔QDMI mappings currently
only allow Instruction instances but the maps also store instruction classes
(e.g., MCXGate, MCPhaseGate); update the annotations to include both classes and
instances by importing typing.Type and typing.Union and changing occurrences
like dict[str, Instruction] or set[Instruction] to dict[str, Union[Instruction,
Type[Instruction]]] or set[Union[Instruction, Type[Instruction]]] (or use
Type[Instruction] where only classes are stored); apply this change to the
function return annotation that currently reads ") -> tuple[dict[str, set[str]],
dict[str, Instruction]]" and to the other map/type hints referenced around the
same module (the maps defined near lines ~86–87, ~158–159, and ~324–325) so all
places that can hold classes or instances accept Type[Instruction] as well as
Instruction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4dc3b6e4-f9ad-47f3-9a4c-107a82596cf9
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
CHANGELOG.mdUPGRADING.mdcmake/AddMQTCoreLibrary.cmakecmake/PackageAddTest.cmakedocs/qdmi/driver.mdinclude/mqt-core/qdmi/driver/Driver.hppjson/sc/device.jsonpyproject.tomlpython/mqt/core/plugins/qiskit/backend.pysrc/dd/Simulation.cppsrc/ir/QuantumComputation.cppsrc/qdmi/devices/dd/CMakeLists.txtsrc/qdmi/devices/dd/Device.cppsrc/qdmi/devices/na/CMakeLists.txtsrc/qdmi/devices/na/DynDevice.cppsrc/qdmi/devices/sc/CMakeLists.txtsrc/qdmi/devices/sc/DynDevice.cppsrc/qdmi/driver/CMakeLists.txtsrc/qdmi/driver/Driver.cpptest/fomac/CMakeLists.txttest/na/fomac/CMakeLists.txttest/python/plugins/qiskit/conftest.pytest/python/plugins/qiskit/test_backend.pytest/python/plugins/qiskit/test_converters.pytest/python/plugins/qiskit/test_estimator.pytest/python/plugins/qiskit/test_mock_backend.pytest/python/plugins/qiskit/test_provider.pytest/python/plugins/qiskit/test_sampler.pytest/qdmi/devices/dd/CMakeLists.txttest/qdmi/devices/na/CMakeLists.txttest/qdmi/devices/sc/CMakeLists.txttest/qdmi/driver/CMakeLists.txttest/qdmi/driver/test_driver.cpp
💤 Files with no reviewable changes (7)
- src/qdmi/devices/dd/Device.cpp
- test/python/plugins/qiskit/test_provider.py
- src/qdmi/devices/sc/DynDevice.cpp
- include/mqt-core/qdmi/driver/Driver.hpp
- test/python/plugins/qiskit/test_converters.py
- src/qdmi/devices/na/DynDevice.cpp
- test/python/plugins/qiskit/conftest.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/python/plugins/qiskit/test_mock_backend.py`:
- Line 189: The zip in the dict comprehension that builds the outcome->count
mapping should use strict=True to ensure outcomes and counts_list have the same
length; update the expression that currently uses zip(outcomes, counts_list,
strict=False) to zip(outcomes, counts_list, strict=True) so any future length
mismatches are caught (look for the comprehension referencing outcomes and
counts_list in test_mock_backend.py).
- Around line 174-175: The code returns {} when self._num_clbits == 0 which
breaks the counts-sum invariant; change that branch to return {"": self._shots}
so the single empty-bitstring outcome accumulates all shots (use self._shots and
keep the empty-string key). Also update the zip call that iterates outcomes (the
one pairing sequences of length num_outcomes) to use zip(..., strict=True) to
enforce equal lengths and catch future mismatches; look for the zip(...) using
num_outcomes and replace with zip(..., strict=True).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b85bc0b-faa2-4ed8-8292-4c0b8c4056ef
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
pyproject.tomltest/python/plugins/qiskit/test_mock_backend.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR originally only aimed to address #1352.
However, working on it prompted several follow-ups and improvements for the QDMI support layer as well as a couple of bugfixes.
The main change here is still that all QDMI devices are now built as shared libraries by default and that the respective dynamic wrappers have been removed.
The QDMI Qiskit wrapper now supports multi-controlled gates as exposed by the DDSIM device.
The QDMI Python tests have been significantly streamlined, and a lot of the mocking code is replaced with actual device executions.
I will leave this in draft until CI is green because I expect Windows to cause some problems.
Edit: As expected, Windows is acting up big again.
Edit2: I think I have a working version now. Only took 10h 🫠
Fixes #1352
Checklist
If PR contains AI-assisted content:
Assisted-by: [Model Name] via [Tool Name]footer.