-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Expr
support to QuantumCircuit.compose
#10375
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 5598447243
💛 - Coveralls |
This relatively straightforwardly generalises the mapping of variables that already exists in `QuantumCircuit.compose` to be able to handle arbitrary `Expr` nodes as well. We must take care not to accidentally mutate the `Expr` nodes in the input circuit in the name of efficiency.
3d77997
to
9d540d1
Compare
Rebased over |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
def visit_value(self, node, /): | ||
return expr.Value(node.value, node.type) | ||
|
||
def visit_unary(self, node, /): | ||
return expr.Unary(node.op, node.operand.accept(self), node.type) | ||
|
||
def visit_binary(self, node, /): | ||
return expr.Binary(node.op, node.left.accept(self), node.right.accept(self), node.type) | ||
|
||
def visit_cast(self, node, /): | ||
return expr.Cast(node.operand.accept(self), node.type, implicit=node.implicit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it wouldn't really come up again, but if it does, it might make sense to have a transformer base class specific for expr.Expr
that has default implementations like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean like a subclass of ExprVisitor
that (by default) copies the structure by producing the exact same Expr
, but visiting each child node, with the intention that a consumer overrides only the behaviour they need?
I could potentially see that being useful, yeah - I'd maybe like to wait to see if we have other use-cases for it first, before adding more API surface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, exactly 😄. Totally agree, makes sense to wait.
Summary
This relatively straightforwardly generalises the mapping of variables that already exists in
QuantumCircuit.compose
to be able to handle arbitraryExpr
nodes as well. We must take care not to accidentally mutate theExpr
nodes in the input circuit in the name of efficiency.Details and comments
Resolve #10227. Depends on #10359. Changelog to come in #10331.