-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Update and correct pow function for PauliSum #6019
Conversation
Updated pow function for PauliSum to use binary exponentiation
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. |
+Corrected formatting +Corrected initialisation of remainder to identity
+Corrected formatting +Corrected initialisation of remainder to identity
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 add a new test to cirq-core/cirq/ops/linear_combinations_test.py which covers this change, i.e. the test should have failed on the old code and pass on new code.
closes #6017 |
Thanks everyone for your inputs, It was helpful in developing the below understanding: There are two ways to implement the pow function i.e either use a linear multiplication approach or binary exponentiation. To understand the performance aspect of both of them. I am leaving three comments. |
Various Approaches: If we have a PauliSum with n PauliStrings each of length l acting on x qubits. Time complexity to exponentiate it to the power e Linear Multiplication
Time Complexity: Binary exponentiation
Comparing Linear and Binary exponentiation
The reason why this performs worse than linear multiplication: we go in the loop when e reduces to 1 and perform a extra base*base which costs very high Modified Binary exponentiation 1
We are still multiplying if remainder equals identity when we exit the loop, which can be removed Modified Binary exponentiation 2
We are still multiplying if remainder equals identity when we exit the loop, which can be removed Modified Binary exponentiation 3
Observations: Final Binary exponentiation
In all of these we are missing that, every time we multiply PauliSum of n terms with itself - we have n PauliStrings multiplying with each other taking o(n**2) steps. The resultant PauliSum can have number of terms between [1, n**2 - n + 1]. As we exponentiate, more terms will merge to identity and the number of terms in PauliSum decreases Later as we run tests we will observe its effects. |
Example test: (One can vary n, e, and construction of PauliSum)
From test runs we see, for n = 7 and e = 7 the binary exponentiation takes twice as time as linear which is counter intuitive as n**5 + n**6 - n PauliString multiplications should have been prevented in the binary exponentiation approach.
Looking at more details For Binary Overall total multiplications performed = 7 + 7 + 49 + 21*7 + 21*21 + 56*41 = 2947 For Linear 1st iteration in while loop was 0.0014419999999999433 Overall total multiplications performed = 7 + 7*7 + 21*7 + 41*7 + 56*7 + 62*7 + 63*7 + 64*7 = 2205 The rate at which the number of PauliStrings that decreased with increasing power when multipled by constant number of Paulistrings gave better results. Until the exponent saturates(meeting point) the cost of binary exponentiation is higher.This is because: If n = x meeting point is generally 2**(x-1) for the above category of PauliSum. Question is can we say for sure that meeting point cant be behind 2**(x-1) for other inputs as well. If we can prove that it ensures most delayed saturation we can probably implement the pow function as
Intuitively, I chose each PauliString in the PauliSum referencing a unique qubit as it tends to delay thes saturation. For saturation we need X2 = Y2 = Z**2 = -iXYZ = I However, given that we dont have a formal proof and the improvements are for larger values of e which itself increases as n increases. The percentage of cases in which it performs betters looks small. Will be going ahead and pushing the implementation of regular multiplication. |
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.
Nearly there, but we should also test with exponent = 1.
Hi Tarun, when updating the PR, please add new changes as new commits on top of your branch rather than amending old commits, which may result in complicated commit graphs with duplicate original and amended commits. It is also not necessary to merge-in the mainline master in every update - provided the master does not touch files worked on in this PR. It is not a big deal as in the end we squash each PR to a single commit on master, |
Thanks for the comment. I am new to using git, I thought amending a commit would edit the last commit. Now I understand the behavior, that it only amends the last commit on the local repository (on my laptop). The commit on the remote (forked repository) is already written. e.g: I will be adding in new commits on top of my branch (which is main branch of my local repository for this issue, but will create separate branches from now on). |
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 updates Tarun.
LGTM with 2 small suggestions, also please fix the formatting issues reported by the CI check.
Otherwise it is good to go.
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
* Update and correct pow function for PauliSum Updated pow function for PauliSum to use binary exponentiation * Format and remainder initialisation correction +Corrected formatting +Corrected initialisation of remainder to identity * Format and remainder initialisation correction +Corrected formatting +Corrected initialisation of remainder to identity * using local variable instead of self and reformatting * reverting to regular linear multiplication and adding tests * Adding test for range (1,9) * trailing blank * remove trailing blank test * Getting rid of extraneous variables * Fix up final formatting issue --------- Co-authored-by: Tanuj Khattar <tanujkhattar@google.com> Co-authored-by: Pavol Juhas <juhas@google.com>
Updated pow function for PauliSum to use binary exponentiation.