Cut graphs can be fragmented into subgraphs and communication graph #2153
Conversation
…ithub.com:PennyLaneAI/pennylane into sc-13390-a-user-can-convert-a-tape-to-an-accurate
…stateprep conversion logic
…ithub.com:PennyLaneAI/pennylane into sc-13390-a-user-can-convert-a-tape-to-an-accurate
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
anthayes92
changed the title
…ithub.com:PennyLaneAI/pennylane into sc-14127-a-user-can-fragment-a-circuit-graph-into
|
Hello. You may have forgotten to update the changelog!
|
…ithub.com:PennyLaneAI/pennylane into sc-14127-a-user-can-fragment-a-circuit-graph-into
There was a problem hiding this comment.
Thanks @anthayes92! Great work! Have left some suggestions.
pennylane/transforms/qcut.py
Outdated
|
|
||
| Args: | ||
| graph (MultiDiGraph): directed multigraph containing measure and prepare | ||
| nodes at cut locations |
There was a problem hiding this comment.
| nodes at cut locations | |
| nodes at cut locations |
| sub_1_expected_edges, | ||
| sub_2_expected_edges, | ||
| sub_3_expected_edges, | ||
| ] |
There was a problem hiding this comment.
Looks like a pain to code up!
| compare_fragment_nodes(list(subgraph.nodes(data=True)), expected_n) | ||
|
|
||
| for subgraph, expected_e in zip(subgraphs, expected_edges): | ||
| compare_fragment_edges(list(subgraph.edges(data=True)), expected_e) |
There was a problem hiding this comment.
These tests are great! I'd like to consider having a few more "integration level" tests with a variety of starting configurations. But perhaps this is best left until once we have graph_to_tape(). 🤔
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
pennylane/transforms/qcut.py
Outdated
| if isinstance(node1, MeasureNode): | ||
| assert isinstance(node2, PrepareNode) | ||
| cut_edges.append((node1, node2)) | ||
| graph.remove_edge(node1, node2) |
There was a problem hiding this comment.
I would make a (deep) copy of graph beforehand to avoid of risk of unwanted side effects, as long as the graph is not too large, which is the case for us right? Adding edges back in could change the internal order of how things are stored, which is always a risk.
pennylane/transforms/qcut.py
Outdated
| if isinstance(node1, MeasureNode): | ||
| assert isinstance(node2, PrepareNode) | ||
| cut_edges.append((node1, node2)) | ||
| graph.remove_edge(node1, node2) |
There was a problem hiding this comment.
If things go multiqubit, the wire should also be stored in the data of cut_edges right?
pennylane/transforms/qcut.py
Outdated
| if subgraph.has_node(node2): | ||
| end_fragment = i | ||
|
|
||
| communication_graph.add_edge(start_fragment, end_fragment, pair=(node1, node2)) |
There was a problem hiding this comment.
I would say even without multiquibit Measure/Prepare, it should be helpful if we store the wire index in the edge as well.
There was a problem hiding this comment.
Agreed, added thanks!
| subgraphs = tuple(graph.subgraph(n) for n in subgraph_nodes) | ||
|
|
||
| communication_graph = MultiDiGraph() | ||
| communication_graph.add_nodes_from(range(len(subgraphs))) |
There was a problem hiding this comment.
Not a real suggestion but just wanted to throw it out there. Instead of constructing a graph with integer nodes, if we were to stick the subgraphs into the nodes' data attributes for the communication_graph (or, if possible, directly use subgraphs as nodes), we would only need to return the communication_graph which carries the subgraphs inside it. But again I would imagine it to be quite difficult to use with nx's default UI.
There was a problem hiding this comment.
We also discussed this possibility, we came to the conclusion that there were fairly equal pros and cons so it came down to that fact that it's already coded up, would interfere with the current tests and we're pushing to implement quickly.
Happy to be persuaded otherwise though 😁
There was a problem hiding this comment.
Thanks @anthayes92, approved! Although I'd be curious to quickly resolve the copy/deepcopy question first.
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
anthayes92
deleted the
sc-14127-a-user-can-fragment-a-circuit-graph-into
branch
Context:
The circuit cutting pipeline consists of manually placing a wirecut operator during construction of a circuit, converting this circuit to a graph and then replacing the wirecut nodes with measure and prepare nodes. At this stage the graph is ready to be fragmented into subgraphs. A communication graph is also returned, this is a graph where each node represents a fragment and edges denote the flow of qubits between fragments. This is required for reconstruction in following steps.
Description of the Change:
A method for fragmenting cut graphs has been added
Benefits:
Facilitates the circuit cutting compiler