Avoid inefficient list construction in qubit_tapering_from_stabilizer.py#1262
Avoid inefficient list construction in qubit_tapering_from_stabilizer.py#1262mhucka wants to merge 6 commits intoquantumlib:mainfrom
qubit_tapering_from_stabilizer.py#1262Conversation
Replaced inefficient `list += [item]` patterns with `list.append(item)` in `_reduce_terms` and `_reduce_terms_keep_length`. This avoids unnecessary temporary list creation and is a standard Python optimization for iterative list building. Tests passed and code review confirmed the correctness of the changes.
There was a problem hiding this comment.
Code Review
This pull request refactors list construction patterns in qubit tapering logic by replacing list concatenation (+=) with list append() calls. While the reviewer suggested further optimization by using list comprehensions for better performance and idiomatic style, the current changes are accepted as an improvement over the previous implementation. I have no further feedback to provide.
|
do you have any performance benchmarks that back up this change? |
@mpharrigan Here is a short benchmark program to demonstrate the performance of import timeit
setup_code = """
def construct_with_iadd(n):
l = []
for i in range(n):
l += [i]
return l
def construct_with_append(n):
l = []
for i in range(n):
l.append(i)
return l
"""
sizes = [10, 100, 1000, 10000, 100000]
print("## Benchmark Results")
print("| N items | `list += [item]` (ms) | `list.append(item)` (ms) | Speedup |")
print("|---------|-----------------------|--------------------------|---------|")
for n in sizes:
# Scale iterations down for larger lists to keep the benchmark fast
number = 10000 if n < 100000 else 1000
t_iadd = timeit.timeit(f"construct_with_iadd({n})", setup=setup_code, number=number)
t_append = timeit.timeit(f"construct_with_append({n})", setup=setup_code, number=number)
# Calculate average time per call in milliseconds
avg_iadd = (t_iadd / number) * 1000
avg_append = (t_append / number) * 1000
speedup = avg_iadd / avg_append
print(f"| {n:<7} | {avg_iadd:<21.4f} | {avg_append:<24.4f} | **{speedup:.2f}x** |")Running this program on a Linux VM using Python 3.12.11 produces the following output: Would this be a sufficient benchmark? |
This replaces some inefficient
list += [item]patterns withlist.append(item)in_reduce_termsand_reduce_terms_keep_lengthof the filequbit_tapering_from_stabilizer.py. This alternative approach avoids unnecessary temporary list creation and is a standard Python optimization for iterative list building.