58 run first hardware benchmarks #110
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Initial dockerfile, breaking out requirements. * Install requirements in Dockerfile, add -e pip flag * Automated deploy script to AWS * merge in new shell script changes (#104) * Move and expand test for logical equivalence Test moved to test_compile.py * Delete test_circuit_equivalence.py Test moved to test_compile.py * Remove unnecessary qubit parameterizations Remove unnecessary qubit parameterizations in `test_compiled_circuits_equivalent` and `test_compilation_retains_gateset` * add basic expectation value benchmark (#96) * add basic expectation value benchmark * move compile function to `common.py` * write data out to json * make data pipeline more functional * simplify real check * Add bug label to new bug report issues * Add feature tag to feature template issues * 97 reorg files (#98) * add basic expectation value benchmark * move compile function to `common.py` * write data out to json * make data pipeline more functional * simplify real check * Update naming convention for different benchmarks * Dry up save results into common function * Fix init import issue * Move gate_counters into scripts/common.py for simplicity * Fix notebook errors, remove old data * Remove duplicate funciton definitions * Remove duplicate lines * Add folder to docstring, call out alterntive benchmark name --------- Co-authored-by: nate stemen <nate@unitary.fund> --------- Co-authored-by: Misty-W <82074193+Misty-W@users.noreply.github.com> Co-authored-by: nate stemen <nate@unitary.fund> * Update PyTKET rebase pass --------- Co-authored-by: Misty-W <82074193+Misty-W@users.noreply.github.com> Co-authored-by: nate stemen <nate@unitary.fund>
…/unitaryfund/ucc into 58-run-first-hardware-benchmarks
natestemen
reviewed
Dec 3, 2024
There was a problem hiding this comment.
Nice work taking UCC to hardware! A really important step towards assessing its viability.
Overall this looks really good! I have a few small notes/questions, but the fact that it runs is awesome.
- I think there are some changes unrelated to hardware benchmarks in this PR. Is that intended?
- Can we plot error in the observables rather than raw values?
- Do you know why the UCC circuit contains a
$R_z(-\pi/2)R_z(\pi/2)$ sequence? I guess that's introduced in the mapping phase somehow?
Meta note: reviewing python notebooks on GitHub is not ideal since we lack the capability to comment on specific lines in the rich-diff. The raw-diff works fine, but then it's hard to tell what's happening in the code/images. I'm not sure if there is a worthwhile workaround right now, however.
There was a problem hiding this comment.
should this file be contained in this PR? It seems unrelated to hardware benchmarks.
Dockerfile
Outdated
There was a problem hiding this comment.
This seems unrelated to hardware benchmarks.
| coupling_list = target_device.get_edges() | ||
| elif isinstance(target_device, list): | ||
| # What kind of list is this? |
There was a problem hiding this comment.
Should we make this a TODO or an issue?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For this small initial test on a Bell state, UCC looks like it's not quite as performant as Qiskit on IBM's own hardware.
I'm pretty sure this is down to an inefficient mapping pass in UCC. The circuit UCC generated in this case were more than twice as long as Qiskit:
Qiskit hardware mapped circuit
UCC hardware mapped circuit
