Conversation
…nteraction-graph-a-property-of-the-Circuit-expressed-in-terms-of-simple-types
…nteraction-graph-a-property-of-the-Circuit-expressed-in-terms-of-simple-types
…graph in map method
elenbaasc
left a comment
There was a problem hiding this comment.
Thanks for the effort!
I think we should pass the circuit/self to the Circuit.map method, such that the passes themselves can pick up the interaction graph where needed.
| def _ir_to_networkx(ir: IR) -> nx.Graph: | ||
| """Build an undirected interaction graph representation of the IR. |
There was a problem hiding this comment.
Wouldn't use package specific name in the functionality.
Also is it 'undirected'? Or should it be unidirected or bi-directional?
| def _ir_to_networkx(ir: IR) -> nx.Graph: | |
| """Build an undirected interaction graph representation of the IR. | |
| def _ir_to_graph(ir: IR) -> nx.Graph: | |
| """Build an undirected interaction graph representation of the IR. |
| raise ValueError(msg) | ||
|
|
||
| circuit_graph = self._ir_to_interaction_graph(ir) | ||
| if interaction_graph is None: |
There was a problem hiding this comment.
| if interaction_graph is None: | |
| if not interaction_graph: |
| return interaction_graph | ||
|
|
||
| @staticmethod | ||
| def _interaction_graph_to_networkx(edges: dict[tuple[int, int], int]) -> nx.Graph: |
There was a problem hiding this comment.
| def _interaction_graph_to_networkx(edges: dict[tuple[int, int], int]) -> nx.Graph: | |
| def _convert_interaction_graph(edges: dict[tuple[int, int], int]) -> nx.Graph: |
| self, | ||
| ir: IR, | ||
| qubit_register_size: int, | ||
| interaction_graph: dict[tuple[int, int], int] | None = None, |
There was a problem hiding this comment.
Why is the interaction graph an input argument to the map method? If it is a property of the circuit, then just provide the circuit as input (instead of just the IR) and take the interaction graph from there...
And the passes that don't required it (e.g. MIP mapper), don't take it from the circuit... am I missing something?
|
|
||
| InstructionCount = dict[str, int] | ||
| MeasurementToBitMap = defaultdict[str, list[int]] | ||
|
|
There was a problem hiding this comment.
| InteractionGraph = dict[tuple[int, int], int] |
| ): | ||
| waiting_time = schedulable_data.data["timing_constraints"][0].rel_time | ||
| assert waiting_time == -1.0 * expected_waiting_cycle * CYCLE_TIME | ||
| assert waiting_time == -1.0 * expected_waiting_cycle * CYCLE_TIME # noqa: RUF069 |
There was a problem hiding this comment.
Same here as previous comment.
| used_interaction_graph = {"called": False} | ||
| used_ir = {"called": False} | ||
|
|
||
| def fake_interaction_graph_to_networkx(_: Any) -> Any: |
There was a problem hiding this comment.
fake -> mock
| def fake_interaction_graph_to_networkx(_: Any) -> Any: | |
| def mock_interaction_graph_to_networkx(_: Any) -> Any: |
| CNOT q[0], q[1] | ||
| CNOT q[0], q[2] | ||
| CNOT q[1], q[2] | ||
| """ |
There was a problem hiding this comment.
Add a CNOT q[1], q[0], and adjust the interaction_graph
| if interaction_graph is not None: | ||
| msg = ( | ||
| "The MIPMapper does not effectively use the interaction graph, thus this argument should be set to None" | ||
| ) | ||
| warnings.warn(msg, stacklevel=2) |
There was a problem hiding this comment.
This should not be needed... the interaction graph should simply not be picked up by this mapper pass.
| def test_interaction_graph_warning(mapper1: MIPMapper, circuit1: Circuit) -> None: | ||
| msg = "The MIPMapper does not effectively use the interaction graph, thus this argument should be set to None" | ||
| with pytest.warns(Warning, match=msg): | ||
| mapper1.map(circuit1.ir, circuit1.qubit_register_size, interaction_graph=circuit1.interaction_graph) |
There was a problem hiding this comment.
If the above changes are made, then these tests don't need to be done.
No description provided.