Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

Migrate Quantum Volume circuits to use circuit library #460

Merged
merged 5 commits into from
Jul 31, 2020
Merged

Migrate Quantum Volume circuits to use circuit library #460

merged 5 commits into from
Jul 31, 2020

Conversation

mtreinish
Copy link
Collaborator

@mtreinish mtreinish commented Jul 29, 2020 •
edited
Loading

Summary

This commit starts the process of using the terra circuits library by
refactoring the quantum volume circuit generator function to internally
use the quantum volume circuit from the library instead of constructing
it manually. It also tries to rationalize the input parameters since
several didn't have any effect and others were not properly listed as
required. This is done in a backwards compatible manner to not break
existing users.

Moving forward we probably want to deprecate this entire function and write a new implementation that has simpler and more flexible inputs. The current function is both trying to do too much (ie handle layout via qubit_lists) and not doing enough (ie not giving flexibility on depth). We should figure out how we expect people to want to be able run QV experiments and what the inputs to that would look like and use that as the model for how circuit generation is done.

Details and comments

Partially addresses #395

@mtreinish
Migrate Quantum Volume circuits to use circuit library
0a98095 
This commit starts the process of using the terra circuits library by
refactoring the quantum volume circuit generator function to internally
use the quantum volume circuit from the library instead of constructing
it manually. It also tries to rationalize the input parameters since
several didn't have any effect and others were not properly listed as
required.

Partially addresses qiskit-community#395
@mtreinish mtreinish requested a review from chriseclectic July 29, 2020 18:00
@ShellyGarion
Copy link
Contributor

ShellyGarion commented Jul 30, 2020 •
edited
Loading

Also generate_data for the tests should be updated to include seed:
https://github.com/Qiskit/qiskit-ignis/blob/master/test/quantum_volume/generate_data.py

mtreinish added 2 commits July 30, 2020 06:53
@mtreinish
Remove unused new arg
6a7893d
@mtreinish
Remove storaed data and run Aer during fitter test
936d64e 
Previously the results from executionn of qv circuits on Aer were run
once and stored in the repo. Then the fitters tests would just load the
stored results and fit that data. This was presumably done for run time,
however in practice it only takes a few seconds to simulate the circuits
and is not a significant overhead. This commit removes the generated
data and moves the circuit execution pieces from the generation script
into the test file.
@mtreinish
Copy link
Collaborator Author

mtreinish commented Jul 30, 2020 •
edited
Loading

Also generate_data for the tests should be updated to include seed:
https://github.com/Qiskit/qiskit-ignis/blob/master/test/quantum_volume/generate_data.py

I did some testing locally and adding the QV circuit simulations in the fitter test only took about 5-6 seconds on my laptop. So instead of updating the results I just baked the simulation into the fitter test directly in 936d64e. That way we don't have to worry about updating it every time.

@mtreinish mtreinish changed the title [WIP] Migrate Quantum Volume circuits to use circuit library Migrate Quantum Volume circuits to use circuit library Jul 31, 2020
@chriseclectic chriseclectic merged commit 6fe2999 into qiskit-community:master Jul 31, 2020
mtreinish added a commit to mtreinish/qiskit-ignis that referenced this pull request Aug 4, 2020
@mtreinish
Replace QV shallow copy with deepcopy in QV circuit generation
fb7ab5e 
In qiskit-community#460 we migrated the QV circuit generation to use the QV circuit from
terra's circuit library instead of constructing it from scratch. As part
of that the circuit generation was updated to construct a single virtual
circuit via the library and just copy it for the no measure output
circuits. However, this was done using a shallow copy (in the interest
of performance) which means that any references between the circuits
will not be duplicated. For cases where the qubit_lists input is not a
contiguous list this would result in the measurements showing up in the
no measurement circuits because of shared references. This commit fixes
this by using deepcopy instead of a shallow copy so that there are no
shared references between the measurement circuit and the no measurement
circuit.
mtreinish added a commit to mtreinish/qiskit-ignis that referenced this pull request Aug 4, 2020
@mtreinish
Replace shallow copy with deepcopy in QV circuit generation
fd3da56 
In qiskit-community#460 we migrated the QV circuit generation to use the QV circuit from
terra's circuit library instead of constructing it from scratch. As part
of that the circuit generation was updated to construct a single virtual
circuit via the library and just copy it for the no measure output
circuits. However, this was done using a shallow copy (in the interest
of performance) which means that any references between the circuits
will not be duplicated. For cases where the qubit_lists input is not a
contiguous list this would result in the measurements showing up in the
no measurement circuits because of shared references. This commit fixes
this by using deepcopy instead of a shallow copy so that there are no
shared references between the measurement circuit and the no measurement
circuit.
@mtreinish mtreinish mentioned this pull request Aug 4, 2020
Merged
chriseclectic pushed a commit that referenced this pull request Aug 5, 2020
@mtreinish
Replace shallow copy with deepcopy in QV circuit generation (#462)
a705a6e 
* Replace shallow copy with deepcopy in QV circuit generation

In #460 we migrated the QV circuit generation to use the QV circuit from
terra's circuit library instead of constructing it from scratch. As part
of that the circuit generation was updated to construct a single virtual
circuit via the library and just copy it for the no measure output
circuits. However, this was done using a shallow copy (in the interest
of performance) which means that any references between the circuits
will not be duplicated. For cases where the qubit_lists input is not a
contiguous list this would result in the measurements showing up in the
no measurement circuits because of shared references. This commit fixes
this by using deepcopy instead of a shallow copy so that there are no
shared references between the measurement circuit and the no measurement
circuit.

* Fix lint

* Adjust seed usage to account for QuantumVolume class rng usage

The seed parameter beign passed directly to the QuantumVolume circuit
library resulted in the generated circuits all being identical. To
ensure that we get reproducible but useful output this changes the seed
usage to create it's own random Generator and use the seed parameter as
the seed for that. Then the generator is used to generate a sequence of
random integers which get passed to each QuantumVolume object.
@mtreinish mtreinish added Changelog: Deprecation Include in Deprecated section of changelog Changelog: New Feature Include in the Added section of the changelog labels Aug 6, 2020
@mtreinish mtreinish deleted the refactor-qv branch August 6, 2020 14:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chriseclectic chriseclectic chriseclectic approved these changes

Assignees
No one assigned
Labels
Changelog: Deprecation Include in Deprecated section of changelog Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mtreinish @ShellyGarion @chriseclectic