[Stretch] Support `Duration` type in classical expressions. #13844

kevinhartman · 2025-02-14T00:17:11Z

Summary

Adds new type Duration to the classical expression type system, as well as a new (and separate) core Duration type to qiskit.circuit.

Details and comments

~~Based on #13832. Here's a readable diff in the meantime.~~ Rebased on main.

Core type

The new core type Duration is a pyclass implemented in Rust as an enum type. For now, it's got variants for each of the supported timing kinds defined in OpenQASM (ns, us, ms, s, dt). All of these are f64s, except for dt which is a u64.

Equality of Duration currently follows the same choices made by #13816, in that durations are not first brought to the same unit and compared for magnitude. Perhaps in the future, we can consider equality between walltimes in different units if that feels useful.

Expression types

Like in QASM, the Duration type represents a length of time, possibly negative. Expressions of type Duration may be resolvable immediately, or may contain stretch expressions (to be added in #13852).

To-do

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.

This feels like a bit of a hack, but the idea is to override a Var to report itself as a constant expression only in the case that its type is Stretch. I would argue that it's not quite as hack-y as it appears, since Stretch is probably the only kind of constant we'll ever allow in Qiskit without an in-line initializer. If ever we want to support declaring other kinds of constants (e.g. Uint), we'll probably want to introduce a `expr.Const` type whose constructor requires a const initializer expression.

jakelishman

Hard to review cleanly until we're more merged, but I think the couple of comments I've got here are important to get out sooner rather than later.

I'm also thinking that circuit.Duration ought to become a valid input type to Delay, but if that happens (and if you've already done it, just ignore me) it'd be a follow-up anyway.

jakelishman · 2025-02-28T23:37:10Z

qiskit/circuit/classical/expr/expr.py

        super().__setattr__("type", type)
-        super().__setattr__("const", False)
+        # For now, Stretch is the only kind of const variable we allow.
+        # In the future, we may want to add a 'const' constructor arg here
+        # to let users create other kinds of constants, or perhaps introduce
+        # a separate expr.Const that requires a const expr initializer for this
+        # purpose. `QuantumCircuit.add_stretch` is the official way to create
+        # stretches, and makes no promise that we will track stretches using
+        # `Var` (it accepts just a name and returns just _some_ `Expr`).
+        super().__setattr__("const", type.kind is types.Stretch)


Given what we talked about offline, how plausible is it to instead have stretch be represented by a new node in the Expr tree called Const?

I would like to keep the type-system association we'd newly made with the move of the const system to Expr where Var is associated with a mutable l-value, Value was an immediate value, and now Const is some compile-time resolvable object. I know it might involve relatively major changes throughout the control-flow builders, but the DAG wires I think should automatically work, because we weren't adding them anyway.

I can live with it if is too much work - Var(const=True) and Var(const=False) is still distinguishable, if not as pretty as having it encoded in the types, not just the values.

Honestly, I'm not sure that a Const node would even be a good fit for uninitialized stretch variables. I think we will want a Const node eventually to represent const definitions with initializers, and I'd expect such a node to be constructed via expr.Const.new(name: str, type: types.Type, initializer: expr.Expr) where initializer gets validated to be a constant expression coercable to type.

I can live with it if is too much work - Var(const=True) and Var(const=False) is still distinguishable

I think that Var is not a good candidate for representing initialized constants, even with a potential const=True constructor argument, since all initialized constants require an initializer. Perhaps Var will take an initializer in Qiskit eventually, but it'd be optional and not required to be a constant expression.

That said, Var can be declared uninitialized, which IMO makes it the more natural fit for representing uninitialized stretch variables (between Var and an imagined Const).

What we may want eventually is a third expression kind, StretchVar. I didn't want to attempt to introduce that this late in the development. I think we can get away with piggybacking off of Var for now as an implementation detail. In #13852 I was planning to make QuantumCircuit.add_stretch take only a name (no var) and return just an expr.Expr, and then add separate methods for iterating and getting stretch variables + filtering them out of the normal var methods. It's unclear to me if they should still show up in the same set of captures, though.

jakelishman · 2025-02-28T23:50:01Z

qiskit/circuit/classical/types/ordering.py

+    (Duration, Duration): lambda _a, _b, /: Ordering.EQUAL,
+    (Duration, Stretch): lambda _a, _b, /: Ordering.LESS,
+    (Stretch, Stretch): lambda _a, _b, /: Ordering.EQUAL,
+    (Stretch, Duration): lambda _a, _b, /: Ordering.GREATER,


I'm not dead certain that this is right. The introduction of the classes calls Duration "a length of time, possibly negative", and Stretch "some not-yet-known non-negative duration". By those terms, let's associate Duration with int and Stretch with uint.

By comparison to integers: no int is ever less than any uint (because no uint can ever represent all the values of an int), but int can be greater than uint if its width is enough.

I think what we might actually be after here is to make Duration the "duration expression" type, and make Stretch an explicit subtype of Duration that is non-negative. Then the ordering should be Stretch < Duration, because Stretch is a strict subtype of it. So that's backwards to what we have here, but that's expected - Stretch should be stricter than Duration.

(Note that I don't mean that Stretch inherits from Duration in the Python-space class system - OO inheritance is not a good model for type systems - just that the type-theory type Stretch is a subtype of Duration.)

Starting out with this comment, since I think it's the most fundamental for us to make sure we get right!

The introduction of the classes calls Duration "a length of time, possibly negative", and Stretch "some not-yet-known non-negative duration".

I want to be clear that this is not something I've come up with, but rather something I pulled directly from the OpenQASM docs here:

Stretchable durations have variable non-negative duration that are permitted to grow as necessary to satisfy constraints.
Source: https://openqasm.com/language/delays.html#duration-and-stretch-types

and later

Negative durations are allowed, however passing a negative duration to a gate[duration] or box[duration] expression will result in an error.
Source: https://openqasm.com/language/delays.html#operations-on-durations

My assumption based on that last bit is that this must be some kind of compile-time check, separate from QASM's type system, e.g. "evaluate all compile-time expressions, and if a stretch resolves to a negative duration which gets fed into a timing-aware instruction, blow up."

In the second link, we also see this example:

duration a = 300ns; duration b = durationof({x $0;}); stretch c; // stretchy duration with min=300ns stretch d = a + 2 * c; // stretchy duration with backtracking by up to half b stretch e = -0.5 * b + c;

Particularly, stretch d = a + 2 * c; implies that Duration + Stretch => Stretch, which certainly makes it seem like Stretch is greater than Duration in the typing.

Stepping back from that, I think we have 3 separate things going on in all of this:

We have uninitialized stretch variables, which are considered constants.

We have stretch expressions, which can (must?) include uninitialized stretch variables, constrained by duration literals.

We need a type that represents just a literal duration (e.g. 1000ns), since there are transformations only applicable to these, like Duration / Duration => Float. From what I understand Stretch / Stretch is not defined in QASM.

On point 2., my assumption has been that stretch s = 1000dt; is valid and implies a stretch-i-ness of 0, hence the ordering relationship in this PR. If that's not the case, then I think Stretch and Duration are unordered. We can't put a Stretch into a Duration, since we'll lose point 3.

Oh yeah, I wasn't thinking you were making it up - I just think it's not quite the cleanest way of representing the OQ3 spec. I see where you're coming from, I just think there's a combination of typing restrictions and value restrictions in play here, and we're not in the right one.

For your points one and two: the statement stretch d (= Expr)?; introduces a new degree of freedom into the system. If the initialisation expression is present, it's just a lower bound on the constrained value, clipped to 0 (because stretches can never be negative). That doesn't necessarily mean that the type of Expr needs to be <= stretch - it's actually kind of the other way round; as long as you can embed stretch in the valid total ordering of the type of the initialisation expression, you're fine. The statement stretch d = a + 2*c; might be mathematically clearer as stretch d >= a + 2*c;. So for:

Particularly, stretch d = a + 2 * c; implies that Duration + Stretch => Stretch, which certainly makes it seem like Stretch is greater than Duration in the typing.

I think that's a mistaken inference: a + 2*c need not be positive at all, and stretch d = -500dt; is still a valid initialiser and doesn't require -500dt to be a valid value in stretch. (In my example, there's two immediate restrictions on d: d >= -500dt from the explicit restriction, and d >= 0dt from the typing.

On point 2., my assumption has been that stretch s = 1000dt; is valid and implies a stretch-i-ness of 0,

I think this isn't right: stretch s introduces a new degree of freedom, and stretch s = 1000dt; is saying s >= 1000dt. Otherwise your a + 2*c also wouldn't have introduced a new degree of freedom - it'd be constrained entirely by a and c.

Just below your linked section, there's the line

OpenQASM and OpenPulse have a delay instruction, whose duration is defined by a duration.

So in the statement delay[<expr>], <expr> must be a valid duration. There's then a value restriction on that (as opposed to a typing one) that it must resolve (after stretch resolution and constant folding, etc) to a concrete non-negative value. That's not unprecedented in type systems: Python indexes with int, but negative values have a totally different meaning (you could consider them a different operation, also defined in terms of non-negative integers); and array indexing has an upper bound on the valid values too to make it a valid instruction, regardless of the type width.

I would say that a stretch always resolves to a valid non-negative duration. That a stretch always resolves to a duration at all is already a statement that stretch <= duration in the typing, really. duration in OQ3 is more properly a "time delta", which can also be negative. A delay takes a delta as argument, but is value-constrained to be a positive delta.

For point 3: I get what you're saying, but I don't think you necessarily need this to be in the type system, so it's a trade-off between this property or the partial ordering in terms of representable values. There's already plenty of operations that are ill-defined for certain values of a type - / of course isn't defined for 0 values on the RHS for integers. I think it's fine to admit all of:

a / b is valid for duration

stretch < duration

and a / b is undefined if b resolves to have degrees of freedom.

I'd accept the argument that stretch and duration could be unordered because the division operation isn't defined, but if we do that, then we have to special-case all the "cast" logic, which most naturally wants to use the partial ordering of representable values.

Using the type system to avoid unresolvable operations would be nice, but it's not universally possible, and I'd call division just one of these cases.

Final bits / clarifying:

it's already possible to use our type system to create unresolvable stretches - have box { delay [a] $0; delay [a + 10dt] $1; } and there's no valid resolution of a. I'd argue we just put "division" of stretches into the same vein, and not all divisions are necessarily unresolvable: if stretch a = 10dt;, then a / a is mathematically well-defined to be 1 (because a can't be zero - that's the only thing that would be a problem).

I think keeping the partial ordering to just mean "in terms of representable values is what we want". For example, uint[16] usefully defined as being < int[32] for the purposes of safe casting, but int[32] supports only a subset of the operations of uint[16] (no bitwise stuff).

We could argue that stretch isn't a "type" at all, it's just an unknown duration (though I probably wouldn't - it's harder to track them, I think).

I'd say that the only objects of type stretch are the exact names declared stretch <id>. I'd call the initialiser expression (if present) a duration, and interpret it as a limit duration(<stretch id>) >= <duration expr>.

From offline conversations: we actually decided that stretch needn't be a separate type from duration at all. Most of the rest of this conversation stands, though.

Yeah, and thanks for this comment, it was quite helpful.

the statement stretch d (= Expr)?; introduces a new degree of freedom into the system. If the initialisation expression is present, it's just a lower bound on the constrained value, clipped to 0 (because stretches can never be negative).

This is what I wasn't getting—the = operator in QASM is more or less overloaded for a stretch declaration. I'd assumed we weren't introducing a new degree of freedom at all. As you pointed out, really the stretch declaration always introduces a new degree of freedom, and optionally adds a lower bound if there's an "initializer".

All of this should be more or less handled now in the later PRs of this series.

qiskit/circuit/__init__.py

kevinhartman · 2025-03-02T16:48:22Z

After chatting a bit offline, me and @jakelishman came to the conclusion that it'd likely be more appropriate to represent stretch as an expression node of type Duration, and to forego having a separate Stretch type altogether. More details to follow in the later PRs of this series!

jakelishman

Thanks for the changes - small comments only, most we can just ignore if we don't want to make the changes in this PR.

crates/circuit/src/duration.rs

jakelishman · 2025-03-03T15:20:19Z

crates/circuit/src/duration.rs

+/// In Python 3.10+, you can use it in a match statement::
+///
+///   match duration:
+///      case Duration.dt(dt):
+///          return dt
+///      case Duration.s(seconds):
+///          return seconds / 5e-7
+///      case _:
+///          raise ValueError("expected dt or seconds")
+///
+/// And in Python 3.9, you can use ``isinstance`` to determine which variant
+/// is populated::
+///
+///   if isinstance(duration, Duration.dt):
+///       return duration[0]
+///   elif isinstance(duration, Duration.s):
+///       return duration[0] / 5e-7
+///   else:
+///       raise ValueError("expected dt or seconds")


This is a bit of a nuisance in Python 3.9. Perhaps cleaner to add a unit pyfunction that returns the string name, so you can dispatch on it? E.g.

#[pymethods] impl Duration { /// The corresponding ``unit`` of the duration. fn unit(&self) -> &'static str { match Self { Duration::ns(_) => "ns", // ... } } }

then Python-space has a cheaper way of doing the dispatch programmatically, and it maybe provides a slightly easier link with the legacy (value, unit) 2-tuple.

I'd say that ^that particular one is a candidate for doing in this PR (if you think it's worthwhile), but ones that would certainly be follow-ups if we wanted them to be: this class is a sensible place to add methods like as_seconds(self) -> f64, as_dt(self, dt_in_seconds) -> u64, etc, which would be useful if/when we swap to using this more in the internals.

Done in 78b2951.

I also added a py_value method to get the int or float (exposed in Python as Duration.value())

qiskit/circuit/classical/expr/visitors.py

jakelishman · 2025-03-03T15:26:09Z

qiskit/qasm3/ast.py

+class StretchType(ClassicalType):
+    """Type information for a stretch."""
+
+    __slots__ = ()
+
+


Maybe just semantics: my reading was we give-or-take decided to interpret the OQ3 spec as saying that stretch x; didn't involve a type stretch so much as just being a special keyword for a duration-typed declaration. I'm also fine to leave it the way we have (heaven knows this AST is a jumble), but in another world: would it make more sense to handle stretch x (= Expr)?; as a

class StretchDeclaration(Statement): identifier: Identifier bound: Optional[Expr]

rather than a type?

Ah yeah, that makes a ton more sense. We don't use this in this PR anyway, so if it's alright, I'll leave it for now and make the switch in the PR that actually does.

jakelishman · 2025-03-03T15:26:59Z

qiskit/qpy/__init__.py

+===========================  =========  ============================================================
+Python type / class          Type code  Payload
+===========================  =========  ============================================================
+``float``                    ``f``      One ``double value``.

-===========  =========  ============================================================================
+:class:`~.circuit.Duration`  ``t``      One ``DURATION``.
+
+===========================  =========  ============================================================


circuit.Duration is still a Python type, tbf

I think there's a risk with using t: I think that type code is already used elsewhere to represent "tuple", and there's a risk of a clash (despite the organisational structure on type_keys.py, the enums in there are not necessarily separate namespaces in QPY land). That said, I think EXPR_VALUE is localised enough that it can never hit the clash, but PARAM_TYPE would. Up to you if you think it's worth changing.

I might not be understanding. Are you saying that we should try to have unique characters across all of type_keys.py in case we introduce a bug that reads/writes a byte in the wrong place? My understanding of these is that we're manually reading / writing a byte that can only be one of e.g. TypeKeys.ExprType.<KEYS> in the place that we're reading / writing.

I can't remember the exact circumstances, but I remember having trouble with QPY at one point because there were locations where type keys from more than one TypeKeys.<Thing> were valid choices - I think the abstraction is leaky.

That said, given that I added the ExprType ones after that, I'm relatively sure that it's not a problem here, so it's ok to leave.

crates/circuit/src/duration.rs

jakelishman

Thanks, this looks good to go now!

jakelishman · 2025-03-03T18:19:11Z

qiskit/qpy/type_keys.py

+            if unit == "dt":
+                return cls.DT
+            if unit == "ns":
+                return cls.NS
+            if unit == "us":
+                return cls.US
+            if unit == "ms":
+                return cls.MS
+            if unit == "s":
+                return cls.S


tbf, we could have done this stuff (and elsewhere in the file) with static-dict lookups, but it's not at all important

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 8 commits February 27, 2025 17:03

Merge branch 'expr-float' into expr-timing

2d38178

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

42258b8

Merge branch 'expr-float' into expr-timing

481700a

Address review comments.

cbd55db

Update release note.

f12afb4

Merge branch 'expr-float' into expr-timing

8d059cd

Update docs.

e05d9ff

jakelishman reviewed Mar 1, 2025

View reviewed changes

kevinhartman added 3 commits February 28, 2025 19:06

Address review comments.

0f14cca

Merge branch 'expr-float' into expr-timing

7db8222

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

34e615b

kevinhartman removed the on hold Can not fix yet label Mar 1, 2025

Remove Stretch type.

bbe67a6

kevinhartman changed the title ~~[Stretch] Support Duration and Stretch types in classical expressions.~~ [Stretch] Support Duration type in classical expressions. Mar 2, 2025

kevinhartman added 2 commits March 2, 2025 11:52

Remove a few more mentions of the binned stretch type.

0acc450

Add docstring for Duration.

7fea216

kevinhartman requested a review from jakelishman March 2, 2025 17:52

jakelishman reviewed Mar 3, 2025

View reviewed changes

kevinhartman added 3 commits March 3, 2025 12:01

Address review comments.

78b2951

Remove unused import.

97bbeba

Remove visitor short circuit.

49f2af8

jakelishman approved these changes Mar 3, 2025

View reviewed changes

jakelishman enabled auto-merge March 3, 2025 18:19

jakelishman added Changelog: None Do not include in changelog mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library labels Mar 3, 2025

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

Merged via the queue into Qiskit:main with commit 7a1d51b Mar 3, 2025
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Stretch] Support `Duration` type in classical expressions. #13844

[Stretch] Support `Duration` type in classical expressions. #13844

kevinhartman commented Feb 14, 2025 •

edited

Loading

jakelishman left a comment

jakelishman Feb 28, 2025

kevinhartman Mar 1, 2025 •

edited

Loading

jakelishman Feb 28, 2025

kevinhartman Mar 1, 2025

jakelishman Mar 1, 2025

jakelishman Mar 3, 2025

kevinhartman Mar 3, 2025

kevinhartman commented Mar 2, 2025

jakelishman left a comment

jakelishman Mar 3, 2025

kevinhartman Mar 3, 2025

jakelishman Mar 3, 2025

kevinhartman Mar 3, 2025

jakelishman Mar 3, 2025

kevinhartman Mar 3, 2025

jakelishman Mar 3, 2025

jakelishman left a comment

jakelishman Mar 3, 2025

[Stretch] Support Duration type in classical expressions. #13844

[Stretch] Support Duration type in classical expressions. #13844

Conversation

kevinhartman commented Feb 14, 2025 • edited Loading

Summary

Details and comments

Core type

Expression types

To-do

jakelishman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinhartman Mar 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinhartman commented Mar 2, 2025

jakelishman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakelishman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[Stretch] Support `Duration` type in classical expressions. #13844

[Stretch] Support `Duration` type in classical expressions. #13844

kevinhartman commented Feb 14, 2025 •

edited

Loading

kevinhartman Mar 1, 2025 •

edited

Loading