Make SabreSwap account for clbits in predecessors

[jakelishman]

Summary

Previously, SabreSwap only checked that it was valid to add a gate back
to the DAG by ensuring that all its qubit predecessors had already been
added to the DAG. It did not check for clbit successors. This meant
that measurements and (potentially, unverified) classically conditioned
gates could be re-ordered out from the correct topological ordering.

The most notable effect of this was that measurements writing to the
same bit could be re-ordered, changing the output of the circuit.

See gh-7950 for more detail.

Details and comments

Fix #7950. I also believe this to be the cause of the sporadic randomised test failures (see an example in the linked issue).

Ali (@ajavadia) - this might be something you ought to look at, since I don't understand SabreSwap well.

[qiskit-bot]

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 2191575209

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• 3 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.005%) to 83.964%

---

Files with Coverage Reduction	New Missed Lines	%
qiskit/pulse/library/waveform.py	3	89.36%

Totals
Change from base Build 2191310058:	-0.005%
Covered Lines:	54235
Relevant Lines:	64593

---

💛 - Coveralls

[kevinhartman]

I approve of this bugfix, though I was sitting next to Jake while he crafted it 😁 (and the same bug actually exists in TOQM too!).

[mtreinish]

This LGTM it makes sense to me that we should be respecting all edges in the dag when looking for successors because there is a ordering dependency there and only looking at the quantum successors is giving us an incomplete picture. Just one inline comment on one small improvement

[ajavadia]

looks good. I wonder if this fixes #7707

[mtreinish]

looks good. I wonder if this fixes #7707

I ran the reproduce script in the issue with this applied locally and it returned and didn't get stuck in an infinite loop.

[jakelishman]

The issue with commit b5b79d2 was that DAGCircuit.successors and DAGCircuit.edges have different behaviours when dealing with parallel edges. The version with edges (which does only do the output edges, so SabreSwap._successors is correctly named) will repeat the successor for each parallel edges, whereas successors only shows each one once. Using successors meant that things like measurements or barriers (which may well have the same qubit and clbit going in) were handled differently in the short form I'd written.

The PR now uses edges again, so it all works, but this doesn't fix #7707; Matt had seen improvements with the bust version of this PR because it caused several gates to get dropped, preventing the pass from reaching the loop state.