Oxidize CheckGateDirection

[eliarbel]

Summary

This PR ports the CheckGateDirection analysis pass to Rust.

Details and comments

This PR includes:

• Until we have a Rust version of CouplingMap, the Python code converts the given coupling map to a set of edge pairs and pass it as PySet to the Rust code.
• The pass relies on the DAGCircuit.qubits registry for mapping instruction qubit args to qubit indices when checking gate direction against either a coupling map edge set or a target.
• An unrelated change in dag_circuit.rs: making a pure Rust-friendly version of two_qubits_op

Close #12256

[coveralls]

Pull Request Test Coverage Report for Build 10757805126

Details

• 84 of 89 (94.38%) changed or added relevant lines in 5 files are covered.
• 10 unchanged lines in 3 files lost coverage.
• Overall coverage decreased (-0.001%) to 89.151%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/circuit/src/dag_circuit.rs	12	13	92.31%
crates/accelerate/src/gate_direction.rs	69	73	94.52%

Files with Coverage Reduction	New Missed Lines	%
qiskit/synthesis/two_qubit/xx_decompose/decomposer.py	1	95.42%
crates/qasm2/src/lex.rs	3	92.98%
crates/qasm2/src/parse.rs	6	97.15%

Totals
Change from base Build 10746759838:	-0.001%
Covered Lines:	72924
Relevant Lines:	81798

---

💛 - Coveralls

[eliarbel]

Some benchmark data, using circuits generated like this:

tqc = generate_preset_pass_manager(1, coupling_map=CouplingMap.from_heavy_hex(7))
    .run(qc)

and this:

tqc = generate_preset_pass_manager(1, GenericBackendV2(qubits))
    .run(qc)

where:

 qubits = 100
 qc = random_circuit(qubits, qubits, seed=12234)

(note that DAGCircuit.count_ops() gives different stats for the coupling-based vs. target-bases circuits but this is irrelevant for this check)

And results are measured like this:

%%timeit
check_direction_pass.run(dag)

Implementation	CouplingMap	Target
Python	1.48 s ± 46.5 ms	641 ms ± 11.4 ms
Rust	130 ms ± 7.02 ms	131 ms ± 5.23

So ~5-11X improvement in this check by moving to Rust. Note that CouplingMap is (still) in Python while Target is in Rust, which may explain the run-time differences in Python and the speed-up differences between the target vs coupling map cases.

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core
• @kevinhartman
• @mtreinish

[ElePT]

It might be helpful for benchmarking to generate a target with the same coupling map as the "coupling_map" case, as GenericBackendV2 generates a target with all-to-all connectivity by default. You could do this by running:

tqc = generate_preset_pass_manager(1, GenericBackendV2(qubits, coupling_map=CouplingMap.from_heavy_hex(7))).run(qc)

[eliarbel]

Thanks @ElePT . There are 2 paths for this pass: one is based on coupling map and one on target. So I think it doesn't really matter for this benchmarking to have the same coupling constraints, as long as we use the same constraints in each path separately both in Python and Rust .

Anyway, FWIW, here is another comparison, this time using the same coupling constraints, basis gates and traspiler seed in all 4 runs (python/rust X coupling/target). DAGCircuit.count_ops() is now the same for all: {'rz': 27177, 'sx': 6193, 'cx': 161650, 'x': 70}:

Implementation	CouplingMap	Target
Python	3.2 s ± 537 ms	3.21 s ± 104 ms
Rust	263 ms ± 26.2	213 ms ± 4.2 ms

I see that I need to resolve conflicts in dag_circuit.rs now that Jake's PR #13033 is merged; Will do soon

[mtreinish]

This looks good, thanks for doing this. I left a few inline comments, but at a high level this has everything needed.

[eliarbel]

Thanks @mtreinish for the review and very helpful comments!

Here is an updated benchmark result. The Target path became slower for some reason. I didn't explore why, I'd defer this for a later phase.

Implementation	CouplingMap	Target
Python	3.19 s ± 450	3.18 s ± 221
Rust	137 ms ± 6.81 ms	567 ms ± 15.6

[mtreinish]

This is looking good, I think it's basically ready now thanks for making the updates. I just have a few small inline comments. But after those are addressed I think we're good to merge this.

[mtreinish]

This LGTM now, thanks for making all the updates.