QAMP - Expose IQP circuit generator via C bindings #15396
Conversation
qiskit-bot
added
the
Community PR
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 19807786529Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
Thanks for opening the PR, Ameya! Just FYI, you don't need to add a detailed changelog like that usually. It's nice to summarize the PR if it is a complicated change, but here it would've been fine without (or much shorter) 🙂 |
| /// - `view` is square and symmetric. | ||
| /// - Any error from `from_standard_gates` is considered unreachable for valid | ||
| /// inputs, so we use `unwrap()` like other cext code. | ||
| fn iqp_from_view(view : ArrayView2<'_, i64>) -> CircuitData { |
There was a problem hiding this comment.
This looks like a legacy function from an earlier design -- why not just move this into the main function, since it's just 2 lines (or effectively one)?
There was a problem hiding this comment.
Good point — I’ve removed iqp_from_view and now call CircuitData::from_standard_gates(..., iqp(view), …) directly in qk_circuit_library_iqp.
| /// matrix is not symmetric. | ||
| #[unsafe(no_mangle)] | ||
| #[cfg(feature = "cbinding")] | ||
| pub extern "C" fn qk_circuit_library_iqp_( |
There was a problem hiding this comment.
Could we remove the final underscore?
There was a problem hiding this comment.
Updated the C API names to qk_circuit_library_iqp and qk_circuit_library_random_iqp (no trailing underscore) and adjusted the header + tests accordingly.
|
|
||
| let num_qubits = num_qubits as usize; | ||
| if num_qubits == 0 { | ||
| return std::ptr::null_mut(); |
There was a problem hiding this comment.
I guess it would be valid to return a QkCircuit* object here, but just with 0 qubits.
There was a problem hiding this comment.
Handled num_qubits == 0 by returning an empty but valid circuit via from_standard_gates(0, std::iter::empty(), Param::Float(0.0)) instead of `NULL.
| /// - A newly allocated `QkCircuit*` (caller must free with `qk_circuit_free`). | ||
| #[unsafe(no_mangle)] | ||
| #[cfg(feature = "cbinding")] | ||
| pub extern "C" fn qk_circuit_library_random_iqp_( |
There was a problem hiding this comment.
Same here on the final _ 🙂
| nrows as u32, | ||
| iqp(view), | ||
| Param::Float(0.0) | ||
| ).unwrap() |
There was a problem hiding this comment.
Could you replace the unwraps with expects that state what broke? E.g. here we could have .expect("Failed building the circuit from IQP data.") or something along those lines
There was a problem hiding this comment.
Done — replaced the unwrap()s with expect(...) messages in both the IQP and random IQP paths.
| // Wrap the flat buffer as an `ndarray` view with shape (n, n). | ||
| let view = ndarray::ArrayView2::from_shape((num_qubits, num_qubits), buf).unwrap(); | ||
|
|
||
| // Symmetry on upper triangle only. |
There was a problem hiding this comment.
I would have a slight preference to avoid data verification here, since we're targeting a performant C interface. If you think it would be helpful we could add a flag to optionally include checks?
There was a problem hiding this comment.
Added a check_input bool parameter: when true we run the symmetry check and return NULL on mismatch; when false we skip validation for the fast path.
| return std::ptr::null_mut(); | ||
| } | ||
|
|
||
| // SAFETY: caller guarantees at least n*n elements are readable. |
There was a problem hiding this comment.
Clarified the docstring so that n is explicitly defined as num_qubits in the description of the interactions matrix.
| // Optional symmetry check on the upper triangle only. | ||
| if check_input { | ||
| let mut i = 0; | ||
| while i < num_qubits { |
There was a problem hiding this comment.
Actually -- there's already a symmetry check in the Python call for IQP, see py_iqp. It would be cleaner to just move this (optional) check into the iqp function and have it in a single place. There's an existing check_symmetric function there, too.
4 participants
Title: Expose IQP circuit generator via C bindings
Summary
This PR exposes the IQP circuit generator from
qiskit-circuit-librarythrough the C API and adds C tests for the new entry points.Changes
crates/circuit_library/src/iqp.rscrates/cext/src/circuit_library/iqp.rsinteractionsas ann × nrow-majorint64_tmatrix.NULLpointer,num_qubits > 0, and enforces symmetry on the upper triangle.ArrayView2and buildsCircuitDatausing the sharediqphelper.NULLfor invalid input (null pointer, zero qubits, or non-symmetric matrix).py_random_iqp(num_qubits, Option<u64>)to generate a random IQP circuit.seedto indicate “draw from OS entropy”, mirroring existing C bindings.test/c/test_iqp.cqk_circuit_library_iqp_to returnNULL.