-
Notifications
You must be signed in to change notification settings - Fork 2.7k
QDrift C API Implementation #15390
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
base: main
Are you sure you want to change the base?
QDrift C API Implementation #15390
Conversation
…g, fixing an MSVC incompatibility
|
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:
|
…prox_real with Result
Cryoris
left a comment
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.
Thanks for the PR @Didgety, looks already good and I left some comments below. I think it would be good to separate the exposure of global_phase in a separate PR (including tests and a release note), if you'd like to do that.
| let rescaled_time = 2.0 * lambda / num_gates as f64 * time; | ||
|
|
||
| // sample term indices (0..n_terms) | ||
| let sampled_indices: Vec<usize> = (0..num_gates).map(|_| dist.sample(&mut rng)).collect(); |
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.
Is it possible to write this and the following circuit constructor as single iteration and avoid the Vec creations? Possibly something along the lines of
let evos = (0..num_gates)
.map(|_| dist.sample(&mut rng))
.map(|i| {
// compute pauli_string, indices, theta
// call sparse_term_evolution directly
});
CircuitData::from_packed_operations(..., evos, ...)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.
Great observation, reworked in cb2d151
| pauli_strings.push(pauli_string); | ||
| pauli_indices.push(dense_indices); |
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 would expect we don't need to use "dense" indices and Paulis of size num_qubits but could just use a sparse format where we only store the non-identity Paulis and the corresponding indices, no?
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.
also resolved by cb2d151
| /// with a probability proportional to its weight. | ||
| /// Based on the work of Earl Campbell in Ref. [1]. | ||
| /// | ||
| /// # Parameters |
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.
Could you update this to doxygen style like the other C-facing functions? 🙂
| quantum circuits that approximate the time evolution under a given sparse | ||
| observable using the QDRIFT algorithm. For example:: | ||
|
|
||
| #include <qiskit.h> |
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 think this needs to be captured in a .. code-block:: c. Afaik :: only works for Python
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.
also fixed by f8c6ed7
…ted to use sparse indices.
… sparse construction
Summary
This pull request implements QDrift Trotterization within the C circuit library.
Details and comments
As part of the implementation, some things were made public:
pauli_evolutionmodule from the Rustcircuit_librarycrate, a type alias within that moduleInstruction, and a functionsparse_term_evolutionThe implementation was loosely based on the Python version, but is grounded in the mathematics from the paper: https://arxiv.org/abs/1811.08017
Tests were created for valid observables, invalid observables, gate count scaling, invalid repetitions, observables containing projectors, and global phase.