-
Notifications
You must be signed in to change notification settings - Fork 603
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
Drawing a ControlledQubitUnitary #1174
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov Report
@@ Coverage Diff @@
## master #1174 +/- ##
==========================================
+ Coverage 94.81% 98.12% +3.31%
==========================================
Files 144 144
Lines 10836 10847 +11
==========================================
+ Hits 10274 10644 +370
+ Misses 562 203 -359
Continue to review full report at Codecov.
|
elif base_name == "ControlledQubitUnitary": | ||
if wire in op.control_wires: | ||
return self.charset.CONTROL | ||
|
||
representation = RepresentationResolver._format_controlled_qubit_unitary( | ||
op, "U", self.unitary_matrix_cache | ||
) |
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.
Nice :)
# Saving for the circuit drawer | ||
self.control_wires = control_wires | ||
self.U = U |
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.
If these attributes are required by the circuit drawer, we should probably document it somewhere.
Are there any other operations that are missing these attributes?
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.
Are there any other operations that are missing these attributes?
They seem to be specific to ControlledQubitUnitary
(at least with the current structure of PennyLane). Storing these two are required because:
control_wires
is merged intoself.wires
with the target wires at a later stage;- the unitary is expanded to the entire subsystem using identities on the control qubits and only the expanded unitary is stored in
self.data
.
Therefore, introducing self.control_wires
and self.U
provides a convenient way to gather both the control wires and the unitary.
We could actually consider separating control and target wires natively for other more defined operations, as it can be convenient to have the separation be available easily.
If these attributes are required by the circuit drawer, we should probably document it somewhere.
Indeed, at the moment they are well used by the circuit drawer, but it can easily be that other features will also use them. Any thoughts on where they could be documented? For attributes of operations, we don't seem to have any specific docstrings/docs.
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.
Any thoughts on where they could be documented?
This is a good question which I don't think has a good answer 🤔
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.
Great!
Context:
We consider drawing a
ControlledQubitUnitary
using the circuit drawer, which raises an error due to having no special rule for drawing this operation.Description of the Change:
Adds a rule for rendering the matrix and the operation.
Benefits:
Draws
ControlledQubitUnitary
instances.Possible Drawbacks:
N/A
Related GitHub Issues:
Closes #1171