-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Pauli X, Y, Z measurement instructions #9269
base: main
Are you sure you want to change the base?
Changes from 13 commits
43b702b
0f796bd
7c04baa
2bc2c92
882dad9
49c608e
125e5fc
de2df75
b3fd53e
d34b69b
0e50b05
8ec2965
6421275
7144f26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,9 +21,10 @@ | |
|
||
from qiskit.circuit.quantumregister import QuantumRegister, Qubit | ||
from qiskit.circuit.classicalregister import ClassicalRegister, Clbit | ||
from qiskit.circuit.commutation_checker import CommutationChecker | ||
from qiskit.circuit.measure import Measure | ||
from qiskit.dagcircuit.exceptions import DAGDependencyError | ||
from qiskit.dagcircuit.dagdepnode import DAGDepNode | ||
from qiskit.circuit.commutation_checker import CommutationChecker | ||
|
||
|
||
# ToDo: DagDependency needs to be refactored: | ||
|
@@ -382,8 +383,12 @@ def _create_op_node(self, operation, qargs, cargs): | |
Returns: | ||
DAGDepNode: the newly added node. | ||
""" | ||
directives = ["measure"] | ||
if not getattr(operation, "_directive", False) and operation.name not in directives: | ||
directives = [] | ||
if ( | ||
not getattr(operation, "_directive", False) | ||
and not isinstance(operation, Measure) | ||
and operation.name not in directives | ||
): | ||
Comment on lines
-385
to
+391
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After introducing the |
||
qindices_list = [] | ||
for elem in qargs: | ||
qindices_list.append(self.qubits.index(elem)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -189,7 +189,7 @@ def __init__( | |
# If there is any custom instruction that uses classical bits | ||
# then cregbundle is forced to be False. | ||
for node in itertools.chain.from_iterable(self._nodes): | ||
if node.cargs and node.op.name != "measure": | ||
if node.cargs and not isinstance(node.op, Measure): | ||
if cregbundle: | ||
warn( | ||
"Cregbundle set to False since an instruction needs to refer" | ||
|
@@ -300,7 +300,7 @@ def _get_image_depth(self): | |
self._nodes | ||
and self._nodes[0] | ||
and ( | ||
self._nodes[0][0].op.name == "measure" | ||
isinstance(self._nodes[0][0].op, Measure) | ||
or getattr(self._nodes[0][0].op, "condition", None) | ||
) | ||
): | ||
|
@@ -427,7 +427,7 @@ def _build_latex_array(self): | |
self._nodes | ||
and self._nodes[0] | ||
and ( | ||
self._nodes[0][0].op.name == "measure" | ||
isinstance(self._nodes[0][0].op, Measure) | ||
or getattr(self._nodes[0][0].op, "condition", None) | ||
) | ||
): | ||
|
@@ -579,7 +579,26 @@ def _build_symmetric_gate(self, op, gate_text, wire_list, col): | |
def _build_measure(self, node, col): | ||
"""Build a meter and the lines to the creg""" | ||
wire1 = self._wire_map[node.qargs[0]] | ||
self._latex[wire1][col] = "\\meter" | ||
if node.op.basis: | ||
# Modification of `\meter` in https://www.ctan.org/tex-archive/graphics/qcircuit | ||
# pylint: disable=invalid-name | ||
PAULI_METER = """ | ||
*=<1.8em,1.4em> | ||
{ | ||
\\xy ="j", | ||
"j"-<.778em,.322em>;{"j"+<.778em,-.322em> \\ellipse ur,_{}}, | ||
"j"-<0em,.4em>;p+<.5em,.9em> **\\dir{-}, | ||
"j"+<2.2em,2.2em>*{},"j"-<2.2em,2.2em>*{}, | ||
<-.5em,.4em> *\\txt{\\tiny{%s}} | ||
\\endxy | ||
} | ||
\\POS ="i", | ||
"i"+UR;"i"+UL **\\dir{-};"i"+DL **\\dir{-};"i"+DR **\\dir{-};"i"+UR **\\dir{-}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (On my phone so I can't select properly.) This looks like Perhaps we can use their There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah! Good catch, I have definitely derived it from their code. I did try to use their macro but it proved to be very challenging (it builds an image and you lose the reference points after wrapping it up) so I dropped down to base Not sure how to address this otherwise. I'll think about it, thanks for bringing this up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened a PR to contribute this piece of code to |
||
"i" \\qw | ||
""" | ||
self._latex[wire1][col] = PAULI_METER % node.op.basis.upper() | ||
else: | ||
self._latex[wire1][col] = "\\meter" | ||
|
||
idx_str = "" | ||
cond_offset = 1.5 if getattr(node.op, "condition", None) else 0.0 | ||
|
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.
It's possible that we could accept an inheritance relationship here, but I don't think it's for certain the correct way of doing things, over (for example) a set of flags defined
Instruction
orOperation
. This particular inheritance is definitely not safe, though -Measure
is, and needs to remain, a measure in the Z basis, soMeasureX
isn't an instance of it. There's also the false abstraction here that the base class is only abstract as far as a single-qubit Pauli-basis measurement, which isn't the most general type.Aside from those abstraction/false-hierarchy issues, another thing to consider with inheritance is that we would need to work out how external / further subclasses of
BaseMeasure
(or whatever) would be safely serialised/deserialised by QPY - this PR adds an inheritance structure to instructions that is deliberately avoided in our data model elsewhere. This has some wide-ranging implications that I'm too "on holiday" to think through properly right now (sorry).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.
I see 🤔 let's pick this up again once you come back then! Thanks for the detailed explanation