Set sympy.evaluate(False) on frequent code paths for phase tracking.

[kdk]

Summary

Resolves #5405 by disabling sympy evaluation on code paths that are hit frequently for parameterized global phase tracking.

Details and comments

I attempted to apply this more broadly (in ParameterExpression._apply_operation and ParameterExpression._call) which resulted in several failures for the gradient tests. Maybe this can be looked into more after the release.

N.B. This PR also introduces the potential for bugs/complications as the ParameterExpression._symbol_expr for .global_phase will become an unevaluated sympy expression, so some sympy behaviors which we may have come to depend on (e.g. commutativity of addition/multiplication) will no longer behave as they did previously. (The most likely consequence that comes to mind is false negatives for global phase equality.) (@levbishop confirmed that the changes to ParameterExpression.__eq_ in #5760 should make this a non-issue.)

Evaluating the global phase expression after the fact (via .doit()) resolves these issues but nullifies the performance fix. Likewise for collecting global phase contributions in the BasisTranslator and summing them in a single expression.

This PR is a short term fix at best, and will need some follow up. Would welcome any suggestions or alternatives.

[Cryoris]

I think this makes sense, since I think we should prioritize the performance of binding parameters over showing a nice expression for the global phase (of course if we can have both in the future that would be even better). Could you add a release note pointing out that the global phase is not simplified?

[kdk]

I think this makes sense, since I think we should prioritize the performance of binding parameters over showing a nice expression for the global phase (of course if we can have both in the future that would be even better). Could you add a release note pointing out that the global phase is not simplified?

Modulo performance differences(at what point the sympy simplification happens), this shouldn't have a user facing impact (unless someone is directly accessing ParameterExpression._symbol_expr). e.g.

>>> th = qk.circuit.Parameter('theta')
>>> qc = qk.QuantumCircuit(1)
>>> qc.rz(th, 0)
>>> out = BasisTranslator(sel, ['p'])(qc)
>>> out.global_phase
-0.5*theta
>>> out.global_phase == -0.5 * th
True
>>> out.draw()
global phase: -0.5*theta
     ┌──────────┐
q_0: ┤ P(theta) ├
     └──────────┘

[levbishop]

I think this makes sense, since I think we should prioritize the performance of binding parameters over showing a nice expression for the global phase (of course if we can have both in the future that would be even better). Could you add a release note pointing out that the global phase is not simplified?

Modulo performance differences(at what point the sympy simplification happens), this shouldn't have a user facing impact (unless someone is directly accessing ParameterExpression._symbol_expr). e.g.

>>> th = qk.circuit.Parameter('theta')
>>> qc = qk.QuantumCircuit(1)
>>> qc.rz(th, 0)
>>> out = BasisTranslator(sel, ['p'])(qc)
>>> out.global_phase
-0.5*theta
>>> out.global_phase == -0.5 * th
True
>>> out.draw()
global phase: -0.5*theta
     ┌──────────┐
q_0: ┤ P(theta) ├
     └──────────┘

If you add another qc.rz(-th, 0) before BasisTranslator, won't the global_phase in the drawn circuit change from 0 without this PR to -0.5*theta + 0.5*theta with this PR? I guess can put a doit in the visualizations?

[kdk]

If you add another qc.rz(-th, 0) before BasisTranslator, won't the global_phase in the drawn circuit change from 0 without this PR to -0.5*theta + 0.5*theta with this PR? I guess can put a doit in the visualizations?

That's a good point (this came up in discussions with @Cryoris as well). Given that the simplification is not behavior we're entirely settled on, I can add a note pointing out that a parameterized global phase might no longer be simplified after transpilation.

[kdk]

On this branch, numbers from example code in #5405 :

ExcitationPreserving(num_qubits=6, reps=3)
Construction time: 0.0013430118560791016 seconds
Number of instructions: 114
Transpilation time: 0.19821596145629883 seconds

ExcitationPreserving(num_qubits=12, reps=6)
Construction time: 0.001009225845336914 seconds
Number of instructions: 876
Transpilation time: 1.4568750858306885 seconds

ExcitationPreserving(num_qubits=16, reps=8)
Construction time: 0.0018911361694335938 seconds
Number of instructions: 2064
Transpilation time: 4.395498991012573 seconds

ExcitationPreserving(num_qubits=20, reps=10)
Construction time: 0.0016469955444335938 seconds
Number of instructions: 4020
Transpilation time: 7.775006055831909 seconds

ExcitationPreserving(num_qubits=24, reps=12)
Construction time: 0.0024042129516601562 seconds
Number of instructions: 6936
Transpilation time: 18.742086172103882 seconds

[delapuente]

Regarding the introduction of potential bugs, if that happens, it would be frustrating not being able to recover the original behavior. Why not adding a parameter (i.e. evaluate_symbolic_global_phase) to force the previous behavior?