Skip to content

Optimize the HHL algorithm using amplitude amplification #7141

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

codrut3
Copy link
Contributor

@codrut3 codrut3 commented Mar 16, 2025

This addresses #2216 .

Example output from a run:

Expected observable outputs:
X = 0.14413
Y = 0.413217
Z = -0.899154
Actual values without amplitude amplification: 
X = 0.11042471042471047 from 1295 estimates out of 5000 simulation runs
Y = 0.4290171606864275 from 1282 estimates out of 5000 simulation runs
Z = -0.9037095501183898 from 1267 estimates out of 5000 simulation runs
Success probability of a single run without amplitude amplification: 0.2562666666666667
Actual values with amplitude amplification: 
X = 0.14910226385636227 from 1281 estimates out of 2938 simulation runs
Y = 0.3879781420765027 from 1281 estimates out of 4126 simulation runs
Z = -0.901639344262295 from 1281 estimates out of 3477 simulation runs

To highlight the effect of amplitude amplification, I run the algorithm twice, once without amplification, and once with, targeting the same number of estimates. Amplitude amplification reduces the number of measurements needed for an estimate (from O(1/p) to O(1/sqrt{p})). This shows up as fewer simulation runs.

codrut3 added 2 commits March 16, 2025 11:54
I changed the example to simulate two circuits: the first one is the
previous HHL circuit in the example without amplitude amplification,
and the second one is the HHL circuit with amplitude amplification.

The example demonstrates that amplitude amplification obtains an estimate
faster (using fewer total calls to simulate.run).
@codrut3 codrut3 requested review from vtomole and a team as code owners March 16, 2025 11:41
@codrut3 codrut3 requested a review from maffoo March 16, 2025 11:41
@codrut3
Copy link
Contributor Author

codrut3 commented Mar 25, 2025

I am happy to provide more explanations about the change if it helps with the review. Just let me know!

Copy link
Collaborator

@dstrain115 dstrain115 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to provide some tests for this class, which would help increase confidence that this large code change is working correctly.

If possible, it might be good to come to the cirq-cync on Wednesdays to discuss this so that everyone understands the changes. If not possible, we could probably discuss over GH comments.

examples/hhl.py Outdated
cirq.measure(ancilla, key='a'),
]
)
def __init__(self, A, C, t, register_size, *input_prep_gates):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add types for all the arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

examples/hhl.py Outdated
results.append(np.concatenate(results_for_param, axis=0))
runs.append(num_runs)

for label, result, num_runs in zip(('X', 'Y', 'Z'), results, runs):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be better and more testable if we return a result object and then print it out after it returns.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I moved this to a separate method print_results.

examples/hhl.py Outdated

return circuit

def simulate_without_amplification(self, total_runs):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing type annotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

examples/hhl.py Outdated

return c

def measure_circuit(self, circuit):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type annotations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

examples/hhl.py Outdated
c.append(cirq.X.on_each(qubits))
return c

def amplitude_amplification(self, num_iterations):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type annotations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@codrut3
Copy link
Contributor Author

codrut3 commented Apr 1, 2025

If possible, it might be good to come to the cirq-cync on Wednesdays to discuss this so that everyone understands the changes. If not possible, we could probably discuss over GH comments.

Thank you! I joined the cirq-dev group (my name is Codrut). Please send me the meeting invite, I'll come.

@verult verult self-requested a review April 2, 2025 17:34
Copy link

codecov bot commented Apr 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.65%. Comparing base (02941bf) to head (f925b35).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7141   +/-   ##
=======================================
  Coverage   98.65%   98.65%           
=======================================
  Files        1106     1106           
  Lines       95869    95939   +70     
=======================================
+ Hits        94581    94651   +70     
  Misses       1288     1288           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pavoljuhas pavoljuhas added the priority/after-1.5 Leave for after the Cirq 1.5 release label Apr 4, 2025
@codrut3
Copy link
Contributor Author

codrut3 commented Apr 6, 2025

Thank you for the comments! I added type annotations and created a separate method to print the results.

I can add tests next. However, I don't fully understand the initial example, which prevents me from writing some unit tests. How is PhasedXPowGate used to measure X, Y and Z? When I replace the exponent parameters I don't get the matrices for Pauli X, Y or Z. Furthermore, I am not sure how the expected[] array in the example was derived. @verult it would help me a lot if you could give me some hints here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/after-1.5 Leave for after the Cirq 1.5 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants