[Alternative] Move Parameter to Rust #14757

Cryoris · 2025-07-18T09:59:58Z

Summary

Based on the excellent work in #14207, this is PR implements a slightly alternative approach to porting the ParameterExpression to Rust, based on discussions with @doichanj, @kevinhartman and @mtreinish. The main differences are outlined below. We should compare both approaches and check which one is closer to what we'd like. Of course the efforts could also be merged.

Details and comments

This PR does not supersede #14207, it is merely a slightly different approach and it's majority is based on #14207. The main differences are:

Keep ParameterExpression, Parameter and ParameterVectorElement fully in Rust, without a Python-side class. This is done to reduce the code base and maintainer effort. This could also improve performance as we need less back and forth between the Rust and Python boundaries.
Store the symbol's UUID on the Symbol in the symbolic expression engine itself, rather than keep a separate map of symbols to their UUID. This is done to couple the UUID and symbol closer instead of relying on a third object to make this connection.
Implement pickling via QPY instead of string serialization -- we already have a serialization format which we could use. Maybe this allows to remove string-based operations in the future for safety?
... some other bits I probably forgot

Things that could make this PR better (maybe in this PR or a follow up)

Keep a separate ParameterExpression and PyParameterExpression for clearer Python split. Doing this would allow us to make the expression's name_map use Symbols directly. Let's think about how we want to use this from C!
Make Symbol not a pyclass (probably easy to do, just didn't get to it yet)
Should we remove the index from Symbol and have an IndexedSymbol, like in Move Parameter classes from Python to Rust #14207? Answer: no, it won't reduce memory footprint, so we don't bother now.

~~try to remove string parsing~~ this will be a follow up
... ~~some other bits I surely did forget and am unaware of~~

Co-authored-by: "U-AzureAD\JUNDOI" doichan@jp.ibm.com

-- 606 failing tests

- name conflicts - hash(2.3) == hash(ParamExpr(2.3)) - some error messages

This allows keeping track of which parameters are still in the expression, even though the expression is optimized. E.g. x*0 = 0, but x is still valid to bind.

- don't use UUID in to_string, only in string_id (using strings for comparisons should probably be avoided in future) - clippy - relax Python is-checks to eq-checks

this is rather a temporary solution, maybe we ought to disable string parsing in favor or a proper SymPy->parameter expression conversion

qiskit-bot · 2025-07-18T10:00:05Z

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

@Cryoris
@Qiskit/terra-core
@ajavadia
@mtreinish
@nkanazawa1989

coveralls · 2025-07-18T11:39:14Z

Pull Request Test Coverage Report for Build 16633739615

Details

1204 of 1475 (81.63%) changed or added relevant lines in 12 files are covered.
100 unchanged lines in 5 files lost coverage.
Overall coverage decreased (-0.1%) to 87.636%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/parameterexpression.py	46	47	97.87%
crates/circuit/src/parameter/symbol_parser.rs	1	3	33.33%
crates/circuit/src/parameter_table.rs	3	10	30.0%
qiskit/qpy/binary_io/value.py	18	26	69.23%
crates/circuit/src/parameter/symbol_expr.rs	239	316	75.63%
crates/circuit/src/parameter/parameter_expression.rs	878	1054	83.3%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.63%
crates/qasm2/src/lex.rs	3	91.75%
crates/circuit/src/parameter_table.rs	4	89.67%
qiskit/qpy/common.py	21	55.24%
qiskit/qpy/binary_io/value.py	71	68.25%

Totals
Change from base Build 16630632380:	-0.1%
Covered Lines:	81724
Relevant Lines:	93254

💛 - Coveralls

mainly removing some unnecessary clones, and fixing Hash for ParameterExpression

--- and just reconstruct the Py symbols from ParameterExpression.name_map

Cryoris · 2025-07-28T11:26:45Z

I ran some preliminary benchmarks comparing Qiskit v2.1.1 vs this and it seems this PR speeds things up a bit:

-- Construct + bind efficient_su(num_qubits=n, reps=n)
(our PR)
--- n=100 took 1.002s
--- n=200 took 4.394s
--- n=500 took 30.157s
(2.1.1)
--- 100 took 1.180s
--- 200 took 5.945s
--- 500 took 41.248s

-- Compute sum_{i=1}^n (x_i - i/100) ** 2
(our PR)
--- 100 took 0.008s
--- 200 took 0.026s
--- 500 took 0.104s
(2.1.1)
--- 100 took 0.133s
--- 200 took 0.070s
--- 500 took 0.397s

Note that the first benchmarks measures time to construct the circuit too, so the binding speedup is hidden a bit.

Co-authored-by: "U-AzureAD\JUNDOI" doichan@jp.ibm.com

kevinhartman

This completes my first pass of review. Thanks to both you and @doichanj for all of this work.

Overall, this looks good to me. The one high-level question I've got concerns the split between PyParameterExpression, ParameterExpression, and SymbolExpr. Am I correct in my understanding that the only reason to have a ParameterExpression is so we have some place to maintain a name_map? If not for this, I would think we could get away with just having PyParameterExpression for Python and SymbolExpr for Rust.

crates/circuit/src/parameter/parameter_expression.rs

qiskit/qpy/binary_io/value.py

test/python/circuit/test_circuit_load_from_qpy.py

test/python/circuit/test_parameter_expression.py

test/python/circuit/test_parameters.py

Cryoris · 2025-07-29T11:32:31Z

The one high-level question I've got concerns the split between PyParameterExpression, ParameterExpression, and SymbolExpr. Am I correct in my understanding that the only reason to have a ParameterExpression is so we have some place to maintain a name_map? If not for this, I would think we could get away with just having PyParameterExpression for Python and SymbolExpr for Rust.

Yeah I also thought about this (in fact it's what I had initially). There's 2 reasons I added the name_map:

If we only keep SymbolExpr, which is an enum, we can't store any cached states though, right? Meaning that if we repeatedly query the parameters we would have to traverse the tree each time. Since we do this quite a lot, e.g. for assigning parameters or checking for parameter conflicts, I thought it would be better to have the cache.
The current symbolic expressions optimize things like 0*x to 0 and lose the Symbol. However, if we do qc.rx(0 * x, 0) then Qiskit expects that I can still bind x. We need to keep track of this symbol somewhere, which is the name_map.

That being said -- the ParameterTable already does some caching for parameter assignment, and I didn't benchmark what happens when we remove that cache. I can try and see what happens, if we want 🙂 We might be able to use the same cache to check for the existence of x in 0*x-type expressions.

An advantage of not having ParameterExpression would of course be one layer less, which is nicer to maintain.

mainly rm unused code and dangling comments

- dangling commented code - error for derivative

Co-authored-by: Jun Doi <doichan@jp.ibm.com>

kevinhartman

This looks good to me now. I have one minor comment, but I don't think we need to hold this up.

For posterity, we talked offline about the idea of eliminating ParameterExpression (and just keeping SymbolExpr) and our conclusion was that SymbolExpr is more of a Qiskit-independent symbol layer, and ParameterExpression is the piece that wraps it with our extra domain-specific logic. I suggested that we might want to move SymbolExpr to its own crate to avoid future coupling, but we've decided to defer that for a potential follow-up.

kevinhartman · 2025-07-31T16:36:38Z

crates/circuit/src/parameter/symbol_expr.rs

@@ -905,7 +900,7 @@ impl SymbolExpr {
    }

    pub fn string_id(&self) -> String {


I'm still not a fan of calling this an "id" since it sounds like what gets returned is a simple ID of a constant length.

In the recently merged Qiskit#14757 a few docs lines were overindented. Newer versions of clippy detect this and raise a warning but the version we run in CI doesn't have this rule so this went uncaught. This commit fixes this small oversight so clippy is happy and the docs are properly formatted.

In the recently merged #14757 a few docs lines were overindented. Newer versions of clippy detect this and raise a warning but the version we run in CI doesn't have this rule so this went uncaught. This commit fixes this small oversight so clippy is happy and the docs are properly formatted.

* Split py and py-free methods * playing with Symbol as Parameter -- 606 failing tests * fix errors * about to start the mess * suite passing * PVE in Rust * start assign * add assign * dealing with unknown params and conflicts * an attempt at vector fixing * fix more tests - name conflicts - hash(2.3) == hash(ParamExpr(2.3)) - some error messages * implement name_map as track record This allows keeping track of which parameters are still in the expression, even though the expression is optimized. E.g. x*0 = 0, but x is still valid to bind. * The string debacle - don't use UUID in to_string, only in string_id (using strings for comparisons should probably be avoided in future) - clippy - relax Python is-checks to eq-checks * some housekeeping * implement _qpy_replay * implement sympify * fix copy/deepcopy * Implement pickle via QPY and Parameter.assign to return Parameter * fix template matching this is rather a temporary solution, maybe we ought to disable string parsing in favor or a proper SymPy->parameter expression conversion * fix rhs - lhs mixup * fix more tests on binding {x: 0*y} * fix last failing tests (see numeric(strict=False)) * clippy * a lot of docs * rename PyParameterExpression to ParameterExpression * Thou shan't fake hyperrefs * proper split: (Py)ParameterExpression * rm pyclass from Symbol * TODO's and no more RwLock - rm Arc<RwLock<..>> for inner, since expressions are immutable and not shared, this is not needed - address a bunch of todos, that include - rm rsub/rdiv/rpow - introduce try_to_value and _to_symbol - make internal data private * First round of review comments mainly removing some unnecessary clones, and fixing Hash for ParameterExpression * rm `name_map` from PyParameterExpression --- and just reconstruct the Py symbols from ParameterExpression.name_map * Fix vector element parsing Co-authored-by: "U-AzureAD\JUNDOI" doichan@jp.ibm.com * finish 1st review round mainly rm unused code and dangling comments * left over review comments - dangling commented code - error for derivative * rename SymbolExpr::display --> repr Co-authored-by: Jun Doi <doichan@jp.ibm.com> --------- Co-authored-by: Jun Doi <doichan@jp.ibm.com>

In the recently merged Qiskit#14757 a few docs lines were overindented. Newer versions of clippy detect this and raise a warning but the version we run in CI doesn't have this rule so this went uncaught. This commit fixes this small oversight so clippy is happy and the docs are properly formatted.

Cryoris added 25 commits June 5, 2025 13:54

Split py and py-free methods

06f1f5d

playing with Symbol as Parameter

fb35a74

-- 606 failing tests

fix errors

f52ca71

about to start the mess

b4b2635

suite passing

a3fa027

PVE in Rust

6ac10c9

start assign

8f495fd

add assign

2ab1a97

dealing with unknown params and conflicts

c79f36e

an attempt at vector fixing

803a777

fix more tests

103dbd9

- name conflicts - hash(2.3) == hash(ParamExpr(2.3)) - some error messages

implement name_map as track record

3df71fd

This allows keeping track of which parameters are still in the expression, even though the expression is optimized. E.g. x*0 = 0, but x is still valid to bind.

The string debacle

2b561dc

- don't use UUID in to_string, only in string_id (using strings for comparisons should probably be avoided in future) - clippy - relax Python is-checks to eq-checks

some housekeeping

b6b21af

implement _qpy_replay

04c1030

implement sympify

2aec3de

fix copy/deepcopy

775a379

Implement pickle via QPY and Parameter.assign to return Parameter

19f23d2

fix template matching

72e8eeb

this is rather a temporary solution, maybe we ought to disable string parsing in favor or a proper SymPy->parameter expression conversion

fix rhs - lhs mixup

e53442f

fix more tests on binding {x: 0*y}

ce6e863

fix last failing tests (see numeric(strict=False))

5ae9d5a

clippy

58eb92b

a lot of docs

dfbdbe9

rename PyParameterExpression to ParameterExpression

3197c5b

Cryoris requested a review from a team as a code owner July 18, 2025 09:59

Cryoris added the Rust This PR or issue is related to Rust code in the repository label Jul 18, 2025

Merge branch 'main' into rust-param-pyparam

9a07ded

Cryoris added 2 commits July 28, 2025 11:59

First round of review comments

fad2c31

mainly removing some unnecessary clones, and fixing Hash for ParameterExpression

rm name_map from PyParameterExpression

5c73172

--- and just reconstruct the Py symbols from ParameterExpression.name_map

Fix vector element parsing

190f608

Co-authored-by: "U-AzureAD\JUNDOI" doichan@jp.ibm.com

kevinhartman reviewed Jul 28, 2025

View reviewed changes

finish 1st review round

c14727c

mainly rm unused code and dangling comments

Cryoris mentioned this pull request Jul 30, 2025

Python-free path for circuit parameters #14799

Merged

3 tasks

left over review comments

2a892a0

- dangling commented code - error for derivative

Cryoris force-pushed the rust-param-pyparam branch from d2a233d to 2a23689 Compare July 30, 2025 21:02

rename SymbolExpr::display --> repr

5d336aa

Co-authored-by: Jun Doi <doichan@jp.ibm.com>

Cryoris force-pushed the rust-param-pyparam branch from 2a23689 to 5d336aa Compare July 30, 2025 21:04

raynelfss added this to the 2.2.0 milestone Jul 31, 2025

kevinhartman self-assigned this Jul 31, 2025

kevinhartman approved these changes Jul 31, 2025

View reviewed changes

kevinhartman added this pull request to the merge queue Jul 31, 2025

Merged via the queue into Qiskit:main with commit 3c164c9 Jul 31, 2025
26 checks passed

mtreinish mentioned this pull request Jul 31, 2025

Move Parameter classes from Python to Rust #14207

Closed

mtreinish mentioned this pull request Jul 31, 2025

Fix doc indents in parameter expression docs #14808

Merged

This was referenced Aug 4, 2025

Investigate using Arc for ParameterExpression::expr #14818

Open

String-based comparisons in symbolic expression operations #14819

Open

Aqil-Ahmad mentioned this pull request Aug 9, 2025

Fix ParameterExpression.gradient() to return numeric values for constant derivatives #14848

Open

raynelfss added the Changelog: None Do not include in changelog label Aug 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Alternative] Move Parameter to Rust #14757

[Alternative] Move Parameter to Rust #14757

Uh oh!

Cryoris commented Jul 18, 2025 •

edited

Loading

Uh oh!

qiskit-bot commented Jul 18, 2025

Uh oh!

coveralls commented Jul 18, 2025 •

edited

Loading

Uh oh!

Cryoris commented Jul 28, 2025

Uh oh!

kevinhartman left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Cryoris commented Jul 29, 2025 •

edited

Loading

Uh oh!

kevinhartman left a comment

Uh oh!

kevinhartman Jul 31, 2025

Uh oh!

Uh oh!

Uh oh!

		@@ -905,7 +900,7 @@ impl SymbolExpr {
		}

		pub fn string_id(&self) -> String {

[Alternative] Move Parameter to Rust #14757

[Alternative] Move Parameter to Rust #14757

Uh oh!

Conversation

Cryoris commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

Uh oh!

qiskit-bot commented Jul 18, 2025

Uh oh!

coveralls commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 16633739615

Details

💛 - Coveralls

Uh oh!

Cryoris commented Jul 28, 2025

Uh oh!

kevinhartman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Cryoris commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinhartman left a comment

Choose a reason for hiding this comment

Uh oh!

kevinhartman Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Cryoris commented Jul 18, 2025 •

edited

Loading

coveralls commented Jul 18, 2025 •

edited

Loading

Cryoris commented Jul 29, 2025 •

edited

Loading