-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial C API for circuit construction #14006
base: main
Are you sure you want to change the base?
Conversation
One or more of the following people are relevant to this code:
|
d3d226a
to
590e7a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really cool! I left some comments below 🙂
2e02dc1
to
bbde5e5
Compare
Pull Request Test Coverage Report for Build 13924464330Warning: 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 |
Building off the infrastructure for the sparse observable added in PR Qiskit#13445 this commit adds a C FFI for building Quantum Circuits. Right now there is a function to create an empty circuit with n qubits and m clbits and then a function to add standard gates to a circuit and then also get the op counts out of a circuit. This is a start of the functionality for a C API around interacting with circuits, later PRs will expand this so we can have a more fully featured C API in the future. Part of Qiskit#13276
Co-authored-by: Julien Gacon <[email protected]>
This also updates the test logic to make sure we always free even in case of a failure.
3e62ec6
to
669695a
Compare
/// Behavior is undefined ``circuit`` is not a valid, non-null pointer to a ``QkCircuit``. | ||
#[no_mangle] | ||
#[cfg(feature = "cbinding")] | ||
pub unsafe extern "C" fn qk_circuit_append_measure( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just call this qk_circuit_measure
(same for the other directives)? The append
doesn't really add any information, or what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can drop append if you prefer. I named it this way because of the gate function I used append.
circuit: *mut CircuitData, | ||
qubit: u32, | ||
) -> ExitCode { | ||
let circuit = unsafe { mut_ptr_as_ref(circuit) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised cargo doesn't complain (maybe because of unsafe?) but the circuit
should be declared mut
if we push onto it, no? The same holds for the other methods 🙂
/// Behavior is undefined if ``inst`` is not an object returned by ``qk_circuit_get_instruction``. | ||
#[no_mangle] | ||
#[cfg(feature = "cbinding")] | ||
pub unsafe extern "C" fn qk_free_circuit_instruction(inst: CInstruction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be qk_circuit_instruction_free
for consistency -- and should it take a pointer to a CInstruction
otherwise the struct would get passed by value? Same question actually below for OpCounts
🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does take it by value, but that's because we're freeing the pointers that are contained in the struct not the struct itself. Each CInstruction
has 4 pointers to a heap allocated arrays that we need to free. If we just drop the instruction those would be leaked. We can take a pointer to CInstruction
but it functionally doesn't matter in this case because the struct itself isn't what we're freeing.
Co-authored-by: Julien Gacon <[email protected]>
Locally I was using clang-format-19 while in CI we run with clang-format-14. Version 14 was flagging this difference while version 19 was happy. This commit makes the change to make version 14 happy in CI.
Summary
Building off the infrastructure for the sparse observable added in
PR #13445 this commit adds a C FFI for building Quantum Circuits. Right
now there is a function to create an empty circuit with n qubits and m
clbits and then a function to add standard gates to a circuit and then
also get the op counts out of a circuit. This is a start of the
functionality for a C API around interacting with circuits, later PRs
will expand this so we can have a more fully featured C API in the
future.
Details and comments
Part of #13276
This PR is based on top of #13986 and will need to rebased after that merges. To see the contents of just this PR you can look at: mtreinish/qiskit-core@no-py-token-constructor-path...mtreinish:qiskit-core:c-api-circuit-rebased
TODO: