[Stretch] Support stretch and duration expressions for Delay instruction.
#13853
Conversation
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.
| class _EvalDurationImpl(expr.ExprVisitor[float]): | ||
| """Evaluates the expression to a single float result. | ||
|
|
||
| If ``dt`` is provided or all durations are already in ``dt``, the result is in ``dt``. | ||
| Otherwise, the result will be in seconds, and all durations MUST be in wall-time (SI). | ||
| """ | ||
|
|
||
| __slots__ = ("dt", "has_dt", "has_si") |
There was a problem hiding this comment.
Absolutely applaud the effort here haha. I'm not certain who's going to write an expression using only duration literals in a way that is valid through this pass via constant folding, but I love that it'll work if they do.
Python 3.9 might not define the | operator for ParameterValueType since it's an alias.
There was a problem hiding this comment.
Overall this looks great, thanks for doing this! I left a few inline comments but none of them need fixing (most are just idle musings on my part) except for the tests which will cause a conflict because main has changed on you and you can't use the deprecated arguments to transpile you were using before.
| raise CircuitError(f"Expression duration of type '{parameter.type}' is not valid.") | ||
| if not parameter.const: | ||
| raise CircuitError("Duration expressions must be constant.") | ||
| return parameter | ||
| elif isinstance(parameter, ParameterExpression): |
There was a problem hiding this comment.
It always surprises me that we allow this. Who is going to make their delay acos(A) / pi + e^(pi*B)
| ) -> InstructionSet: | ||
| """Apply :class:`~.circuit.Delay`. If qarg is ``None``, applies to all qubits. | ||
| When applying to multiple qubits, delays with the same duration will be created. | ||
|
|
||
| Args: | ||
| duration (int or float or ParameterExpression): duration of the delay. | ||
| duration (Object): |
There was a problem hiding this comment.
FWIW you can just drop (Object) here it's all superseded by the type hinting in the docs.
| # passes anyway, so we bail out. In theory, we _could_ still traverse | ||
| # through the stretch expression and replace any Duration value nodes it may | ||
| # contain with ones of the same units, but it'd be complex and probably unuseful. | ||
| self.property_set["time_unit"] = "stretch" |
There was a problem hiding this comment.
Does anything use this property set value? If so how do they handle the string stretch here? I would expect our current scheduling passes to just skip or fail in the presence of stretches.
Is this what raises the error when transpilation is called with a stretch duration?
There was a problem hiding this comment.
Yeah, the scheduling passes look at this and raise if it's "stretch". I think some of the other uses of time_unit in the property set have gone away since I started this work, but I think we still need something like this to let the scheduling passes know they shouldn't run, unless we want to examine all of the delays again.
Summary
Adds support for using classical expressions of type
StretchandDurationas thedurationof aDelayinstruction.This is probably the final PR in the series 🎉. All the missing QPY stuff I'll be doing on the earlier PR branches, and will merge all of that plus fixes forward through to this.
Details and comments
Based on #13852. More readable diff here.To-do
[ ] Release note.edit: I'd say this is already captured in previous release notes of this series