Fix single clbit display in condition for all 3 circuit drawers

[enavarro51]

Summary

Fixes #7248
Fixes #7284

Details and comments

This PR fixes an issue with displaying a conditional gate using a registerless Clbit in all 3 circuit drawers. In addition several inconsistencies in the drawers related to Clbits in a condition have been fixed. The rules for display are now,

• If the condition is a registerless Clbit, the bit will be shown as an open or closed circle based on being F or T. No value is displayed below the condition.
• If the condition is a register Clbit and cregbundle is off, it displays the same as a registerless Clbit.
• If a register Clbit and cregbundle is on, the bit name and the value are displayed below the bit.
• If the condition is a whole register, everything works as it did before.

I am marking this as a WIP since there is still a problem, noted in #7284, with _get_layered_instructions that prevents some of the tests that need to be added to this PR from working. So once #7284 has been addressed, the additional tests can be added.

• Add tests of multiple gates with conditions and measures using Clbits.

[coveralls]

Pull Request Test Coverage Report for Build 1508813903

• 151 of 169 (89.35%) changed or added relevant lines in 4 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.03%) to 82.841%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/visualization/text.py	28	29	96.55%
qiskit/visualization/latex.py	69	71	97.18%
qiskit/visualization/matplotlib.py	12	27	44.44%

Totals
Change from base Build 1508812883:	0.03%
Covered Lines:	50108
Relevant Lines:	60487

---

💛 - Coveralls

[jakelishman]

Do you want review on this now, or would you prefer to wait til a solution to #7284? I'm not sure I'm going to have much time for the next week or so, at any rate, sorry.

[enavarro51]

It should wait. The tests that need to be added for this won't work until #7284 is done. Thanks.

[enavarro51]

This PR has now been expanded to fix these 3 areas,

1. Now all drawers display conditions using registerless bits.
2. Now all drawers display gates with conditions on the same bit as a measure to the right of the measure if the gate follows the measure in the gate list. Previously this only worked if condition and measure were both on registers.
3. This also fixes a 'latex' drawer only problem, where the measure would not target the correct bit if a bit instead of a register was used and cregbundle was False.

Additional tests and reno to follow shortly.

[javabster]

Hi Ed, this is looking good so far! Having done some manual testing of your branch I have a few comments:

1. I know we spoke already about the use of F/T for the single bit case (your 2nd bullet option) but now that I'm looking at the different options together I think actually keeping the hex format is better for the sake of consistency with the other labels (sorry 😅 )

2. Can you add a release note please?

3. When testing manually for each of the cases mentioned in your description they pretty much all worked except I found that for the following example an error is thrown for the text drawer (mpl and latex are fine):

bits = [Qubit(), Qubit(), Clbit(), Clbit()]
crx = ClassicalRegister(3)
circ = QuantumCircuit(bits, crx)

circ.x(0).c_if(crx[1], 0)
circ.measure(0, bits[2])

circ.draw('latex')

ValueError                                Traceback (most recent call last)
~/opt/anaconda3/envs/qiskit-dev/lib/python3.7/site-packages/IPython/core/formatters.py in __call__(self, obj)
    700                 type_pprinters=self.type_printers,
    701                 deferred_pprinters=self.deferred_printers)
--> 702             printer.pretty(obj)
    703             printer.flush()
    704             return stream.getvalue()

~/opt/anaconda3/envs/qiskit-dev/lib/python3.7/site-packages/IPython/lib/pretty.py in pretty(self, obj)
    392                         if cls is not object \
    393                                 and callable(cls.__dict__.get('__repr__')):
--> 394                             return _repr_pprint(obj, self, cycle)
    395 
    396             return _default_pprint(obj, self, cycle)

~/opt/anaconda3/envs/qiskit-dev/lib/python3.7/site-packages/IPython/lib/pretty.py in _repr_pprint(obj, p, cycle)
    698     """A pprint that just redirects to the normal repr function."""
    699     # Find newlines and replace them with p.break_()
--> 700     output = repr(obj)
    701     lines = output.splitlines()
    702     with p.group():

~/ibm/quantum/qiskit-terra/qiskit/visualization/text.py in __repr__(self)
    655 
    656     def __repr__(self):
--> 657         return self.single_string()
    658 
    659     def single_string(self):

~/ibm/quantum/qiskit-terra/qiskit/visualization/text.py in single_string(self)
    663         """
    664         try:
--> 665             return "\n".join(self.lines()).encode(self.encoding).decode(self.encoding)
    666         except (UnicodeEncodeError, UnicodeDecodeError):
    667             warn(

~/ibm/quantum/qiskit-terra/qiskit/visualization/text.py in lines(self, line_length)
    707 
    708         try:
--> 709             layers = self.build_layers()
    710         except TextDrawerCregBundle:
    711             self.cregbundle = False

~/ibm/quantum/qiskit-terra/qiskit/visualization/text.py in build_layers(self)
   1136 
   1137             for node in node_layer:
-> 1138                 layer, current_connections, connection_label = self._node_to_gate(node, layer)
   1139 
   1140                 layer.connections.append((connection_label, current_connections))

~/ibm/quantum/qiskit-terra/qiskit/visualization/text.py in _node_to_gate(self, node, layer)
   1024                 layer.set_clbit(
   1025                     node.cargs[0],
-> 1026                     MeasureTo(str(self.bit_locations[node.cargs[0]]["index"])),
   1027                 )
   1028             else:

~/ibm/quantum/qiskit-terra/qiskit/visualization/text.py in set_clbit(self, clbit, element)
   1208         """
   1209         if self.cregbundle:
-> 1210             self.clbit_layer[self.clbits.index(self._clbit_locations[clbit]["register"])] = element
   1211         else:
   1212             self.clbit_layer[self.clbits.index(clbit)] = element

ValueError: None is not in list

[enavarro51]

Thanks, @javabster.

1. I would still argue for the T/F because in Instruction.c_if, the value is changed to a bool if it's a Clbit. It also makes it clear that it's only T/F or 0/1, whereas 0x1 could imply other values are possible. Will defer, of course. 🙂
2. On the way.
3. YABB. (yet another bit bug). Will be fixed in next commit.

I'll also be adding 3 text tests and 4 latex ones.

[javabster]

This is looking good! Implementation looks fine by me, testing manually works fine and binder tests all pass.

The only thing I'm having second thoughts on is whether we should be removing the labels for single bit registers - is it intuitive enough that an open circle equals 0 and a closed circle equals 1? I'm on the fence 🤔 Perhaps @1ucian0 has thoughts?

[enavarro51]

My thinking on it is that the less clutter the better and the numbering is redundant. We already use only the open and closed circles for controls, and assume users know that open is a ctrl_state=0 and closed is ctrl_state=1.

[jakelishman]

This all looks entirely reasonable to me. I'm not an expert in the visualisation code, but the changes all appear sensible to me, and the output looks good!

Thanks a lot for the continued management of the drawers!

[enavarro51]

Thanks, Abby and Jake.