Add representation of box
#13869
Add representation of box
#13869
Conversation
jakelishman
added
Changelog: New Feature
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 13687126255Details
💛 - Coveralls |
jakelishman
force-pushed
the
box/repr
branch
from
f3a933d to
ce18a24
Compare
jakelishman
force-pushed
the
box/repr
branch
from
ce18a24 to
13fcd65
Compare
This adds in the base `box` control-flow construction, with support for containing instructions and having a literal delay, like the `Delay` instruction. This supports basic output to OpenQASM 3, QPY and some rudimentary support in the text and mpl drawers. The transpiler largely handles things already, since control flow is handled generically in most places. Known issues: - We expect this to be able to accept stretches in its duration, just as `Delay` can, which will need a follow-up. - We expect `Box` to support "annotations" in a future release of Qiskit. - There is currently no way in OpenQASM 3 to represent a qubit that is idle during a `box` without inserting a magic instruction on it. - IBM backends don't claim support for `box` yet, so `transpile` against a backend will fail, though you can modify the `Target` to add the instruction manually. Add tests of box
jakelishman
force-pushed
the
box/repr
branch
from
13fcd65 to
6a29bd4
Compare
|
One or more of the following people are relevant to this code:
|
jakelishman
force-pushed
the
box/repr
branch
from
4e2df03 to
c3cb4dd
Compare
| @params.setter | ||
| def params(self, parameters): | ||
| # pylint: disable=cyclic-import | ||
| from qiskit.circuit import QuantumCircuit | ||
|
|
||
| (body,) = parameters | ||
|
|
||
| if not isinstance(body, QuantumCircuit): | ||
| raise CircuitError( | ||
| "BoxOp expects a body parameter of type " | ||
| f"QuantumCircuit, but received {type(body)}." | ||
| ) | ||
|
|
||
| if body.num_qubits != self.num_qubits or body.num_clbits != self.num_clbits: | ||
| raise CircuitError( | ||
| "Attempted to assign a body parameter with a num_qubits or " | ||
| "num_clbits different than that of the BoxOp. " | ||
| f"BoxOp num_qubits/clbits: {self.num_qubits}/{self.num_clbits} " | ||
| f"Supplied body num_qubits/clbits: {body.num_qubits}/{body.num_clbits}." | ||
| ) | ||
|
|
||
| self._params = [body] |
There was a problem hiding this comment.
Do we need to allow inplace mutation like this? I guess it's a required part of the data model, it just seems like playing with fire to support it given how much trouble we had with this on gates.
But this also potentially problematic when we migrate control flow to rust.
There was a problem hiding this comment.
Yeah, unfortunately I just copied it from the other control-flow ops - we're already in trouble.
There was a problem hiding this comment.
I guess I was thinking we don't need to continue the trend here. Or raise a NotImplementedError if it is required. But this is a reminder that we need to add the docs about not supporting mutating Instruction subclasses after inserting them into a circuit. We've done it piecemeal as we moved things into rust, but we meant to document it in 2.0 as not allowed for anything.
There was a problem hiding this comment.
I'd rather be consistent with all control-flow ops, and not add new restrictions now - I wouldn't want to bet it's not used by the Instruction init or even some other library code.
There was a problem hiding this comment.
In retrospect: this needs to be settable so QuantumCircuit.assign_parameters works (or maybe we only mutate the list in-place? Can't remember.)
| super().__init__("box", body.num_qubits, body.num_clbits, [body], label=label) | ||
| self.duration = duration | ||
| self.unit = unit | ||
|
|
There was a problem hiding this comment.
Would you be opposed to yet another alias to the blocks? But specifically, to get the only element.
@property
def content(self):
return self.params[0]There was a problem hiding this comment.
Done in 1367570. I called it body, though, if not only to match the variable terminology used in the class.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
This adds in the base
boxcontrol-flow construction, with support for containing instructions and having a literal delay, like theDelayinstruction.This supports basic output to OpenQASM 3, QPY and some rudimentary support in the text and mpl drawers. The transpiler largely handles things already, since control flow is handled generically in most places.
Known issues:
Delaycan, which will need a follow-up.Boxto support "annotations" in a future release of Qiskit.boxwithout inserting a magic instruction on it.boxyet, sotranspileagainst a backend will fail, though you can modify theTargetto add the instruction manually.Summary
Details and comments
No tests yet, but I tested various things casually - this is just the WIP to show what's going on.Close #13772.