Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Enable Aer's parameter binding #1206

Merged
merged 6 commits into from Aug 24, 2020

Conversation

hhorii
Copy link
Contributor

@hhorii hhorii commented Aug 18, 2020

Summary

Use Aer's parameter-binding if QasmSimulator or StatevectorSimulator is used

Details and comments

Enable Aer's parameter-binding instead of Terra's.

This PR assumes that Parameter is the only parameter type in CircuitSampler. Currently, CircuitSampler.convert() is called only by VQE. VQE uses only Parameter (and not ParameterVector and ParameterExpression) for parameter binding.

@hhorii hhorii force-pushed the use_aer_param_bindings branch 4 times, most recently from 201f7ad to acbb4be Compare August 19, 2020 05:09
@woodsp-ibm
Copy link
Member

@hhorii Hi, thanks for looking at this. Do you have any result around what sort of performance improvement can be expected? This presumably avoids the assemble stage each time - but presently this is custom to Aer it seems?

Also do you think there is anyway to have this custom Aer parameterization in the Quantum Instance so other algorithms using parameterized ccts e.g VQC could take advantage of it too, and use it there from CircuitSampler rather than have it i CircuitSampler? In general we have tried to keep backend/execution capabilities in Quantum Instance to allow more general usage throughout Aqua.

@hhorii
Copy link
Contributor Author

hhorii commented Aug 19, 2020

@woodsp-ibm
image
(Xeon Gold 6140 (2.30GHz, 18core x 2HT) x 2socket, 384GB DDR4RAM)

I used MolecularGroundStateEnergy with qiskit.chemistry.components.variational_forms.UCCSD as var_form, qiskit.aqua.components.optimizers.SLSQP(maxiter=5000) as optimizer while enabling Aer's expectation value calculation (include_custom=True).

To make this optimization more general, I guess that QuantumInstance.execute() need take parameterized circuits and param_bindings. In addition, QuantumInstance needs to cache executed circuits in the previous execute(). In Aer, parameter values of each gate are replaced with specified values. In other word, Aer needs a dummy circuit that does not have any ParameterExpression (In this PR, _transpiled_circ_templates is a dummy. It is sent to Aer for every execute()).

@hhorii hhorii changed the title [WIP] Enable Aer's parameter binding Enable Aer's parameter binding Aug 19, 2020
@woodsp-ibm
Copy link
Member

woodsp-ibm commented Aug 19, 2020

On the QuantumInstance, then let's leave things how you have it for now since VQE is main area where performance improvements are wanted presently. We can figure where we want to go with this more generally later perhaps, if this seems difficult to do for now.

But I do think we should have a couple of unit tests with Aer and VQE whose main goal is to ensure this function continues to work as expected. My guess is that the coverage we happen to have is by virtue of the test_readme_sample in chemistry that happens to use Aer. Normally the unit tests here used BasicAer hence I needed to look to figure where the unit test coverage was coming from.
But if we have a VQE+Aer test say with a TwoLocal circuit and also a test with UCCSD, since its parameterization is around evolution, then these will give explicit coverage and not rely on our readme sample, which may change, but today just happens to use VQE with Aer that it provided coverage. I can potentially help with the unit tests here if needed.

@hhorii hhorii requested a review from pbark as a code owner August 24, 2020 21:32
@woodsp-ibm
Copy link
Member

I added some extra unit testing with Aer so as to ensure we have adequate coverage since now the parameterization behavior with VQE circuits depends on whether Aer is being used or not.

@woodsp-ibm woodsp-ibm merged commit 474ad90 into qiskit-community:master Aug 24, 2020
Cryoris pushed a commit to Cryoris/qiskit-aqua that referenced this pull request Sep 3, 2020
* enable aer's parameter binding

* Additional unit testing with Aer

Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: woodsp <woodsp@us.ibm.com>
pbark pushed a commit to pbark/qiskit-aqua that referenced this pull request Sep 16, 2020
* enable aer's parameter binding

* Additional unit testing with Aer

Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: woodsp <woodsp@us.ibm.com>
mtreinish pushed a commit to mtreinish/qiskit-core that referenced this pull request Nov 20, 2020
* enable aer's parameter binding

* Additional unit testing with Aer

Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: woodsp <woodsp@us.ibm.com>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 2, 2020
* enable aer's parameter binding

* Additional unit testing with Aer

Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: woodsp <woodsp@us.ibm.com>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 7, 2020
* enable aer's parameter binding

* Additional unit testing with Aer

Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: woodsp <woodsp@us.ibm.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants