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

Add QPY serialization for PauliEvolutionGate #7374

Merged
merged 21 commits into from Dec 8, 2021

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Dec 8, 2021

Summary

This commit adds serialization for the PauliEvolutionGate class so that
we can exactly reproduce a PauliEvolutionGate over QPY. This works by
bumping the qpy format version and adding new structs to represent the
PauliEvolutionGate and all it's child attribute types.

Details and comments

If this can be finished and thoroughly tested in time for 0.19.1 we can include this. Otherwise we can come up with a faster workaround and then just save this for 0.20.0 (and probably QPY v4 assuming #7372 is included in 0.19.1 with a format bump to represent ParameterVectorElements).

TODO:

  • release notes
  • add support for synthesis class
  • fix PauliEvolutionGate.operator to not use PauliSumOp (which is incorrect) and use SparsePauliOp directly
  • Add more test coverage
  • Add qpy compat tests

This commit adds serialization for the PauliEvolutionGate class so that
we can exactly reproduce a PauliEvolutionGate over QPY. This works by
bumping the qpy format version and adding new structs to represent the
PauliEvolutionGate and all it's child attribute types.
@mtreinish mtreinish added on hold Can not fix yet Changelog: Bugfix Include in the "Fixed" section of the changelog labels Dec 8, 2021
@mtreinish mtreinish added this to the 0.19.1 milestone Dec 8, 2021
@mtreinish mtreinish requested a review from a team as a code owner December 8, 2021 00:28
@mtreinish mtreinish added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Dec 8, 2021
@mtreinish
Copy link
Member Author

I've tagged this for 0.19.1 but I'm not optimistic it'll be ready in time so we should not block on this.

This commit fixes the handling of the operator attribute in the
PauliEvolutionGate. With this commit we can serialize a
PauliEvolutionGate correctly with the exception of it's synthesis class
which still needs to be supported.
@coveralls
Copy link

coveralls commented Dec 8, 2021

Pull Request Test Coverage Report for Build 1555898624

  • 118 of 124 (95.16%) changed or added relevant lines in 6 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 83.12%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/circuit/library/pauli_evolution.py 9 10 90.0%
qiskit/synthesis/evolution/evolution_synthesis.py 3 4 75.0%
qiskit/synthesis/evolution/lie_trotter.py 5 6 83.33%
qiskit/synthesis/evolution/product_formula.py 7 8 87.5%
qiskit/circuit/qpy_serialization.py 93 95 97.89%
Files with Coverage Reduction New Missed Lines %
qiskit/extensions/quantum_initializer/uc.py 2 88.65%
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 1555133627: 0.02%
Covered Lines: 51786
Relevant Lines: 62303

💛 - Coveralls

@mtreinish mtreinish changed the title [WIP] Add qpy serialization for PauliEvolutionGate Add QPY serialization for PauliEvolutionGate Dec 8, 2021
@mtreinish mtreinish removed the on hold Can not fix yet label Dec 8, 2021
@mtreinish
Copy link
Member Author

Ok, this should be ready for review now. I think all the use cases are working.

qiskit/circuit/qpy_serialization.py Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Outdated Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Outdated Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Outdated Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Outdated Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Outdated Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Outdated Show resolved Hide resolved
qiskit/circuit/qpy_serialization.py Outdated Show resolved Hide resolved
mtreinish and others added 3 commits December 8, 2021 13:00
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
jakelishman
jakelishman previously approved these changes Dec 8, 2021
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Looks good to me now. Julien (@Cryoris), have you tested it to make sure it handles your use-case?

Comment on lines +1036 to +1037
with io.BytesIO() as element_buf:
with io.BytesIO() as buf:
Copy link
Member

Choose a reason for hiding this comment

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

No need to change this, but for the future you can avoid an extra scope by doing

with io.BytesIO() as element_buf, io.BytesIO() as buf:

the scope order is guaranteed to be left-to-right. (Assuming it's not a line-length constraint!)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah cool, I didn't realize that

@Cryoris
Copy link
Contributor

Cryoris commented Dec 8, 2021

Yes, it fixes the problem locally 👍🏻 as soon as we merged it we can properly test it on the runtime image using Terra main.

jakelishman
jakelishman previously approved these changes Dec 8, 2021
@mergify mergify bot merged commit 9af5ca1 into Qiskit:main Dec 8, 2021
mergify bot pushed a commit that referenced this pull request Dec 8, 2021
* WIP: Add qpy serialization for PauliEvolutionGate

This commit adds serialization for the PauliEvolutionGate class so that
we can exactly reproduce a PauliEvolutionGate over QPY. This works by
bumping the qpy format version and adding new structs to represent the
PauliEvolutionGate and all it's child attribute types.

* Fix handling of operator in PauliEvolutionGate

This commit fixes the handling of the operator attribute in the
PauliEvolutionGate. With this commit we can serialize a
PauliEvolutionGate correctly with the exception of it's synthesis class
which still needs to be supported.

* settings for synth

* Add support for custom synthesis classes

* Expand test coverage

* Add release notes

* Update release note

* fix param binding in PauliEvo

* allow time as an int

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Update qiskit/circuit/qpy_serialization.py

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Update qiskit/circuit/qpy_serialization.py

* Rerun with latest black

* Close buffers when finished

* Fix release note wording

* Fix lint

* Adjust tests around extra layer of gates

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
(cherry picked from commit 9af5ca1)
@mtreinish mtreinish deleted the qpy-evo-gate branch December 8, 2021 21:48
mergify bot added a commit that referenced this pull request Dec 8, 2021
* WIP: Add qpy serialization for PauliEvolutionGate

This commit adds serialization for the PauliEvolutionGate class so that
we can exactly reproduce a PauliEvolutionGate over QPY. This works by
bumping the qpy format version and adding new structs to represent the
PauliEvolutionGate and all it's child attribute types.

* Fix handling of operator in PauliEvolutionGate

This commit fixes the handling of the operator attribute in the
PauliEvolutionGate. With this commit we can serialize a
PauliEvolutionGate correctly with the exception of it's synthesis class
which still needs to be supported.

* settings for synth

* Add support for custom synthesis classes

* Expand test coverage

* Add release notes

* Update release note

* fix param binding in PauliEvo

* allow time as an int

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Update qiskit/circuit/qpy_serialization.py

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Update qiskit/circuit/qpy_serialization.py

* Rerun with latest black

* Close buffers when finished

* Fix release note wording

* Fix lint

* Adjust tests around extra layer of gates

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
(cherry picked from commit 9af5ca1)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@HuangJunye HuangJunye added the mod: qpy Related to QPY serialization label Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: qpy Related to QPY serialization stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants