✨ Add Sampler and Estimator Primitives to the QDMI-Qiskit Interface
#1507
Conversation
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
marcelwa
added
feature
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds QDMIEstimator and QDMISampler Qiskit primitives and exports; introduces a "gphase" gate alias; updates classical-bit parsing in tests; adds unit tests and documentation for the new primitives. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Estimator as QDMIEstimator
participant ObsCircuits as ObservableCircuits
participant Backend as QDMIBackend
participant Results as ResultProcessor
User->>Estimator: run(pubs, precision)
Estimator->>Estimator: coerce to EstimatorPub, select precision
Estimator->>ObsCircuits: _get_observable_circuits(observables)
ObsCircuits-->>Estimator: (coeffs, term_circuits, indices)
Estimator->>Estimator: bind params, compose full circuits
Estimator->>Backend: execute(all_circuits, shots)
Backend-->>Estimator: counts per circuit
Estimator->>Results: compute expectations & variances
Results-->>User: PubResult (evs, stds, metadata)
sequenceDiagram
actor User
participant Sampler as QDMISampler
participant Binder as ParamBinder
participant Backend as QDMIBackend
participant Converter as BitArrayConverter
User->>Sampler: run(pubs, shots)
Sampler->>Sampler: coerce to SamplerPub, determine shots
Sampler->>Binder: bind parameters -> bound circuits
Binder-->>Sampler: bound circuits
alt circuits exist
Sampler->>Backend: execute(bound_circuits, shots)
Backend-->>Sampler: counts per circuit
Sampler->>Converter: _get_bit_arrays(cregs, counts, shape)
Converter-->>User: shaped BitArray results + metadata
else no circuits
Sampler-->>User: empty SamplerPubResult
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@python/mqt/core/plugins/qiskit/sampler.py`:
- Around line 174-177: The code skips reshaping when shape is an empty tuple
(scalar) because "if shape:" is falsy; update the logic around
BitArray.from_counts and array.reshape so scalars are handled: replace the
truthy check with an explicit None check and call reshape with unpacking so
scalar tuple becomes a no-arg reshape (i.e., use if shape is not None and call
array.reshape(*shape)); reference BitArray.from_counts and array.reshape to
locate the change.
In `@test/python/plugins/qiskit/test_estimator.py`:
- Around line 165-173: Remove the dead assignment to vals that is immediately
overwritten: delete the line vals = [[0.0], [np.pi / 2], [np.pi]] so only the
intended test value assignment (vals = [[0.0], [np.pi]]) remains in
test_estimator.py; refer to the vals variable in the broadcasting test to locate
the correct assignment to keep.
In `@test/python/plugins/qiskit/test_sampler.py`:
- Around line 39-63: In test_sampler_run_simple_circuit add an assertion that
the measurement bit-array shape is the scalar empty-tuple case by asserting
bit_array.shape == (), next to the existing checks for num_shots/num_bits;
update the test function test_sampler_run_simple_circuit so pub_result (from
sampler.run) and data.meas (bit_array) are validated for scalar shape to guard
the falsy-empty-tuple bug in _get_bit_arrays.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/python/plugins/qiskit/test_estimator.py`:
- Around line 153-179: Update the outdated comments in
test_estimator_broadcasting to reflect the actual test data: remove references
to "3 Parameter sets", "2 observables × 3 parameters", and reshaping/outer
product and instead state that vals contains 2 parameter sets (shape (2,) / two
values) and the test asserts broadcasting for 2 observables × 2 parameter sets;
adjust the comment around the EstimatorPub.coerce((qc, ops, vals)) and the
subsequent assertions to describe the current shape expectations (data.evs.shape
== (2,), data.stds.shape == (2,)).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/python/plugins/qiskit/test_estimator.py`:
- Line 69: Remove the unnecessary cast when calling estimator.run: replace
estimator.run(cast("Any", [pub])) with estimator.run([pub]) because pub is
already an EstimatorPub (an EstimatorPubLike) and [pub] satisfies
Iterable[EstimatorPubLike]; leave other intentional casts (e.g., cast("Any",
pub_result.data)) untouched.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/qdmi/qdmi_backend.md`:
- Line 401: Fix the typo in the heading text "# Get results for the first pub
(Primitive Unified Bloc)" by changing "Bloc" to "Block" so the heading reads "#
Get results for the first pub (Primitive Unified Block)"; update any other
occurrences of "Primitive Unified Bloc" in the same document to "Primitive
Unified Block" to keep the acronym PUB consistent.
- Around line 520-526: Replace the ambiguous phrase "verification circuits" in
the "Primitives Implementation" section with clearer wording such as "provided
circuits" or "input circuits" wherever it appears (e.g., the sentence in the
Sampler/Estimator description that reads "appends necessary basis rotations and
measurements to the verification circuits"); update the Estimator description to
say it appends rotations/measurements to the provided/input circuits and
reconstructs expectation values from their measurement counts so the docs
clearly indicate these are user-supplied circuits.
- Line 398: The example uses a non-standard 1-tuple wrapper when calling
sampler.run; update the call to use bare circuit or an explicit 2-tuple per
BaseSamplerV2 conventions: replace sampler.run([(qc,)], shots=1024) with either
sampler.run([qc], shots=1024) for a non-parametric circuit or sampler.run([(qc,
None)], shots=1024) for an explicit parametric form, keeping the same
sampler.run and qc symbols so intent and API compatibility are clear.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/qdmi/qdmi_backend.md`:
- Line 382: Update the sentence that currently says "It is used to sample
quantum circuits and obtain bitstrings (quasi-distributions)" to remove the
misleading "(quasi-distributions)" and clarify that BaseSamplerV2 returns
SamplerPubResult containing BitArray bitstrings/counts; reference BaseSamplerV2,
SamplerPubResult, and BitArray (and note that QuasiDistribution is specific to
the older Sampler V1) so readers understand V2 returns bitstrings/counts rather
than V1-style quasi-distributions.
In `@test/python/plugins/qiskit/test_estimator.py`:
- Around line 33-200: Add negative/error-path tests alongside the existing
happy-path ones by creating two new pytest functions: one that calls
QDMIEstimator.run with a mismatched Observable vs circuit qubit count (e.g., use
QuantumCircuit(1) with SparsePauliOp("II") or SparsePauliOp("XX")) and asserts
that the call raises the expected exception (ValueError or QiskitError) from
QDMIEstimator/EstimatorPub; and another that calls QDMIEstimator.run with an
invalid precision (e.g., precision=-0.1 and precision=0) and asserts it raises
the input-validation error; add these tests near test_estimator_no_circuits or
test_estimator_precision_handling and reference QDMIEstimator.run and
EstimatorPub.coerce in the assertions so failures are tied to the right code
paths.
# Conflicts: # CHANGELOG.md
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 adds
SamplerandEstimatorprimitives to the QDMI-Qiskit Interface.Checklist: