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

Two-step transpilation for variational algorithms #7217

Merged
merged 17 commits into from
Nov 30, 2021

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Nov 3, 2021

Summary

Allow two transpiler passes in variational algorithms, one for parameterized circuits (executed once) and one for bound circuits (executed in each iteration). This is a requirement for the QAOA runtime program.

Details and comments

Allow two transpiler stages in the QuantumInstance, one for
parameterized circuits and a second one for bound circuits only. This enables variational
algorithms like the VQE to run a pass manager for parameterized
circuits once and, additionally, another transpiler pass on the bound circuits in each
iteration. This is an important feature since not all passes support parameterized circuits.

For example, this feature allows using the pulse-efficient CX decomposition in the VQE.

@Cryoris Cryoris requested review from manoelmarques, woodsp-ibm and a team as code owners November 3, 2021 09:47
@Cryoris Cryoris requested a review from 1ucian0 November 3, 2021 09:47
@Cryoris Cryoris added this to the 0.19 milestone Nov 3, 2021
@Cryoris Cryoris added the Changelog: New Feature Include in the "Added" section of the changelog label Nov 3, 2021
test/python/algorithms/test_vqe.py Show resolved Hide resolved
qiskit/utils/quantum_instance.py Outdated Show resolved Hide resolved
qiskit/utils/quantum_instance.py Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Nov 3, 2021

Pull Request Test Coverage Report for Build 1522789478

  • 16 of 22 (72.73%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.006%) to 82.856%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/utils/quantum_instance.py 13 19 68.42%
Totals Coverage Status
Change from base Build 1522473848: 0.006%
Covered Lines: 50160
Relevant Lines: 60539

💛 - Coveralls

Comment on lines 202 to 203
bound_pass_manager (Optional['PassManager']): Pass manager to apply on bound circuits
only.
Copy link
Member

Choose a reason for hiding this comment

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

Do people generally know what a 'bound circuit' means? Should we state here that this is a circuit whose parameters have been set via the bind/assign so that no symbolic parameters remain (assuming I have it right!). Did it have to be a parameterized circuit in the first place - e.g. is it valid to pass in a circuit with fixed parameters to the transpile the first time around and still use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added more docs in 718bfc3, hopefully it's clearer now 🙂

@ajavadia
Copy link
Member

ajavadia commented Nov 9, 2021

Two-step transpile is the right thing to do for parameterized circuits. 👍

woodsp-ibm
woodsp-ibm previously approved these changes Nov 10, 2021
from qiskit.utils import QuantumInstance, algorithm_globals, has_aer

if has_aer():
from qiskit import Aer


class _Counter(TransformationPass):
Copy link
Member

@1ucian0 1ucian0 Nov 11, 2021

Choose a reason for hiding this comment

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

Should this be an AnalysisPass ? It seems to me that this kind of information should be added to the property pass, since they passes should not hold state among runs.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Overall this LGTM, the code change is straightforward enough. I just have some documentation issues inline. Mainly I think we should have a separate release note for the circuit sampler change and also be a bit more explicit about which transpile() we're talking about in the docstring for the new kwargs.

qiskit/utils/quantum_instance.py Outdated Show resolved Hide resolved
qiskit/utils/quantum_instance.py Outdated Show resolved Hide resolved
@kdk kdk assigned Cryoris and unassigned 1ucian0, jlapeyre and mtreinish Nov 30, 2021
@Cryoris Cryoris requested a review from mtreinish November 30, 2021 16:54
Copy link
Member

@mtreinish mtreinish 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 for making the updates to the docs.

Allow two transpiler stages in the :class:`~qiskit.utils.QuantumInstance`, one for
parameterized circuits and a second one for bound circuits (i.e. no free parameters) only.
If a quantum instance with passes for unbound and bound circuits is passed into a
:class:`~qiskit.opflow.CircuitSampler`, the sampler will attempt to apply the unbound pass
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably have split this into a separate note, but it's fine integrated into this too.

@mergify mergify bot merged commit 0196754 into Qiskit:main Nov 30, 2021
Cryoris added a commit that referenced this pull request Dec 2, 2021
* Fix performance regression of VQE

* Update qiskit/opflow/converters/circuit_sampler.py

Co-authored-by: Julien Gacon <[email protected]>

Co-authored-by: Julien Gacon <[email protected]>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
* first working version

* use additional kwarg, not dict

* add test

* add reno

* black & type hint in tests

* add more docs

* log based testing

* avoid enum and pass pass managers in transpile

* docs

* Apply suggestions from code review

Co-authored-by: Matthew Treinish <[email protected]>

* update docs

Co-authored-by: Luciano Bello <[email protected]>
Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
* first working version

* use additional kwarg, not dict

* add test

* add reno

* black & type hint in tests

* add more docs

* log based testing

* avoid enum and pass pass managers in transpile

* docs

* Apply suggestions from code review

Co-authored-by: Matthew Treinish <[email protected]>

* update docs

Co-authored-by: Luciano Bello <[email protected]>
Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms that referenced this pull request Jul 27, 2023
* first working version

* use additional kwarg, not dict

* add test

* add reno

* black & type hint in tests

* add more docs

* log based testing

* avoid enum and pass pass managers in transpile

* docs

* Apply suggestions from code review

Co-authored-by: Matthew Treinish <[email protected]>

* update docs

Co-authored-by: Luciano Bello <[email protected]>
Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants