Add Qiskit native QPY ParameterExpression serialization

[mtreinish]

Summary

With the release of symengine 0.13.0 we discovered a version dependence on the payload format used for serializing symengine expressions. This was worked around in #13251 but this is not a sustainable solution and only works for symengine 0.11.0 and 0.13.0 (there was no 0.12.0). While there was always the option to use sympy to serialize the underlying symbolic expression (there is a use_symengine flag on qpy.dumps you can set to False to do this) the sympy serialzation has several tradeoffs most importantly is much higher runtime overhead. To solve the issue moving forward a qiskit native representation of the parameter expression object is necessary for serialization.

This commit bumps the QPY format version to 13 and adds a new serialization format for ParameterExpression objects. This new format is a serialization of the API calls made to ParameterExpression that resulted in the creation of the underlying object. To facilitate this the ParameterExpression class is expanded to store an internal "replay" record of the API calls used to construct the ParameterExpression object. This internal list is what gets serialized by QPY and then on deserialization the "replay" is replayed to reconstruct the expression object. This is a different approach to the previous QPY representations of the ParameterExpression objects which instead represented the internal state stored in the ParameterExpression object with the symbolic expression from symengine (or a sympy copy of the expression). Doing this directly in Qiskit isn't viable though because symengine's internal expression tree is not exposed to Python directly. There isn't any method (private or public) to walk the expression tree to construct a serialization format based off of it. Converting symengine to a sympy expression and then using sympy's API to walk the expression tree is a possibility but that would tie us to sympy which would be problematic for #13267 and #13131, have significant runtime overhead, and it would be just easier to rely on sympy's native serialization tools.

The tradeoff with this approach is that it does increase the memory overhead of the ParameterExpression class because for each element in the expression we have to store a record of it. Depending on the depth of the expression tree this also could be a lot larger than symengine's internal representation as we store the raw api calls made to create the ParameterExpression but symengine is likely simplifying it's internal representation as it builds it out. But I personally think this tradeoff is worthwhile as it ties the serialization format to the Qiskit objects and APIs instead of relying on a 3rd party library. This also gives us the flexibility of changing the internal symbolic expression library internally in the future if we decide to stop using symengine at any point.

Details and comments

Fixes #13252

TODO:

• Add QPY v13 format documentation
• Add release notes
• Expand test coverage. We need to ensure we have thorough coverage of all supported expression operations which we weren't previously exercising through QPY.

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core
• @mtreinish
• @nkanazawa1989

[coveralls]

Pull Request Test Coverage Report for Build 11708157805

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 272 of 293 (92.83%) changed or added relevant lines in 5 files are covered.
• 15 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-0.003%) to 88.786%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/parameterexpression.py	102	103	99.03%
qiskit/qpy/binary_io/value.py	162	182	89.01%

Files with Coverage Reduction	New Missed Lines	%
crates/accelerate/src/two_qubit_decompose.rs	1	92.09%
qiskit/qpy/binary_io/value.py	2	87.27%
crates/qasm2/src/lex.rs	6	91.98%
crates/qasm2/src/parse.rs	6	97.62%

Totals
Change from base Build 11706516151:	-0.003%
Covered Lines:	77347
Relevant Lines:	87116

---

💛 - Coveralls

[mtreinish]

The state of this is it's stuck on the overly complex expression example test I added in: 2b352d7 I'm thinking it might be worth approaching the deserialization in a slightly different approach instead of doing nested expressions recursively like it currently is. If anyone has bandwidth and can come up with a fix before me for that test, feel free to just push to the branch. I think when that test is passing we probably have enough coverage between that new test and the expression tests that already existed for circuits and pulse schedules (we might want to convert those expression tests to circuit equivalents as part of the pulse removal in 2.0).

[mtreinish]

Ok, I fixed the issue from last week with: a903387 which switches to a stack based approach instead of a recursive one. I think this should be ready now.

[ElePT]

Thanks @mtreinish. After taking a look I think the PR looks good. I think we agree in the need for a Qiskit-native alternative for the symengine serialization even if there are some tradeoffs in terms of memory footprint. I added mostly documentation-related suggestions, some typos and some rewordings, and a few minor comments. The question I left in read_parameter_expression_v4 might be important, but I might have missed something there. Other than that, I was wondering if we should also communicate somehow that use_symengine isn't useful anymore for QPY versions over 13?

[mtreinish]

Other than that, I was wondering if we should also communicate somehow that use_symengine isn't useful anymore for QPY versions over 13?

I added a sentence to the qpy.dump function's docstring for the use_symengine argument to say it doesn't do anything in qpy 13. Were you thinking we needed an upgrade release note or something to documente it further?

We can drop the argument in 2.0 potentially since 2.0 will have a minimum compatibility version of qpy format version 13 so nothing >=2.0 emits can support the use_symengine flag. But my worry with that is it is a potentially breaking api change for users that might want to support dumping qpy across code run with multiple qiskit versions. So maybe it's better to just document it as not doing anything in 2.0 and remove it in 3.0. Either way that's something we can debate post 1.3, since we probably should not deprecate it here, and only save that for a future release.

[ElePT]

I added a sentence to the qpy.dump function's docstring for the use_symengine argument to say it doesn't do anything in qpy 13. Were you thinking we needed an upgrade release note or something to documente it further?

I didn't have a fixed idea, but I think the docstring is a good place for the moment. I also wasn't pushing for a deprecation now, just wanted to raise the point to make sure we didn't forget to deal with it in the future.

Thanks a lot for applying the review comments so fast! I have taken a second look and cannot come up with any other potential issue, so on my side I think the PR is ready to be merged.