Rework RemoveFinalMeasurements pass (bug fixes)

[kevinhartman]

Summary

Fixes several bugs with RemoveFinalMeasurements and QuantumCircuit.remove_final_measurements discussed in #7089.

Details and comments

RemoveFinalMeasurements

• Fixes determination of final operations (barriers and measures), which previously considered only nodes immediately preceding an output node.
• Fixes determination of final operations which could wrongly consider a barrier to be final, even if other circuit operations followed it.
• Fixes multi-bit classical register removal, where classical registers were not removed even if other bits were idle, unless a final measure was done into each and every bit.
• Fixes handling of classical registers with shared underlying bits which was not previously handled.
• Fixes handling of classical bits without registers.
• Fixes register deletion where registers were deleted from DAGCircuit manually without updating DAGCircuit.{clbits, _wires, input_map, output_map, _multi_graph}, which resulted in an invalid DAG.

DAGCircuit

• Adds method remove_cregs to remove classical registers from the circuit.
• Adds method remove_clbits to remove idle classical bits as well as any registers that reference them.

QuantumCircuit

• Fixes clbit copy from DAG in remove_final_measurements.

Sample output

Before pass

After pass

[kevinhartman]

Todo

• Add release notes.
• Performance validation.
• Fixes for QuantumCircuit.remove_final_measurements which wrongly clears all clbits.
• Unit testing for new methods on DAGCircuit.
• Unit testing for QuantumCircuit.remove_final_measurements.
• Run code style reformatter.
• Quick change needed: recalculation of clbit indices in QuantumCircuit should also map lone clbits to empty register list.

[kevinhartman]

Regarding performance, results of 100000 runs of benchmark PassBenchmarks.time_remove_final_measurements with 20 qubits and depth 1024:

Release 0.18.3: 780.9654858s
Current main: 6.392182523s
PR branch: 7.624031293s

This is about what I'd expect, since the current version of main doesn't account for sequences of final measures and barriers, or properly update DAGCircuit. The huge improvement from 0.18.3 to main likely comes from #7039, and is preserved.

[kdk]

Thanks for the updates. One question in the Clbit removal behavior of RemoveFinalMeasurements.

[jakelishman]

I like this - it seems quite a bit more logical to me than what we had before, and the behaviour seems much more consistent.

I left a few (rather more than I thought I had...) comments in line. A lot of them are about magic Sphinx incantations, and a few comments just on stuff I like. I don't think there were any serious changes at all, but there were a couple of places where comments/documentation had got out-of-sync with code.

[jakelishman]

This looks good to accept to me. Looks like there's a merge conflict - did you base this branch off a main before #7039? If so, we can probably just merge with strategy "ours". You can do the merge locally and push - it's fine to have merge commits in PRs here, because we squash all PRs into a single change before applying, so the history will be fine.

[kevinhartman]

did you base this branch off a main before #7039?

I believe that had already gone in prior to me starting this branch, so maybe some other refactor touched that file. I should have a chance to do the merge shorty!

[jakelishman]

Thanks for all the prompt changes on this. I'm happy to accept it as-is. Kevin was already happy with it (as best as I can tell), so I'll merge now without waiting for him to re-comment because I anticipate a nasty CI crunch on Monday and Tuesday even in the best-case scenario.