State Preparation in MQT ✨ #543
Conversation
|
Hey 👋🏼 The best place to put this would probably be in the I would advise you to also create a After the initial setup, it would be great to add some tests for the new functionality. These should probably placed here: https://github.com/cda-tum/mqt-core/tree/main/test/algorithms. Again, you can take lots of inspiration from the existing tests and should be able to use our own simulator for some experiments. Please don't hesitate to ask questions if anything pops up! |
burgholzer
added
feature
|
Closing for now. Please feal free to reopen if you plan to continue working on this. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #543 +/- ##
======================================
Coverage 92.2% 92.3%
======================================
Files 126 127 +1
Lines 13832 13963 +131
Branches 2153 2178 +25
======================================
+ Hits 12762 12891 +129
- Misses 1070 1072 +2
*This pull request uses carry forward flags. Click here to find out more.
|
|
Hi, sorry for the long wait, I finally got around to working on this again! I do have a question on the new constructor that should be implemented: I do not see any option to reopen this pull request and continue working on it. |
Hey 👋🏼 No worries. I just reopened the PR for you.
I'd imagine that your State Preparation functionality code can be implemented and integrated very similarly. |
|
I refactored both files to match the other algorithms style if you would like to give it a look! Next step would then be tests from what I can see right? |
Getting the CI to a state where it is all-green would be desirable. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Lukas Burgholzer <[email protected]> Signed-off-by: M-J-Hochreiter <[email protected]>
they cannot really be represented with only alphanumericals
There was a problem hiding this comment.
Hey 👋🏼
I was not quite sure whether you were done with your implementation, but I figured I'd do a quick review of the main library code and provide some suggestions for improvements.
I hope these make sense.
You probably also need to merge the latest changes from main so that everything runs smoothly.
| /** | ||
| * Prepares a generic Quantum State from a list of normalized complex | ||
| amplitudes | ||
| * Adapted implementation of Qiskit State Preparation: | ||
| * | ||
| https://github.com/Qiskit/qiskit/blob/main/qiskit/circuit/library/data_preparation/state_preparation.py# | ||
| * based on paper: | ||
| * Shende, Bullock, Markov. Synthesis of Quantum Logic Circuits (2004) | ||
| [`https://ieeexplore.ieee.org/document/1629135`] | ||
| * */ | ||
|
|
||
| /** | ||
| * @throws invalid_argument when amplitudes are not normalized or length not | ||
| * power of 2 | ||
| * @param list of complex amplitudes to initialize to | ||
| * @return MQT Circuit that initializes a state | ||
| * */ |
There was a problem hiding this comment.
| /** | |
| * Prepares a generic Quantum State from a list of normalized complex | |
| amplitudes | |
| * Adapted implementation of Qiskit State Preparation: | |
| * | |
| https://github.com/Qiskit/qiskit/blob/main/qiskit/circuit/library/data_preparation/state_preparation.py# | |
| * based on paper: | |
| * Shende, Bullock, Markov. Synthesis of Quantum Logic Circuits (2004) | |
| [`https://ieeexplore.ieee.org/document/1629135`] | |
| * */ | |
| /** | |
| * @throws invalid_argument when amplitudes are not normalized or length not | |
| * power of 2 | |
| * @param list of complex amplitudes to initialize to | |
| * @return MQT Circuit that initializes a state | |
| * */ | |
| /** | |
| * @brief Prepares a generic quantum state from a list of normalized | |
| * complex amplitudes | |
| * | |
| * Adapted implementation of IBM Qiskit's State Preparation: | |
| * https://github.com/Qiskit/qiskit/blob/e9ccd3f374fd5424214361d47febacfa5919e1e3/qiskit/circuit/library/data_preparation/state_preparation.py | |
| * based on the following paper: | |
| * V. V. Shende, S. S. Bullock and I. L. Markov, "Synthesis of quantum-logic circuits," in IEEE Transactions on Computer-Aided Design of Integrated Circuits and Systems, vol. 25, no. 6, pp. 1000-1010, June 2006, doi: 10.1109/TCAD.2005.855930. | |
| * | |
| * @param amplitudes state (vector) to prepare. Must be normalized and have a size that is a power of two | |
| * @return quantum computation that prepares the state | |
| * @throws invalid_argument @p amplitudes is not normalized or its length is not a | |
| * power of two | |
| **/ |
A suggestion for a slightly improved docstring here. Note that this will be reformatted by clang-format if you simply accept the suggestion.
| } | ||
|
|
||
| auto createStatePreparationCircuit( | ||
| std::vector<std::complex<double>>& amplitudes) -> QuantumComputation { |
There was a problem hiding this comment.
Right now, the function signature suggests that the function will modify the amplitudes vector (since the argument is not a const reference).
This would be fine, as it is more efficient then copying, but it also implies that the amplitude vector is useless after calling this function.
The function docstring should note that the function changes the underlying vector in that case.
I see two options here:
- either make the parameter a constant reference and perform an explicit copy of the state.
- or update the docstring to reflect the behavior
(note that his comment depends on some of the other comments in the review)
src/algorithms/StatePreparation.cpp
Outdated
| QuantumComputation toZeroCircuit = gatesToUncompute(amplitudes, numQubits); | ||
|
|
||
| // invert circuit | ||
| CircuitOptimizer::flattenOperations(toZeroCircuit, false); |
There was a problem hiding this comment.
I'd argue that there is no direct need to flatten the circuit here. You might as well preserve the structure of the algorithm.
src/algorithms/StatePreparation.cpp
Outdated
|
|
||
| // creates circuit that takes desired vector to zero | ||
| [[nodiscard]] auto | ||
| gatesToUncompute(std::vector<std::complex<double>> amplitudes, size_t numQubits) |
There was a problem hiding this comment.
This method should not take its argument by value as this would cause a copy for each invocation. Pass this by reference and modify it in-place.
| rotationsToDisentangle(std::vector<std::complex<double>> amplitudes) | ||
| -> std::tuple<std::vector<std::complex<double>>, std::vector<double>, | ||
| std::vector<double>> { | ||
| std::vector<std::complex<double>> remainingVector; | ||
| std::vector<double> thetas; | ||
| std::vector<double> phis; | ||
| for (size_t i = 0; i < (amplitudes.size() / 2); ++i) { | ||
| auto [remains, theta, phi] = | ||
| blochAngles(amplitudes[2 * i], amplitudes[2 * i + 1]); | ||
| remainingVector.push_back(remains); | ||
| // minus sign because we move it to zero | ||
| thetas.push_back(-theta); | ||
| phis.push_back(-phi); | ||
| } | ||
| return {remainingVector, thetas, phis}; | ||
| } |
There was a problem hiding this comment.
Do not pass the amplitudes vector by value as this will always incur a copy.
Pass it by reference and modify it in-place instead of creating a new vector and returning the new vector.
This will require some adaptations to the general code logic probably.
Also just a general remark: prefer to reserve containers with the right size if you know the size upfront using vec.reserve(X). Furthermore, prefer to use emplace_back over push_back in general.
| Matrix const identity = | ||
| createIdentity(static_cast<size_t>(pow(2., localNumQubits - 2.))); |
There was a problem hiding this comment.
use bit shifts instead of std::pow for efficiency
src/algorithms/StatePreparation.cpp
Outdated
| } | ||
|
|
||
| [[nodiscard]] auto matrixVectorProd(const Matrix& matrix, | ||
| std::vector<double> vector) |
There was a problem hiding this comment.
vector should be a const reference here to avoid a copy on each invocation
src/algorithms/StatePreparation.cpp
Outdated
| return std::abs(1 - twoNorm(vec)) < EPS; | ||
| } | ||
|
|
||
| [[nodiscard]] auto kroneckerProduct(Matrix matrixA, Matrix matrixB) -> Matrix { |
There was a problem hiding this comment.
A nd B should be passed as const references here to avoid copying them on every invocation
src/algorithms/StatePreparation.cpp
Outdated
| template <typename T> [[nodiscard]] auto twoNorm(std::vector<T> vec) -> double { | ||
| double norm = 0; | ||
| for (auto elem : vec) { | ||
| norm += std::norm(elem); | ||
| } | ||
| return sqrt(norm); | ||
| } | ||
|
|
||
| template <typename T> | ||
| [[nodiscard]] auto isNormalized(std::vector<T> vec) -> bool { |
There was a problem hiding this comment.
vet should be a const reference in all of these cases to avoid a copy.
src/algorithms/StatePreparation.cpp
Outdated
| multiplexer.cx(0, static_cast<Qubit>(localNumQubits - 1)); | ||
| } | ||
|
|
||
| CircuitOptimizer::flattenOperations(multiplexer, false); |
There was a problem hiding this comment.
similar to below you might actually decide to skip this flatten operation to maintain the structure of the circuit.
Description
As a part of my bachelor thesis I implemented the Qiskit State Preparation algorithm (https://github.com/Qiskit/qiskit/blob/main/qiskit/circuit/library/data_preparation/state_preparation.py#) in C++ using the MQT Framework.
My advisor Yannick Stade suggested I open a pull request, as it might be interesting to add into MQT.
The state preparation allows a user to create a circuit that initializes a quantum state from a list of complex amplitudes, sometime needed in various quantum algorithms.
I am not sure how/where to add this class, thus I just created a new file.
If you could give me feedback whether this functionality might be interesting to add/what to change I would gladly try my best to fulfill your requests.
I have not yet written any tests, apart from manually comparing outputs of qiskit and my algorithm, and simulating both circuits with qiskit. (can be added in the future)
Checklist: