Oxidize ParameterExpression
#13267
Labels
Milestone
Comments
mtreinish
added
performance
Rust
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this issue
Oct 22, 2024
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 Qiskit#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 Qiskit#13267 and Qiskit#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 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. Fixes Qiskit#13252
3 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Nov 6, 2024
* Add Qiskit native QPY ParameterExpression serialization 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 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. Fixes #13252 * Remove stray comment * Add format documentation * Add release note * Add test and fix some issues with recursive expressions * Add int type for operands * Add dedicated subs test * Pivot to stack based postfix/rpn deserialization This commit changes how the deserialization works to use a postfix stack based approach. Operands are push on the stack and then popped off based on the operation being run. The result of the operation is then pushed on the stack. This handles nested objects much more cleanly than the recursion based approach because we just keep pushing on the stack instead of recursing, making the accounting much simpler. After the expression payload is finished being processed there will be a single value on the stack and that is returned as the final expression. * Apply suggestions from code review Co-authored-by: Elena Peña Tapia <[email protected]> * Change DERIV to GRAD * Change side kwarg to r_side * Change all the v4s to v13s * Correctly handle non-commutative operations This commit fixes a bug with handling the operand order of subtraction, division, and exponentiation. These operations are not commutative but the qpy deserialization code was treating them as such. So in cases where the argument order was reversed qpy was trying to flip the operands around for code simplicity and this would result in incorrect behavior. This commit fixes this by adding explicit op codes for the reversed sub, div, and pow and preserving the operand order correctly in these cases. * Fix lint --------- Co-authored-by: Elena Peña Tapia <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Right now the
ParameterExpressionclass is defined solely in Python. This has been causing a large bottleneck in the use of Rust to accelerate more of Qiskit as we have to explicitly use python any time we interact with aParameterExpression. So we should move the definition ofParameterExpressionto Rust and expose it to python with a compatible api.At a technical level this is a more involved issue than the other pieces from #13264 because our current interface for
ParameterExpressionis dependent on symengine, a C++ library, to handle the actual expressions. We have the option of interfacing with symengine's C api fairly easily from rust, but that adds complexity to our build system, either requiring a dynamic library is installed to use qiskit (which we can bundle in the released wheels for supported platforms) which adds some runtime complexity, or we can statically link the library into qiskit at build time, but this then adds build time complexity (either building symengine from source or pulling a precompiled library from somewhere).The text was updated successfully, but these errors were encountered: