Port classical expressions (`Expr`) to Rust. #14068

kevinhartman · 2025-03-21T18:45:27Z

Summary

Ports the Expr and Type class hierarchies to Rust.

Details and comments

I've done my best here to create a Rust-first interface that is also fully interoperable with the existing Python API, even though the Python type hierarchies use inheritance and polymorphism for node visitation etc.

The Rust code is organized into a new classical mod with two submodule, expr and type. For both of these, I've defined pure Rust types that are entirely separate from their pyclass counterparts. For example, the Expr enum represents an expression in Rust, while a separate PyExpr struct is defined as a pyclass, which wraps the Rust enum. This is necessary for an ergonomic Rust API, since PyO3 has limitations with using complex enums like this one as a pyclass. I followed this pattern for all ported classes for consistency.

The primary Rust types introduced here are the enums Expr and Type:

`expr::Expr` enum

Representation of an expression in native Rust.

pub enum Expr {
    Unary(Box<Unary>),
    Binary(Box<Binary>),
    Cast(Box<Cast>),
    Value(Value),
    Var(Var),
    Stretch(Stretch),
    Index(Box<Index>),
}

Variants which themselves contain Exprs are wrapped in a Box. For example, the Binary expression contains two expressions, a left and a right:

pub struct Binary {
    pub op: BinaryOp,
    pub left: Expr,
    pub right: Expr,
    pub ty: Type,
    pub constant: bool,
}

Rather than wrapping both left and right in their own Boxs, the Expr::Binary variant wraps Binary. This cuts Box usage in half.

`Binary.Op` and `Unary.Op`

Since these inner class enums are part of the public Python API, I had to do some odd stuff to ensure they still behave exactly like Python enums. This is because PyO3 enums do not support the same interface as Python enums (they don't use the EnumType metaclass like Python enums do), e.g., they are not iterable.

The trick I've done is to define a parallel set of private enums in Python that get returned when accessing the Op class attributes of Binary and Unary. When Op is accessed, it loads the Python-defined enum via imports::{BINARY_OP, UNARY_OP}. Further, these are wrapped in Python descriptors to avoid a cyclic dependency.

`types::Type` enum

pub enum Type {
    Bool,
    Duration,
    Float,
    Uint(u16),
}

Representation of an expression's type in Rust.

To-do

Edit PR description.
Create native Rust representation for Expr.
Create native Rust representation for Type.
Replace Python Expr and Type (i.e. wire things up).
Make sure I didn't accidentally drop docstrings while porting.
~~Use new native Rust types in CircuitData and DAGCircuit.~~ => deferred to separate PR

coveralls · 2025-03-21T22:06:42Z

Pull Request Test Coverage Report for Build 14601151850

Details

678 of 804 (84.33%) changed or added relevant lines in 15 files are covered.
20 unchanged lines in 4 files lost coverage.
Overall coverage decreased (-0.08%) to 88.188%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/circuit/src/classical/expr/stretch.rs	66	70	94.29%
crates/circuit/src/classical/expr/var.rs	93	100	93.0%
crates/circuit/src/classical/types.rs	102	111	91.89%
crates/circuit/src/classical/expr/cast.rs	48	58	82.76%
crates/circuit/src/classical/expr/unary.rs	63	73	86.3%
crates/circuit/src/classical/expr/expr.rs	39	125	31.2%

Files with Coverage Reduction	New Missed Lines	%
qiskit/circuit/classical/expr/visitors.py	1	93.22%
qiskit/qpy/binary_io/value.py	4	82.99%
crates/qasm2/src/parse.rs	6	97.15%
crates/qasm2/src/lex.rs	9	91.73%

Totals
Change from base Build 14600547204:	-0.08%
Covered Lines:	74239
Relevant Lines:	84183

💛 - Coveralls

I'll do these in a separate PR.

qiskit-bot · 2025-03-31T21:48:23Z

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

@Qiskit/terra-core

mtreinish · 2025-03-31T21:54:53Z

Cargo.toml

@@ -30,6 +30,7 @@ approx = "0.5"
 itertools = "0.13.0"
 ahash = "0.8.11"
 rayon = "1.10"
+uuid = { version = "1.16", features = ["v4"] }


Should we add the fast-rng feature too? Iirc it depends on rand which we're already using so there shouldn't be a huge compile time impact

raynelfss

I've done a quick passthrough over the code here, it looks promising. I have a couple of questions regarding some methods and docstrings.

raynelfss · 2025-04-14T20:03:21Z

crates/circuit/src/classical/expr/binary.rs

+pub enum BinaryOp {
+    BitAnd = 1,
+    BitOr = 2,
+    BitXor = 3,
+    LogicAnd = 4,
+    LogicOr = 5,
+    Equal = 6,
+    NotEqual = 7,
+    Less = 8,
+    LessEqual = 9,
+    Greater = 10,
+    GreaterEqual = 11,
+    ShiftLeft = 12,
+    ShiftRight = 13,
+    Add = 14,
+    Sub = 15,
+    Mul = 16,
+    Div = 17,
+}


Is there a reason why we're specifying the discriminants for this enumeration? I'm assuming you're doing this to avoid having a 0 case representing a default which syntactically shouldn't make sense.

Good question. The reason is that the exact values are part of our QPY serialization format. While it isn't this enum that gets serialized directly (it's the Binary.Op Python-side sister enum), I'm using bytemuck to convert between this enum and the Python one, so these need to have the same values.

