Cqt 455 implement analyzer pass in open squirrel#682
Conversation
- Implement Analyzer ABC in opensquirrel/passes/analyzer/general_analyzer.py - Implement CircuitAnalyzer with size, IG, GDG, and density metrics - Update Circuit.analyze() return type to dict[str, Any] - Add 14 tests in tests/passes/analyzer/ - Update CHANGELOG
| - `CNOT2CZDecomposer` decomposer pass | ||
| - `RoutingChecker` routing pass | ||
| - Restore SGMQ notation for barrier groups in cQASMv1 Exporter | ||
| - `CircuitAnalyzer` analyzer pass for computing structural circuit metrics (size, interaction graph, gate dependency graph, density) |
There was a problem hiding this comment.
This would need to be added to the #Added section at line 15 in the file.
| def test_size_metrics_on_empty_circuit(analyzer: CircuitAnalyzer, empty_circuit: Circuit) -> None: | ||
| result = analyzer.analyze(empty_circuit) | ||
| assert result["n_qubits"] == 3 | ||
| assert result["n_gates"] == 0 | ||
| assert result["n_two_qubit_gates"] == 0 | ||
| assert result["two_qubit_pct"] == pytest.approx(0.0, abs=1e-9) | ||
| assert result["depth"] == 0 | ||
|
|
||
|
|
||
| def test_size_metrics_on_ghz(analyzer: CircuitAnalyzer, ghz_circuit: Circuit) -> None: | ||
| result = analyzer.analyze(ghz_circuit) | ||
| assert result["n_qubits"] == 3 | ||
| assert result["n_gates"] == 3 | ||
| assert result["n_two_qubit_gates"] == 2 | ||
| assert result["two_qubit_pct"] == pytest.approx(2 / 3, abs=1e-3) | ||
| assert result["depth"] == 3 |
There was a problem hiding this comment.
Maybe you could group these two tests with using @pytest.mark.parametrize?
| def test_depth_with_parallel_gates(analyzer: CircuitAnalyzer, parallel_circuit: Circuit) -> None: | ||
| """Two independent gates can run in the same time-step, so depth is 1.""" | ||
| result = analyzer.analyze(parallel_circuit) | ||
| assert result["depth"] == 1 | ||
| assert result["n_gates"] == 2 | ||
|
|
||
|
|
||
| def test_depth_with_sequential_gates(analyzer: CircuitAnalyzer, sequential_circuit: Circuit) -> None: | ||
| """Gates that share qubits must serialise, so depth equals gate count.""" | ||
| result = analyzer.analyze(sequential_circuit) | ||
| assert result["depth"] == 3 | ||
| assert result["n_gates"] == 3 |
|
"Hey Chris, thanks for the tip! I ve grouped the size, depth, IG, and GDG tests using @pytest.mark.parametrize. I also modified the CHANGELOG.md, and made sure all the linter checks are green. Thanks for the review !" |
No thanks to me! It was Rares (@rares1609) who did the review 😄! Thanks for making the changes |
|
Oops , my bad! Massive thanks to Rares for the review 🙌 |
elenbaasc
left a comment
There was a problem hiding this comment.
Nice work!
Just a few minor comments, nothing too serious :)
|
|
||
| * Size: number of qubits, gates, two-qubit gates, two-qubit gate percentage, depth. | ||
| * Interaction graph (IG): metrics derived from the qubit interaction graph, | ||
| where nodes are qubits and edges are two-qubit gates. |
There was a problem hiding this comment.
| where nodes are qubits and edges are two-qubit gates. | |
| where nodes correspond to qubits and edges correspond to two-qubit gates. |
| for gate in gate_statements: | ||
| qubit_indices = list(gate.qubit_indices) |
There was a problem hiding this comment.
| for gate in gate_statements: | |
| qubit_indices = list(gate.qubit_indices) | |
| for gate_statement in gate_statements: | |
| qubit_indices = list(gate_statement.qubit_indices) |
| if not qubit_indices: | ||
| continue |
There was a problem hiding this comment.
This would be strange. All gates should have qubit indices...
| if not qubit_indices: | |
| continue |
| new_layer = max(layer[i] for i in qubit_indices) + 1 | ||
| for i in qubit_indices: | ||
| layer[i] = new_layer |
There was a problem hiding this comment.
| new_layer = max(layer[i] for i in qubit_indices) + 1 | |
| for i in qubit_indices: | |
| layer[i] = new_layer | |
| new_layer = max(layer[qubit_index] for qubit_index in qubit_indices) + 1 | |
| for qubit_index in qubit_indices: | |
| layer[qubit_index] = new_layer |
| empty: dict[str, Any] = { | ||
| "ig_avg_shortest_path": 0.0, | ||
| "ig_std_adjacency": 0.0, | ||
| "ig_diameter": 0, | ||
| "ig_central_dominance": 0.0, | ||
| "ig_avg_degree": 0.0, | ||
| "ig_n_maximal_cliques": 0, | ||
| "ig_clustering_coefficient": 0.0, | ||
| } | ||
|
|
||
| weighted_edges = circuit.interaction_graph | ||
| if not weighted_edges: | ||
| return empty |
There was a problem hiding this comment.
| empty: dict[str, Any] = { | |
| "ig_avg_shortest_path": 0.0, | |
| "ig_std_adjacency": 0.0, | |
| "ig_diameter": 0, | |
| "ig_central_dominance": 0.0, | |
| "ig_avg_degree": 0.0, | |
| "ig_n_maximal_cliques": 0, | |
| "ig_clustering_coefficient": 0.0, | |
| } | |
| weighted_edges = circuit.interaction_graph | |
| if not weighted_edges: | |
| return empty | |
| weighted_edges = circuit.interaction_graph | |
| if not weighted_edges: | |
| return { | |
| "ig_avg_shortest_path": 0.0, | |
| "ig_std_adjacency": 0.0, | |
| "ig_diameter": 0, | |
| "ig_central_dominance": 0.0, | |
| "ig_avg_degree": 0.0, | |
| "ig_n_maximal_cliques": 0, | |
| "ig_clustering_coefficient": 0.0, | |
| } |
| @staticmethod | ||
| def _build_gate_dependency_graph(gate_statements: list[Gate]) -> nx.DiGraph: | ||
| """Build a DAG where edge (i, j) means gate i must run before gate j.""" | ||
| gdg: nx.DiGraph = nx.DiGraph() |
There was a problem hiding this comment.
I prefer to write this out (also in other places).
| gdg: nx.DiGraph = nx.DiGraph() | |
| gate_dependency_graph: nx.DiGraph = nx.DiGraph() |
| if longest_from[successor] + 1 > longest_from[node]: | ||
| longest_from[node] = longest_from[successor] + 1 | ||
|
|
||
| return longest_to, longest_from |
There was a problem hiding this comment.
Why would you want them in this order (convention?)? Seems more intuitive to go from 'from' to 'to', and not from 'to' to 'from' 😅.
| mean_length = sum(path_lengths) / len(path_lengths) | ||
| variance = sum((x - mean_length) ** 2 for x in path_lengths) / len(path_lengths) |
There was a problem hiding this comment.
Surely there are built-in ways to determine the mean and variance of the path_lengths, right?
| for gate in gate_statements: | ||
| for qubit_index in gate.qubit_indices: |
There was a problem hiding this comment.
| for gate in gate_statements: | |
| for qubit_index in gate.qubit_indices: | |
| for gate_statement in gate_statements: | |
| for qubit_index in gate_statement.qubit_indices: |
| from opensquirrel import CircuitBuilder | ||
| from opensquirrel.circuit import Circuit |
There was a problem hiding this comment.
| from opensquirrel import CircuitBuilder | |
| from opensquirrel.circuit import Circuit | |
| from opensquirrel import CircuitBuilder, Circuit |
|
Hey Chris , thank you for the review ! |
Adds a new ''Analyzer'' pass type and the first concrete implementation, "'CircuitAnalyzer'', which computes structural metrics describing a quantum circuit. Metrics are organized into four categories :