-
Notifications
You must be signed in to change notification settings - Fork 361
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
Support BackendV2 #1875
Support BackendV2 #1875
Conversation
17028f2
to
2826216
Compare
qiskit_aer/primitives/estimator.py
Outdated
new_circ.add_register(creg) | ||
new_circ.compose(meas_circuit, inplace=True) | ||
_update_metadata(new_circ, meas_circuit.metadata) | ||
if circuit.num_qubits >= meas_circuit.num_qubits: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be False
. If you get it, it is lack of validation.
Use passmanager for measurement circuits
Qiskit/qiskit#10956 will fix one of the Neko error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this LGTM. I left a couple of inline questions and a comment about the release notes that need a little expansion, but not worth blocking on any of these. Sorry it took me so long to look at this. Using the v1 -> v2 converter is a good path to supporting the v2 interface in the short term and makes this migration fairly simple and straightforward.
Longer term we'll need to start moving away from using configuration, properties, and defaults as they'll be deprecated in qiskit in the near future (probably not until April next year).
The neko test failures do point to some compatibility issues with using v2 here (for example it looks like QuantumInstance
assume aer is always backendv1). How we handle those is up to @doichanj as documenting them as a breaking change is a viable path forward, but then we need to make sure there is a release note explaining this in greater depth.
--- | ||
features: | ||
- | | ||
Update Aer Backend to BackendV2. Refer to `#1681<https://github.com/Qiskit/qiskit-aer/issues/1681>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a bit more detail to document the user upgrade implications. The only one that I can think of is backend.name
is now a string attribute not a method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the detailed description to the release note.
@@ -123,7 +123,8 @@ def test_extended_stabilizer_sparse_output_probs(self): | |||
for i in range(1, nqubits): | |||
circ.cx(i - 1, i) | |||
circ.measure_all() | |||
circ = transpile(circ, backend) | |||
|
|||
# circ = transpile(circ, backend) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the simulation time becomes so long that the timeout occurs. This transpile method adds non-clifford gate to the circuit so extended stabilizer simulation requires long simulation time. If the time until timeout is enough long and the simulation is completed, the test is passed without this comment out.
@@ -46,7 +46,7 @@ def test_seed_simulator_option_measure(self, method, device): | |||
qc.h([0, 1]) | |||
qc.reset(0) | |||
qc.measure_all() | |||
qc = transpile(qc, backend) | |||
qc = transpile(qc, backend, optimization_level=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to use level 0 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with transpile>0, H gate is converted to P gate in extended_stabilizer method that takes very long time to run this circuit. In backendV1, H gate was not converted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember this now, the issue tracking this in qiskit is: Qiskit/qiskit#10568
Actually on reflection, I'm a bit concerned about the neko failures, because as soon as aer 0.13.0 releases that will cause neko to fail on all other projects unless they pin to |
I created fix to solve backend.name() error Qiskit/qiskit#11065 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, we are ready to merge it
* Support backendv2 * Change API of aerbackend init * fix lint * Fix lint * Add reset gate * Return None if the configuration does not have max_experiments * Change function to constant * Update code to pass the test * Remove print * Fix lint * Change num of qubits in Estimator * Skip transpilation * Change transpile optimization level * Add release notes * Change process of cirucit compose by the number of qubits in estimator * use passmanager for measurement circuits * refactor (change line order) * Fix lint * Add a detail description to the release note --------- Co-authored-by: ikkoham <ikkoham@users.noreply.github.com> Co-authored-by: Hiroshi Horii <hhorii@users.noreply.github.com> Co-authored-by: Jun Doi <doichan@jp.ibm.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Summary
Resolve #1681
Details and comments
Add method in aerbackend class to create Target and return name.