-
Notifications
You must be signed in to change notification settings - Fork 984
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
Create a generalized uniform superposition state gate #6506
Conversation
Creates a generalized uniform superposition state, $\frac{1}{\sqrt{M}} \sum_{j=0}^{M-1} \ket{j} $ (where 1< M <= 2^n), using n qubits, according to the Shukla-Vedula algorithm [SV24]. Note: The Shukla-Vedula algorithm [SV24] offers an efficient approach for creation of a generalized uniform superposition state of the form, $\frac{1}{\sqrt{M}} \sum_{j=0}^{M-1} \ket{j} $, requiring only $O(log_2 (M))$ qubits and $O(log_2 (M))$ gates. This provides an exponential improvement (in the context of reduced resources and complexity) over other approaches in the literature. Reference: [SV24] A. Shukla and P. Vedula, “An efficient quantum algorithm for preparation of uniform quantum superposition states,” Quantum Information Processing, 23(38): pp. 1-32 (2024).
Code tests the creation of M uniform superposition states using generalized_uniform_superposition_gate.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
import cirq | ||
import numpy as np | ||
|
||
def generalized_uniform_superposition_cirqx(M, num_qubits): |
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.
The suffix _cirq is probably unneccessary.
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 comments. The suffix was removed in the updated version.
import cirq | ||
import numpy as np | ||
|
||
def generalized_uniform_superposition_cirqx(M, num_qubits): |
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 is missing type information:
def generalized_uniform_superposition_cirqx(M:int, num_qubits:int)-> 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.
Based on your other comment, now I've implemented a gate-based code. I have taken care to provide the correct type informaiton in the new code.
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
import cirq |
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.
You can't import cirq from within cirq. Make sure to import specific sub-packages.
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.
Thank you for the feedback. Now, only specific sub-packages are being imported.
""" | ||
|
||
if (num_qubits < np.log2(M)): | ||
print('Error Message: Not enough qubits! Try increasing num_qubits ..') |
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 probably raise an error rather than print and return None.
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.
Changed as suggested.
print('Error Message: Not enough qubits! Try increasing num_qubits ..') | ||
return | ||
|
||
qreg = cirq.LineQubit.range(num_qubits) |
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 implementation forces the circuit to use the first few LineQubits. If people want to use GridQubits or any other qubits, they are out of luck (or need to transform qubits afterward.
I think this would make more sense as a Gate / Operation, where these concepts already exist (see some compositie operations, like QFT for examples). Alternatively, I suppose this could pass the qubits in.
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.
Thank you for this suggestion. Following this feedback, I implemented a gate-based approach.
qreg.reverse() | ||
qcircuit = cirq.Circuit() | ||
|
||
# Delete the line below |
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.
Please delete.
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.
Done.
k = len(N) | ||
L = [index for (index,item) in enumerate(N) if item==1] #Locations of '1's | ||
|
||
for i in L[1:k]: |
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 some line comments would be helpful throughout this function.
for i in L[1:k]: | ||
qcircuit.append(cirq.X(qreg[i])) | ||
|
||
Mcurrent = 2**(L[0]) |
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.
Generally, local variables should be lowercase with underscores. I'm reluctant to be too strict if the variables correspond to variables in the paper, but, if possible, we should stick to that naming scheme.
See this style guide:
https://google.github.io/styleguide/pyguide.html#316-naming
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 feedback. I followed the notation used in the referenced paper. It would be helpful to keep it this way and maintain the same variable names. I'll stick to the suggested naming convention in my future submissions.
|
||
Mcurrent = Mcurrent + 2**(L[m]) | ||
|
||
qreg.reverse() |
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.
Why reverse the qubit order here? I'm not sure this has any effect.
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.
Removed this from the updated code.
import cirq | ||
import numpy as np | ||
|
||
def check_uniform_superposition_error(M, n): |
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.
Add docstring. Also consider using a prefix underscore to denote it as a private function.
I would come up with a more specific name to specify what this is checking. For instance, asssert_generated_unitary_is_unifrom or something like that.
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.
Thank you for the feedback. Suggested changes were made.
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 appreciate it if you could please review the changes and provide your feedback/approval at your earliest convenience.
…comments Thanks to dstrain115 for the review and helpful comments. The following changes were made based on these comments: 1) The code has been modified to implement the functionality as a Gate. 2) The missing type information was provided. 3) The code for appropriate value errors was included
…view comments. Added the docstrings and changed the name of the function to make it more meaningful.
Fixed a minor typo and added some comments for clarification.
@vtomole @dstrain115 - The code appears to be working fine on my end. Could you please review it and provide your feedback? |
What's the use case for this? Why are we adding this to Cirq?
Uniform superposition states can be prepared using a single round of amplitude amplification using a single ancilla qubit. See https://github.com/quantumlib/Qualtran/blob/b9c5b5bd33d17f35fe027ff5362dd0b2f6887048/qualtran/bloqs/state_preparation/prepare_uniform_superposition.py#L29 for more details. Can you highlight what's the difference of your approach against previously known results like Fig 12 of https://arxiv.org/pdf/1805.03662.pdf which is implemented in the Qualtran link I've shared above? Also, I think Qualtran might be a better repository for algorithmic primitives like this. @dstrain115 Is this needed in Cirq for a specific use case I'm not aware of? |
@tanujkhattar Thanks for your comments. Please note the following points about the approach suggested in Fig 12 of https://arxiv.org/pdf/1805.03662.pdf vs the approach used in the code in this PR: Considering the above points, it seems the submitted code provides a simpler (just a few lines of code), cleaner, and deterministic approach for the preparation of uniform superposition states as compared to the probabilistic approach given in the arxiv reference. Next coming to - What's the use case for this? Why are we adding this to Cirq? I guess creating uniform superposition states is an important algorithm - it is the first step in many important quantum algorithms - including the case when the number of uniform superposition states N is not of the form of 2^n, which will be useful for a variety of applications. I am open to your comments and suggestions. |
Implemented the suggested change.
Fixed formatting.
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.
@prag16 Hi gates defined incirq.ops
have to provide certain functionalities so please don't turnoff any tests. if you don't want to implement all the functionalities required for a cirq.Gate then you can move the gate to cirq-core/cirq/contrib
where we accept nonproduction contributions
@NoureldinYosri - I turned off the json-related tests temporarily to ensure that all the other tests are passing first (and we are pretty close to that). Next, after including the json-related functionality, I'll turn these tests back on and request your review. |
Fixed log2(m_value) implementation error.
Included some more test cases (in addition to random tests) to ensure coverage.
Removed UnifromSuperpositionGate from not_yet_serializable
Removed cirq.ops.UniformSuperpositionGate form skip_classes
Added UniformSuperpositionGate
Added _json_dict_ and related functionality
Added json and repr tests
@NoureldinYosri - I implemented the json-related functionality and turned on all the tests. All the checks are passing at my end. Can you please run the ci tests at your end to confirm? |
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.
LGTM%fix failing CI
Corrected formatting and added more tests to fix ci coverage check errors
@NoureldinYosri - Thanks for reviewing the changes. Now, I've corrected the formatting and added more tests to fix CI coverage check errors. Hopefully, the CI tests should pass now. |
Added one more test case
@NoureldinYosri - I have taken care of the last remaining coverage error and also fixed the format error. The CI checks are passing locally for me, Hopefully, the checks will pass at your end as well. |
Fixed the format check error
@NoureldinYosri - It is good that all the tests passed (except for the format check). I have fixed this issue now. Could you please run the CI tests on your end? |
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.
@prag16 thanks for your contribution, great work
@NoureldinYosri - Thank you very much for reviewing the code and for your helpful suggestions! I really appreciate it! |
* Create generalized_uniform_superposition_gate.py Creates a generalized uniform superposition state, $\frac{1}{\sqrt{M}} \sum_{j=0}^{M-1} \ket{j} $ (where 1< M <= 2^n), using n qubits, according to the Shukla-Vedula algorithm [SV24]. Note: The Shukla-Vedula algorithm [SV24] offers an efficient approach for creation of a generalized uniform superposition state of the form, $\frac{1}{\sqrt{M}} \sum_{j=0}^{M-1} \ket{j} $, requiring only $O(log_2 (M))$ qubits and $O(log_2 (M))$ gates. This provides an exponential improvement (in the context of reduced resources and complexity) over other approaches in the literature. Reference: [SV24] A. Shukla and P. Vedula, “An efficient quantum algorithm for preparation of uniform quantum superposition states,” Quantum Information Processing, 23(38): pp. 1-32 (2024).
Creates a generalized uniform superposition state,$$\lvert \Psi \rangle = \frac{1}{\sqrt{M}} \sum_{j=0}^{M-1} \ket{j}$$ (where $1 < M \leq 2^n$ ), using $n$ qubits, according to the Shukla-Vedula algorithm [SV24]. Note that when $M$ is of the form $2^n$ , creating uniform superposition states is trivial (just apply $n$ Hadamard gates on $\ket{0}^{\otimes n}$ ). However, in the general case, when $M$ is not of this form, creating uniform superposition states is non-trivial. The Shukla-Vedula algorithm [SV24] offers an efficient approach for the creation of a generalized uniform superposition state $ \lvert \Psi \rangle$ (as defined above) requiring only $O(\log_2 (M))$ qubits and $O(\log_2 (M))$ gates. This provides an exponential improvement (in the context of reduced resources and complexity) over other approaches in the literature.
Args:$M (> 1)$ representing the number of computational basis states with an amplitude of $\frac{1}{\sqrt{M}}$ in the uniform superposition state $$\lvert \Psi \rangle = \frac{1}{\sqrt{M}} \sum_{j=0}^{M-1} \ket{j}.$$ Note that the remaining $(2^n - M)$ computational basis states have zero amplitudes. Here $M$ need not be an integer power of $2$ .
M (integer):
A positive integer
References:
[SV24] A. Shukla and P. Vedula, “An efficient quantum algorithm for the preparation of uniform quantum superposition states,” Quantum Information Processing, 23(38), pp. 1-32 (2024).