[Stretch] Add `+`, `-`, `*`, `/` operators for classical expressions. by kevinhartman · Pull Request #13850 · Qiskit/qiskit

kevinhartman · 2025-02-15T02:53:37Z

Summary

Adds new arithmetic operators for use with numeric and a duration expressions. These are needed to express relationships between stretch variables and durations.

Details and comments

~~Based on #13844. More readable diff here..~~

All new operations support Uint and Float, but these cannot be intermixed. You must first cast one to the other explicitly, since Float => Uint and Uint => Float are both considered dangerous cast kinds. If two Uints of different width are used, the wider one is chosen for the expression's output type.
Addition and subtraction support for Duration.
Multiplication is supported Duration and Float operands. Currently, a Uint must first be cast to a Float to be used as a timing multiplier.
- The type system does not prevent us from representing non-const expressions of type Duration, but the expr constructors will raise a ValueError if a user tries to produce such an expression (this was not possible in the original design of const-types, but what we have now feels more future-proof than baking this as a hard requirement into the type system).
- A user would also be able to create a Var with a Duration type, which would not be a constant expression. If desired, we can block users from adding such variables to a circuit.
Division is supported between Duration and Float operands, but only where the duration is in the left position. It is also supported between two Duration types, in which case the result is a Float. Dividing a Duration by a non-const expression will raise a ValueError.

To-do

Release note.

This reverts commit e2b3221.

Types that have some natural order no longer have an ordering when one of them is strictly greater but has an incompatible const-ness (i.e. when the greater type is const but the other type is not).

We need to reject types with const=True in QPY until it supports them. For now, I've also made the Index and shift operator constructors lift their RHS to the same const-ness as the target to make it less likely that existing users of expr run into issues when serializing to older QPY versions.

This is probably a better default in general, since we don't really have much use for const except for timing stuff.

Since we're going for using a Cast node when const-ness differs, this will be fine.

I wasn't going to have this, but since we have DANGEROUS Float => Int, and we have Int => Bool, I think this makes the most sense.

