[WIP] Add ParameterExpression in Rust

[doichanj]

Summary

This is draft of ParameterExpression in Rust by using symengine's C wrapper interface for #13267

Details and comments

Currently, conan install fails from pip install . It passes when running cargo build in crates/circuit

interfaces to python and C will be added...

[qiskit-bot]

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

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 15065615407

Details

• 2151 of 2811 (76.52%) changed or added relevant lines in 9 files are covered.
• 16 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-0.4%) to 88.302%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/qpy/binary_io/value.py	0	4	0.0%
crates/circuit/src/symbol_parser.rs	165	193	85.49%
crates/circuit/src/parameter_expression.rs	278	368	75.54%
crates/circuit/src/symbol_expr.rs	1655	2193	75.47%
<!--	Total:	2151	2811

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	1	91.98%
crates/transpiler/src/passes/unitary_synthesis.rs	1	94.85%
qiskit/circuit/parameterexpression.py	2	96.97%
crates/qasm2/src/parse.rs	12	96.68%
<!--	Total:	16

Totals
Change from base Build 15055964365:	-0.4%
Covered Lines:	78448
Relevant Lines:	88841

---

💛 - Coveralls

[mtreinish]

Add complex number support (is it necessary ?)

@Cryoris can confirm, but I think on main we don't have to worry about this for 2.0 since we're dropping pulse support. AFAIK the complex parameter binding and handling was only necessary for pulse expressions. But maybe there is a use case I'm overlooking.

[Cryoris]

For circuits, real is enough. But we currently also allow parameter expressions in the SparsePauliOp, where there's no restriction to real values. Beyond being more difficult to implement, complex number support doesn't come with a performance hit, does it?

[ShellyGarion]

I just wanted to point out that not all of the ParameterExpression methods are documented in the API docs. One example is pow another one in mul (see #13510). As part of this PR some other methods (like complex) are being added, so we need to make sure that everything is well documented and tested.

[doichanj]

I'm trying to fix errors in test cases but I'm feeling difficulty to solve complicated test cases (e.g. test.python.quantum_info.operators.symplectic.test_sparse_pauli_op.TestSparsePauliOpMethods.test_compose_7) Is there good way to debug these test cases?

[mtreinish]

I squashed the history here. The last merge commit was a bit jumbled up because the history of the working branches between this PR's branch on github @doichanj's local branch had diverged. When it got merged back in this confused reno and it couldn't resolve where the release note had come from. There was no way to resolve this short of reverting the merge and rebasing the commits to have a linear history. But when I tried to do this the line feed changes in the merge commit made it next to impossible to resolve the diffs. Since the windows newlines caused git to identify every newline as different. I couldn't come up with an elegant solution to rework it, there might have been a way if I did an interactive rebase and manually applied dos2unix on the commits in the right order to get git to resolve the diffs correctly. But I spent too much time futzing with it and decided I was just squash the history at the HEAD of this branch and rebase that commit on top of main. I just forced pushed that up which should resolve the reno issue (and then pushed a couple of fixes on top).

For posterity I copied the branch from before the squash and force push and pushed it on my fork https://github.com/mtreinish/qiskit-core/tree/ParameterExpression_rust_backup_2_26 so if we need to pull any commits out in isolation they exist over there.

[mtreinish]

I expanded the test coverage a bit in: https://github.com/Qiskit/qiskit/pull/13278/commits/29a77a1be78b7f9c32934d6f6b65ba79f90a0bfa to ensure we had more complete coverage of the Python ParameterExpression's API surface. I found that the conjugate implementation was incomplete, which I fixed here: https://github.com/Qiskit/qiskit/pull/13278/commits/8257d89f4b1886356bdbfa5170661b622ffd2d3d .

Locally I'm hitting 3 failures around the pow implementation that I'm still debugging. When doing sign(abc ** 1), tan(abc ** 1), and sin(abc ** 1) seem to be returning incorrect results. I figured I should push up the tests though in case I don't have time to keep working on it before someone else gets an opportunity to look at it.

[mtreinish]

I expanded the test coverage a bit in: 29a77a1 to ensure we had more complete coverage of the Python ParameterExpression's API surface. I found that the conjugate implementation was incomplete, which I fixed here: 8257d89 .

Locally I'm hitting 3 failures around the pow implementation that I'm still debugging. When doing sign(abc ** 1), tan(abc ** 1), and sin(abc ** 1) seem to be returning incorrect results. I figured I should push up the tests though in case I don't have time to keep working on it before someone else gets an opportunity to look at it.

This should be fixed now by: https://github.com/Qiskit/qiskit/pull/13278/commits/af6029de7eebfe94fc64671b4b0644874bf96754

[doichanj]

I noticed my implementation is very slow compared to main branch. I think I have to stop reordering and optimizing equation every time appending a new node to tree. I'm very sorry but I need some time to solve this issue

I compared using following benchmark.

from qiskit.circuit import ParameterVector
import time

for nparams in range(10, 110, 10):
    xparams = ParameterVector("x", nparams)
    yparams = ParameterVector("y", nparams)

    test = 0

    start = time.perf_counter()
    for i in range(nparams):
        test = test + xparams[i] * yparams[i]
    end = time.perf_counter()
    tparam = end-start

    values = [1.0]*nparams
    start = time.perf_counter()
    test.bind(dict(zip(xparams,values)))
    test.bind(dict(zip(yparams,values)))
    end = time.perf_counter()
    tbind = end-start

    print('{}, {:.5f}, {:.5f}'.format(nparams, tparam, tbind))

This graph shows time to build parameterized equation (tparam in the code)

This graph shows time to bind values to the parameterized equation (tbind in the code)

[mtreinish]

Given @jakelishman's review and the performance issues @doichanj's identified I've deferred this to 2.1 since it's not going to be ready in time for the 2.0 release. We can circle back and discuss the direction here once 2.0rc1 is out the door and the dust has settled a bit on that release.

[doichanj]

Now the performance issue is almost solved.

[doichanj]

I think I finished reflecting review comments and I solved performance issue