Add separable_circuits method to DAGCircuit class

[caleb-johnson]

Summary

There is currently no easy way to retrieve all of the disconnected subsets of qubits in a given circuit, even though this information is easily accessible in the graph representation of a quantum circuit.

Create an instance method, DAGCircuit.separable_circuits, that returns a list of DAGCircuits -- one for each set of disconnected qubits in the quantum circuit.

Closes #9459

[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

[CLAassistant]

All committers have signed the CLA.

[coveralls]

Pull Request Test Coverage Report for Build 5592777414

• 25 of 25 (100.0%) changed or added relevant lines in 1 file are covered.
• 28 unchanged lines in 3 files lost coverage.
• Overall coverage decreased (-0.02%) to 86.025%

---

Files with Coverage Reduction	New Missed Lines	%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	1	90.33%
crates/qasm2/src/lex.rs	3	91.39%
crates/qasm2/src/parse.rs	24	95.71%

Totals
Change from base Build 5592160564:	-0.02%
Covered Lines:	72400
Relevant Lines:	84162

---

💛 - Coveralls

[jakelishman]

This is definitely an interesting function, and I'm sure it'll be useful, especially as hardware develops that has sparse links between denser separate chips like IBM Heron. I had a couple of significant design questions in the comments, some about the mathematics and some about the intended usage - I think there might be some hidden complexity here.

I get this is WIP, but later on, don't forget we'll need tests and a release note.

[kdk]

Thanks @caleb-johnson , this looks good so far! A few comments below.

[jakelishman]

Thanks for this Caleb, and sorry for the slow review. The actual function itself looks like it's in good shape to me. Please could we add a test of the global_phase behaviour, since it's a defined part of the interface, and make sure that some tests of remove_idle_wires include testing explicit forms, especially with regard to the special handling that unconnected clbits get?

For a little bit further docs: could you comment somewhere in the docstring what the behaviour is for the separable DAGs with regard to clbits / classical registers, and make sure we're testing that?

[jakelishman]

I think this one introduces a completely disjoint change, right? It would be good to put that in a separate test case, so they both run, even if one fails.

Also, it'd be better to construct the compare1, compare2 and compare3 elements as a list of DAGCircuit, and then compare the two lists directly - that way we get more information on how many / which circuits fail, if there's a failure. I'm also a bit nervous that this test requires that the output has a particular order, but I don't think rustworkx.weakly_connect_components will guarantee that for us? It might be ok to leave for now, but if there's an easy way to make the test resistant to re-orderings of the list, that would be good.

[garrison]

I don't think rustworkx.weakly_connect_components will guarantee that for us?

I asked about rustworkx.connected_components on Qiskit Slack, and this is what Matthew said:

There isn't an explicit guarantee to the order, mostly because I don't think we want to commit to it being stable in case we want to change the internal implementation of the algorithm in the future.

In that case, it was easy to make the operation deterministic by calling sort or the result. Something similar may be possible here.

[jakelishman]

That's a good idea, thanks Jim. I think we could find a way to choose the "first" busy qubit in each returned DAG, and sort the final list based on that qubit's index into the parent DAG. Given the nature of the function, that should be a unique sort key, so should deterministically order the list.

Just to be clear: I think we only need to bother with that in the test; I don't think we need to commit to any particular order in the method itself, just like weakly_connected_components doesn't.

[jakelishman]

This looks great now Caleb, thanks. I'd still like to put in the output sorting/normalisation into the test if possible - let me know if you're working on it, or I can add it - but beyond that, this is ready to go.

[caleb-johnson]

This looks great now Caleb, thanks. I'd still like to put in the output sorting/normalisation into the test if possible - let me know if you're working on it, or I can add it - but beyond that, this is ready to go.

Thanks a lot for the help getting this in. Let me know if you'd like any last minute tweaks

[jakelishman]

I don't think this is quite correct for the case of remove_idle_qubits=True, because it's going to take indices from the cut-down DAGs rather than the big dags, so the indices won't be the same. I think you'll want to get

def sort_key(indices: dict[Qubit, int]):
    def sorter(dag):
        try:
            first_op = next(dag.topological_op_nodes())
        except StopIteration:
            return -1
        return min(indices[q] for q in first_op.qargs)
    return sorter

def test_separable_circuits(self):
    big_dag = ...
    indices = {bit: i for i, bit in enumerate(big_dag)}
    components = [...]
    test = sorted(big_dag.separable_circuits(), key=sort_key(indices))
    expected = sorted(components, key=sort_key(indices))
    self.assertEqual(test, expected)

(or something like this - I just wrote that cold into GitHub)

[caleb-johnson]

Thanks again, this worked as-is :)

[jakelishman]

This looks great Caleb, thanks for sticking with it, for the quick updates, and for indulging my paranoia about test outputs changing from underneath us!

[jakelishman]

I just quickly fixed the type-hint resolution problem in the test - no big deal.

[caleb-johnson]

This looks great Caleb, thanks for sticking with it, for the quick updates, and for indulging my paranoia about test outputs changing from underneath us!

I was a bit sloppy here picking this back up after so much time. Thanks to you for sticking it out as well, hah :D

[jakelishman]

(Don't worry, but there's no need to press "update branch" in 95% of cases when there's no merge conflict - the automerge queue will automatically do it when this PR joins the queue.)