Skip to content
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

Plot expectation value benchmarks #168

Merged
merged 37 commits into from
Feb 13, 2025
Merged

Plot expectation value benchmarks #168

merged 37 commits into from
Feb 13, 2025

Conversation

natestemen
Copy link
Member

@natestemen natestemen commented Jan 14, 2025

Description

This PR refactors the expectation value benchmarking script to ensure it works with the run_benchmarks.sh script. It also introduces new circuits to broaden the test of the expectation value testing. Scripts to visualize relative and absolute errors across different compilers over time are added, with one plot added to the README.

@jordandsullivan we have the option to plot relative or absolute error, but relative is much higher for ucc, which I don't understand the reason for.

@Misty-W Misty-W linked an issue Jan 14, 2025 that may be closed by this pull request
@natestemen natestemen marked this pull request as ready for review January 15, 2025 05:33
Copy link
Collaborator

@jordandsullivan jordandsullivan left a comment

Choose a reason for hiding this comment

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

Thanks for your work in this! Great getting to hack in person together.

I'm wondering why are the relative errors in the hundreds in the first place? What are the actual expectation values? I'd say we want to report errors in terms of a percent.

@natestemen
Copy link
Member Author

why are the relative errors in the hundreds

I think it's because the ideal values are coming out so close to 0 that the relative errors are blowing up. You can find all the most recent results in benchmarks/results/expval_2025-01-14_20.csv. E.g. for QFT the ideal expectation value (last column) is $\mathcal{O}(10^{-21})$ so it's easy to be $>100\%$ off.

ucc,qft,ZZZZZZZZZZ,4.7704895589362195e-18,4.771491994589455e-18,4759.8985323285315,-1.002435653235501e-21
qiskit,qft,ZZZZZZZZZZ,8.673617379884035e-19,8.68364173641639e-19,866.2542786051877,-1.002435653235501e-21
pytket,qft,ZZZZZZZZZZ,-3.2526065174565133e-18,3.2516040818032778e-18,3243.703544769454,-1.002435653235501e-21
cirq,qft,ZZZZZZZZZZ,2.168404344971009e-19,2.178428701503364e-19,217.31356965129692,-1.002435653235501e-21

Not sure what the best course of action here is.

@jordandsullivan
Copy link
Collaborator

jordandsullivan commented Jan 15, 2025

Okay just as a sanity check can you plot the simulated and ideal expectation values and standard deviation, similar to what I did here for #58 (where I was running on real hardware)?
Screenshot 2025-01-15 at 1 10 15 PM

This reminds me, we can also simply measure an array of observables in addition to ZZZZZZ like I did there. Maybe just adding in some that measure like XIIIIIII or XXXXXZZZZZ, etc.

Copy link
Collaborator

@jordandsullivan jordandsullivan left a comment

Choose a reason for hiding this comment

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

Looking good, a few suggestions.

@bachase
Copy link
Collaborator

bachase commented Feb 10, 2025

@natestemen #220 (and f6a53bd) in particular is my attempt to fix the cirq issue. Let me know if this helps with your noise model approach (or I can try testing that myself too)

@natestemen
Copy link
Member Author

I noticed there are many changes to the qasm files. Can you summarize why they had to change?

There are some new additions (circuits of sizes that are reasonable to simulate). These were added so we could have something to test one. If you see any QASM files that were modified let me know, because that was definitely not the intention.

@natestemen natestemen requested a review from Misty-W February 11, 2025 15:41
@Misty-W Misty-W modified the milestone: 0.4.3 Feb 11, 2025
@natestemen natestemen requested a review from bachase February 11, 2025 17:11
Copy link
Collaborator

@bachase bachase left a comment

Choose a reason for hiding this comment

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

Looks great overall.

Several nits and one typo to consider.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Was it intentional to delete this file?

ax.set_xlabel("Date")
ax.set_ylabel("Average Absolute Error")
ax.grid(True)
ax.legend(title="Compiler")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to use the same version labels shown in the other plots?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm I think probably yes, but I'll leave that for a subsequent PR.

@natestemen natestemen requested a review from bachase February 12, 2025 22:26
Copy link
Collaborator

@Misty-W Misty-W left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @natestemen !


simulator = AerSimulator(
method="density_matrix", noise_model=depolarizing_noise
def parse_arguments() -> tuple[str, str, str, bool]:
Copy link
Collaborator

@bachase bachase Feb 13, 2025

Choose a reason for hiding this comment

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

Another style nit for the future -- we should consider [argparse](https://docs.python.org/3/library/argparse.html) or [click](https://click.palletsprojects.com/en/stable/) for managing command line parsing. Likely some other libraries out there these days too.

for qasm_file in "${QASM_EXPVAL_FILES[@]}"; do
for compiler in "${COMPILERS[@]}"; do
# Combine the common folder path with the QASM file
full_qasm_file="${QASM_FOLDER}${qasm_file}"

# Build the command, passing the results folder as an argument
command="python3 $(dirname "$0")/expval_benchmark.py \"$full_qasm_file\" \"$compiler\" \"$RESULTS_FOLDER\" $LOG_EXPVAL"
command="poetry run python $(dirname "$0")/expval_benchmark.py \"$full_qasm_file\" \"$compiler\" \"$RESULTS_FOLDER\" $LOG_EXPVAL"
Copy link
Collaborator

Choose a reason for hiding this comment

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

One thing to be careful about here is if this script itself is run via poetry, which is how I think the prior benchmark runs were done in the github actions (e.g. they were run via poetry run ./benchmarks/scripts .....

update Suprisingly, this seems to work!

Copy link
Collaborator

@bachase bachase left a comment

Choose a reason for hiding this comment

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

Looks great and thanks for all the changes.

I'll create a few issues for some of the nits/style suggestions that were broader than just this PR

@natestemen natestemen merged commit b92d2fd into main Feb 13, 2025
1 check passed
@natestemen natestemen deleted the plot-expectation-value branch February 13, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add expectation value plot to GH benchmarks pipeline
4 participants