-
Notifications
You must be signed in to change notification settings - Fork 627
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
Improve expand_matrix method #3064
Conversation
* Incrementing the version number to `v0.27.0-dev` (#3044) * Update * Bump * re-add dev changelog * Update doc/development/release_notes.md * docs section * flaky test_single_argument_step Co-authored-by: Antal Szava <antalszava@gmail.com> * fix exp bug * rebase onto release candidate * Update doc/releases/changelog-0.26.0.md * Update tests/optimize/test_optimize_shot_adapative.py * add additional test parametrization Co-authored-by: Romain Moyard <rmoyard@gmail.com> Co-authored-by: Antal Szava <antalszava@gmail.com>
Hello. You may have forgotten to update the changelog!
|
* Update error message to be more descriptive * bug fix * Update doc/releases/changelog-0.26.0.md * split test and checked for more verbose error message * update error message to mention splitting of execution
* simplify output updates * is_hermitian dagger update * dagger for is_unitary
Codecov Report
@@ Coverage Diff @@
## master #3064 +/- ##
=======================================
Coverage 99.68% 99.68%
=======================================
Files 273 273
Lines 23671 23683 +12
=======================================
+ Hits 23597 23609 +12
Misses 74 74
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Here is the script I use for benchmarks: import functools
import random
from time import time
import matplotlib.pyplot as plt
import numpy as np
import pennylane as qml
random.seed(123)
NUM_WIRES = 20
NUM_OPERANDS = 20
def build_new_sum_op(num: int):
summands = [qml.s_prod(0.5, qml.PauliX(i)) for i in num]
return qml.op_sum(*summands)
def build_old_sum_op(num: int):
coeffs = [0.5 for _ in num]
ops = [qml.PauliX(i) for i in num]
return qml.Hamiltonian(coeffs=coeffs, observables=ops)
def build_new_prod_op(num: int):
factors = [qml.s_prod(0.5, qml.PauliX(i)) for i in num]
return qml.prod(*factors)
def build_old_prod_op(num: int):
tensor = functools.reduce(lambda x, y: x @ y, [qml.PauliX(i) for i in num])
return qml.Hamiltonian(coeffs=[0.5], observables=[tensor])
x = np.arange(2, NUM_OPERANDS)
prod_old_times = []
prod_new_times = []
sum_old_times = []
sum_new_times = []
for n in x:
print(n)
random_numbers = [random.randint(0, NUM_WIRES) for _ in range(n)]
new_prod_op = build_new_prod_op(random_numbers)
old_prod_op = build_old_prod_op(random_numbers)
new_sum_op = build_new_sum_op(random_numbers)
old_sum_op = build_old_sum_op(random_numbers)
start = time()
new_prod_op.matrix()
stop = time()
prod_new_times.append(stop - start)
start = time()
qml.matrix(old_prod_op)
stop = time()
prod_old_times.append(stop - start)
start = time()
new_sum_op.matrix()
stop = time()
sum_new_times.append(stop - start)
start = time()
qml.matrix(old_sum_op)
stop = time()
sum_old_times.append(stop - start)
fig, (ax1, ax2) = plt.subplots(1, 2)
fig.suptitle(f"Matrix (num_wires = {NUM_WIRES})")
ax1.set_title("Prod(PauliX(0), PauliX(1), ...) (random)")
ax1.set_ylabel("new_time / old_time")
ax1.set_xlabel("# factors")
ax1.bar(x, np.array(prod_new_times) / np.array(prod_old_times), width=0.8)
ax1.set_xticks(x[::2])
ax2.set_title("Sum(PauliX(0), PauliX(1), ...) (random)")
ax2.set_ylabel("new_time / old_time")
ax2.set_xlabel("# summands")
ax2.bar(x, np.array(sum_new_times) / np.array(sum_old_times), width=0.8)
ax2.set_xticks(x[::2])
plt.tight_layout()
plt.show() |
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.
Looks good! Just some clarifying questions for me to better understand the implementation. Also Missing some tests! Other then that, looks good 👍🏼
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 provided benchmarking script seems to still have a bit of random noise and variation, even though a seed is set. But very impressive improvement for sums with large numbers of wires. A bit cleaner and easier to follow now too 👍
Co-authored-by: Christina Lee <christina@xanadu.ai>
Co-authored-by: Christina Lee <christina@xanadu.ai>
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.
Hi @AlbertMitjans, Nice job here!
I'm happy that this branch outperformed the master in all cases considered in the benchmark.
I'm attaching here a comparison.
One more thing. Could you please check here? |
Co-authored-by: Amintor Dusko <87949283+AmintorDusko@users.noreply.github.com>
Co-authored-by: Amintor Dusko <87949283+AmintorDusko@users.noreply.github.com>
Change dense matrix expansion method, which improves performance. We use the same method that is used for sparse matrices. Consequently, we have merged the two methods (they had a lot of duplicated lines of code).
The current method does the following:
wire_order
that contains all labels inwires
+ expandmat
if this subset is larger than the matrix wires.mat
to matchsubset_wire_order
.qml.math.transpose
to permute the wires and then we reshape back.