A Stretch can always represent a Duration (it's just an expression without any unresolved stretch variables, in this case), so we allow implicit conversion from Duration => Stretch. The reason for a separate Duration type is to support things like Duration / Duration => Float. This is not valid for stretches in OpenQASM (to my knowledge).

Also adds support to expr.lift to create a value expression of type types.Duration from an instance of qiskit.circuit.Duration.

jakelishman

I haven't looked at the test files (need to go to dinner), but here's a super quick scan over the implementation.

jakelishman

Thanks for the changes! I think everything here is all right (though I don't trust that I read every test exactly correctly haha).

jakelishman · 2025-03-05T15:03:38Z

+    """
+    left, right = _lift_binary_operands(left, right)
+    type: types.Type
+    if left.type.kind is right.type.kind is types.Duration:


ooh, chained is comparisons. Not a Python feature you see every day haha

jakelishman · 2025-03-05T15:05:03Z

        right-hand operand.  In all cases, the output bit width is the same as the input, and zeros
        fill in the "exposed" spaces.
+
+        The binary arithmetic operators :data:`ADD`, :data:`SUB:, :data:`MUL`, and :data:`DIV`


Suggested change

The binary arithmetic operators :data:`ADD`, :data:`SUB:, :data:`MUL`, and :data:`DIV`

The binary arithmetic operators :data:`ADD`, :data:`SUB`, :data:`MUL`, and :data:`DIV`

Maybe apply this in the next PR, just so I can get this one in the merge queue already?

jakelishman · 2025-03-05T15:30:35Z

+                opcode, expr.Var(cr, types.Uint(8)), expr.Value(200, types.Uint(8)), types.Uint(8)
+            ),
+        )
+        self.assertFalse(function(cr, 200).const)


Super minor, but the expr.Binary equality check should already be doing the const checks. Could we have written the tests such that the const is set explicitly in the expr.Binary constructor?

(No need to change all the tests now.)

jakelishman · 2025-03-05T15:33:11Z

+        # Multiply timing expressions by non-const floats:
+        non_const_float = expr.Var.new("a", types.Float())
+        with self.assertRaisesRegex(ValueError, "would result in a non-const"):
+            expr.mul(Duration.dt(1000), non_const_float)
+        with self.assertRaisesRegex(ValueError, "would result in a non-const"):
+            expr.mul(non_const_float, Duration.dt(1000))


In a world where we have a use and a way of functioning with runtime-specified delay, I can imagine us relaxing this restriction, but I think it's good to assert right now.

kevinhartman added 30 commits February 7, 2025 16:19

WIP

af4ba52

Add try_const to lift.

e423bf9

Try multiple singletons, new one for const.

0a5917b

Revert "Try multiple singletons, new one for const."

a97434d

This reverts commit e2b3221.

Remove Bool singleton test.

1afc965

Add const handling for stores, fix test bugs.

86655f1

Fix formatting.

aaeae9b

Remove Duration and Stretch for now.

a2a444b

Cleanup, fix const bug in index.

8ac2dc3

Fix ordering issue for types with differing const-ness.

9f8313c

Types that have some natural order no longer have an ordering when one of them is strictly greater but has an incompatible const-ness (i.e. when the greater type is const but the other type is not).

Make expr.Lift default to non-const.

71b7e7a

This is probably a better default in general, since we don't really have much use for const except for timing stuff.

Revert to old test_expr_constructors.py.

2091557

Make binary_logical lift independent again.

7307be9

Since we're going for using a Cast node when const-ness differs, this will be fine.

Update tests, handle a few edge cases.

9b30284

Fix docstring.

ce1faf1

Remove now redundant arg from tests.

4fee48f

Add const testing for ordering.

88ab046

Add const tests for shifts.

c5a230f

Add release note.

7c88d88

Add const store tests.

c58a7b8

Address lint, minor cleanup.

d9e9a8c

Add Float type to classical expressions.

4a56150

Allow DANGEROUS conversion from Float to Bool.

23b5961

I wasn't going to have this, but since we have DANGEROUS Float => Int, and we have Int => Bool, I think this makes the most sense.

Test Float ordering.

8bf2e4f

Improve error messages for using Float with logical operators.

111eb32

Float tests for constructors.

a839d51

Add release note.

a19b39a

Add Duration type to qiskit.circuit.

55af327

Also adds support to expr.lift to create a value expression of type types.Duration from an instance of qiskit.circuit.Duration.

kevinhartman added 11 commits February 28, 2025 19:10

Merge branch 'expr-timing' into math-expr

65f2198

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

34e615b

Remove Stretch type.

bbe67a6

Remove a few more mentions of the binned stretch type.

0acc450

Add docstring for Duration.

7fea216

Merge branch 'expr-timing' into math-expr

4a317e8

Remove Stretch type stuff.

f5709ae

Address review comments.

78b2951

Remove unused import.

97bbeba

Merge branch 'expr-timing' into math-expr

3a34220

Remove visitor short circuit.

49f2af8

jakelishman reviewed Mar 3, 2025

View reviewed changes

kevinhartman added 5 commits March 3, 2025 14:40

Merge branch 'expr-timing' into math-expr

69b985b

Address review comments.

f780d9a

Address review comments.

e7ca9c4

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

d4cef21

Support division by Uint.

60bcfdb

kevinhartman added Changelog: Added Add an "Added" entry in the GitHub Release changelog. mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library and removed on hold Can not fix yet labels Mar 3, 2025

kevinhartman requested a review from jakelishman March 3, 2025 21:55

Fix lint.

d40cb32

1ucian0 assigned jakelishman and unassigned jakelishman Mar 4, 2025

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

ad54644

mtreinish added the priority: high label Mar 4, 2025

jakelishman approved these changes Mar 5, 2025

View reviewed changes

jakelishman added this pull request to the merge queue Mar 5, 2025

Merged via the queue into Qiskit:main with commit 22daa04 Mar 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Stretch] Add `+`, `-`, `*`, `/` operators for classical expressions.#13850

[Stretch] Add `+`, `-`, `*`, `/` operators for classical expressions.#13850
jakelishman merged 121 commits intoQiskit:mainfrom
kevinhartman:math-expr

kevinhartman commented Feb 15, 2025 •

edited

Loading

Uh oh!

jakelishman left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakelishman left a comment

Uh oh!

jakelishman Mar 5, 2025

Uh oh!

jakelishman Mar 5, 2025

Uh oh!

jakelishman Mar 5, 2025

Uh oh!

jakelishman Mar 5, 2025

Uh oh!

jakelishman Mar 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	The binary arithmetic operators :data:`ADD`, :data:`SUB:, :data:`MUL`, and :data:`DIV`
	The binary arithmetic operators :data:`ADD`, :data:`SUB`, :data:`MUL`, and :data:`DIV`

Conversation

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

Uh oh!

Summary

Details and comments

To-do

Uh oh!

jakelishman 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!

jakelishman left a comment

Choose a reason for hiding this comment

Uh oh!

jakelishman Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

jakelishman Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

jakelishman Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

jakelishman Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

jakelishman Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kevinhartman commented Feb 15, 2025 •

edited

Loading