search

qiskit-community / qiskit-aqua

Quantum Algorithms & Applications (**DEPRECATED** since April 2021 - see readme for more info)
https://qiskit.org/aqua
Apache License 2.0
574 stars 376 forks source link

Handle symengine based parameter expressions #1586

Closed mtreinish closed 3 years ago

mtreinish commented 3 years ago

Summary

In Qiskit/qiskit-terra#6270 symengine is being added as an optional (but default on common platforms) backend for parameters and parameter expressions. However, the aqua version of the gradient code there is an implicit assumption that parameterexpressions are internally just wrappers of sympy expressions. This assumption about the internals of terra breaks with Qiskit/qiskit-terra#6270 where they might be symengine or sympy expressions. While symengine and sympy expressions are interchangeable this can only be done with an explicit conversion step (for example, sympy.sympify(symengine.Symbol('x'))). This commit fixes this by updating the derivative base class as was done for terra's version in Qiskit/qiskit-terra#6270 so that the deprecated aqua copy continues to work wither versions of terra after Qiskit/qiskit-terra#6270 merges.

Details and comments

mtreinish commented 3 years ago

I'm labeling this as on hold until we have agreement that Qiskit/qiskit-terra#6270 is going to happen (I assume it will because the performance improvement is significant, but we haven't had the discussion yet). Once there is agreement this will need to merge to unblock CI on Qiskit/qiskit-terra#6270.

mtreinish commented 3 years ago

This is unblocked now, Qiskit/qiskit-terra#6270 has been approved. We need this fix to unblock the terra PR now.

The one open question in my head is do we want to somehow guard against a potential situation where someone has aqua >=0.1.0 installed and symengine installed by terra <0.18.0 ? Because right now if someone installs aqua master with symengine but is using terra 0.17.x I think this will fail (because it's building a symengine expression instead of a sympy expression like what is unconditionally used in terra 0.17.x)