raynelfss · 2025-04-14T20:05:49Z

crates/circuit/src/classical/expr/binary.rs

+/// they are public in the sister Python enum `_BinaryOp` in `expr.py`
+/// and used in our QPY serialization format.
+///
+/// WARNING: If you add more, **be sure to update expr.py**, too.


Maybe add that the CheckedBitPattern trait implementation also needs to be modified.

raynelfss · 2025-04-14T21:03:14Z

crates/circuit/src/classical/expr/index.rs

+    #[new]
+    #[pyo3(text_signature = "(target, index, type)")]
+    fn new(py: Python, target: Expr, index: Expr, ty: Type) -> PyResult<Py<Self>> {
+        let constant = target.is_const() && index.is_const();


I noticed that we're saving this boolean a lot when it seems to just be obtained by the method is_const of the underlying Expr being saved. Could we instead compute these in-place by just creating an is_const method for the Expr in question here or is it easier to just store this attribute?

That's a good observation. We could recursively call is_const on any child Exprs from the implementation of is_const itself rather than doing this in the constructor, but I didn't like the idea of is_const being more expensive for larger expression trees. By pre-computing it in the constructor, we avoid this.

raynelfss · 2025-04-14T21:29:58Z

crates/circuit/src/classical/expr/stretch.rs

+    #[new]
+    fn new0(var: &Bound<PyAny>, name: String) -> PyResult<Py<Self>> {
+        Py::new(
+            var.py(),
+            (
+                PyStretch(Stretch {
+                    uuid: var.getattr(intern!(var.py(), "int"))?.extract()?,
+                    name,
+                }),
+                PyExpr(ExprKind::Stretch),
+            ),
+        )
+    }
+
+    /// Generate a new named stretch variable.
+    #[classmethod]
+    fn new(cls: &Bound<'_, pyo3::types::PyType>, name: String) -> PyResult<Py<Self>> {
+        Py::new(
+            cls.py(),
+            (
+                PyStretch(Stretch {
+                    uuid: Uuid::new_v4().as_u128(),
+                    name,
+                }),
+                PyExpr(ExprKind::Stretch),
+            ),
+        )
+    }


About these two constructors for stretch. What is the first one used for? And also, since both of them seem to do a lot of the same construction (with the only difference being how they extract their respective uuid's. Should we maybe separate the construction into a different method? Or is too much for such little difference.

The first one is actually just the Python "new", e.g. what gets invoked with Python code like: Stretch(uuid.uuid4(), "a"). The second one is for use like: Stretch.new("a"). I probably should have named new0 as py_new instead, since we have the pattern elsewhere.

Regarding the duplication, I'm not sure it'd be helpful to abstract this. It's all pretty declarative without much logic, so IMO we'd be making things more obscure.

raynelfss · 2025-04-14T21:34:45Z

crates/circuit/src/classical/expr/unary.rs

+/// they are public in the sister Python enum `_UnaryOp` in `expr.py`
+/// and used in our QPY serialization format.
+///
+/// WARNING: If you add more, **be sure to update expr.py**, too.


... and the CheckedBitPattern trait :)

raynelfss · 2025-04-14T21:56:11Z

crates/circuit/src/classical/expr/var.rs

+    fn new0(var: &Bound<PyAny>, ty: Type, name: Option<String>) -> PyResult<Py<Self>> {
+        let v = if let Some(name) = name {
+            Var::Standalone {
+                uuid: var.getattr(intern!(var.py(), "int"))?.extract()?,
+                name,
+                ty,
+            }
+        } else if let Ok(register) = var.extract::<ClassicalRegister>() {
+            Var::Register { register, ty }
+        } else {
+            Var::Bit {
+                bit: var.extract()?,
+            }
+        };
+        Py::new(var.py(), (PyVar(v), PyExpr(ExprKind::Var)))
+    }
+
+    /// Generate a new named variable that owns its own backing storage.
+    #[classmethod]
+    fn new(cls: &Bound<'_, pyo3::types::PyType>, name: String, ty: Type) -> PyResult<Py<Self>> {
+        Py::new(
+            cls.py(),
+            (
+                PyVar(Var::Standalone {
+                    uuid: Uuid::new_v4().as_u128(),
+                    name,
+                    ty,
+                }),
+                PyExpr(ExprKind::Var),
+            ),
+        )
+    }


Seeing that these two constructors refer to generating vars of different styles (one being new style while the other two refer to older styles). Are there plans for removing the old style in the future, or are the new style vars just an additional use case?

I believe the old style is here to stay, at least for the foreseeable future (also, note that these docstrings I've simply moved from the Python code to their place here).

The "old-style" is pretty much just Vars backed by classical registers and classical bits. The "new-style" ones use the Store instruction to assign values to named variables (which we use UUIDs to track uniquely).

raynelfss · 2025-04-14T22:05:20Z

qiskit/circuit/classical/types/types.py

-class _Singleton(type):
-    """Metaclass to make the child, which should take zero initialization arguments, a singleton
-    object."""
-
-    def _get_singleton_instance(cls):
-        return cls._INSTANCE
-
-    @classmethod
-    def __prepare__(mcs, name, bases):  # pylint: disable=unused-argument
-        return {"__new__": mcs._get_singleton_instance}
-
-    @staticmethod
-    def __new__(cls, name, bases, namespace):
-        out = super().__new__(cls, name, bases, namespace)
-        out._INSTANCE = object.__new__(out)  # pylint: disable=invalid-name
-        return out


I'm assuming we don't use this anymore because of how we've implemented Type from Rust. But I imagined this was used to discriminate against some types. Knowing that the other types are "stateless" the only one that wasn't singleton was/is Unit as it contains a u16.

I know there's a test somewhere that asserts that the Bool type is a singleton. But, our code doesn't actually rely on this anywhere from what I could tell.

...but, all of the types that were singletons in Python I've actually made to be singletons in Rust as well. You'll notice I'm caching these instances at the top of types.rs 🙂.

static BOOL_TYPE: GILOnceCell<Py<PyBool>> = GILOnceCell::new(); static DURATION_TYPE: GILOnceCell<Py<PyDuration>> = GILOnceCell::new(); static FLOAT_TYPE: GILOnceCell<Py<PyFloat>> = GILOnceCell::new();

I thought about also caching common Uint types like Uint(8) or Uint(16), or having a lazily populated table of cached Uints of various sizes, but decided it'd be overkill (esp. since we don't have that today).

raynelfss

This looks about ready to merge. I just have one comment on the naming convention based on something you mentioned before.

crates/circuit/src/classical/expr/stretch.rs

crates/circuit/src/classical/expr/var.rs

raynelfss

LGTM!

kevinhartman added 2 commits March 21, 2025 14:40

Initial commit.

f0e5ba2

Implement types.

b75ce56

kevinhartman added 11 commits March 24, 2025 12:40

Wire-up Rust-based Types.

287f22a

Add more plumbing for Expr.

bbb0324

Expose Op enums.

8430dab

WIP

de02a90

WIP

cab62d8

Test passing.

1badcbd

Refactor module organization.

0efecd9

Fix clippy.

07abfec

Merge branch 'main' of github.com:Qiskit/qiskit into oxidize-expr

1819b8b

Downcast instead of extract.

a266972

Fix merge.

0ad84be

mtreinish mentioned this pull request Mar 27, 2025

Unify identifer handling of Var and Stretch in DAGCircuit #14000

Merged

1 task

kevinhartman added 5 commits March 29, 2025 11:03

Add missing documentation, fix opcode docs.

1ab4f45

Impl repr for expressions.

f2b1117

Merge branch 'main' of github.com:Qiskit/qiskit into oxidize-expr

2123357

Fix pickling for facade enums.

2859e11

Manually revert DAGCircuit changes.

35938ec

I'll do these in a separate PR.

kevinhartman marked this pull request as ready for review March 31, 2025 21:48

kevinhartman requested a review from a team as a code owner March 31, 2025 21:48

mtreinish reviewed Mar 31, 2025

View reviewed changes

kevinhartman mentioned this pull request Apr 4, 2025

Oxidize classical expression use in Rust code. #14176

Merged

Fix lint.

61a99d2

raynelfss self-assigned this Apr 10, 2025

kevinhartman added 2 commits April 11, 2025 10:59

Merge branch 'main' of github.com:Qiskit/qiskit into oxidize-expr

017efea

Use fast-rng feature for uuid.

4931879

kevinhartman requested a review from mtreinish April 14, 2025 18:14

raynelfss reviewed Apr 14, 2025

View reviewed changes

eliarbel added this to the 2.1.0 milestone Apr 15, 2025

raynelfss reviewed Apr 18, 2025

View reviewed changes

crates/circuit/src/classical/expr/stretch.rs Outdated Show resolved Hide resolved

crates/circuit/src/classical/expr/var.rs Outdated Show resolved Hide resolved

kevinhartman added 4 commits April 22, 2025 13:36

Address review comments.

a20a7e7

Merge branch 'main' of github.com:Qiskit/qiskit into oxidize-expr

f47a9c5

Address another review comment.

ff15025

Update Cargo.lock.

a0a3ce2

raynelfss approved these changes Apr 22, 2025

View reviewed changes

raynelfss added this pull request to the merge queue Apr 22, 2025

Merged via the queue into Qiskit:main with commit 2dded80 Apr 22, 2025
24 checks passed

This was referenced Apr 24, 2025

Add a Rust representation of Expr #13275

Closed

Add a Rust reperesentation of Var type #13274

Closed

raynelfss mentioned this pull request May 30, 2025

Add a rust-native substitute_node_with_dag #14512

Closed

5 tasks

eliarbel added the Changelog: None Do not include in changelog label Jun 3, 2025

kevinhartman mentioned this pull request Jun 18, 2025

Port classical expression constructors to Rust #14643

Open

jmmshn mentioned this pull request Aug 19, 2025

Fix GPU builds and Tests (Qiskit>=2.1) Qiskit/qiskit-aer#2361

Open

Port classical expressions (Expr) to Rust. #14068

Port classical expressions (Expr) to Rust. #14068

Uh oh!

Conversation

kevinhartman commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

expr::Expr enum

Binary.Op and Unary.Op

types::Type enum

To-do

Uh oh!

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

Uh oh!

Pull Request Test Coverage Report for Build 14601151850

Details

💛 - Coveralls

Uh oh!

qiskit-bot commented Mar 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raynelfss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raynelfss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

raynelfss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Port classical expressions (`Expr`) to Rust. #14068

Port classical expressions (`Expr`) to Rust. #14068

kevinhartman commented Mar 21, 2025 •

edited

Loading

`expr::Expr` enum

`Binary.Op` and `Unary.Op`

`types::Type` enum

coveralls commented Mar 21, 2025 •

edited

Loading