This seems like an unusual choice to do quite as literal translation of the Python source as this. Feels like we're making life a lot harder for ourselves - it seems like the flow of this function is converting Rust-native objects in `other` into their Python equivalents, using a Python-space mapping to convert them to `self`-based objects, then having to extract everything back out of Python-space again to get the Rust-native types in `self`.
I'd have thought we'd be able to build up qubit_map: Vec<Qubit> where the indices are the qubits in other and the values are the corresponding Qubit in self, and never need to involve Python space beyond converting the function arguments into self's Qubit objects (like qubits.iter()?.map(|bit| self.qubits.get(bit?)).collect::<PyResult<Vec<Qubit>>>()? or something).
Let's not change it now, but we definitely should bring this more Rust-native in follow-ups; if nothing else, it'll make the file easier to read.
Following #12550, DAGCircuit.compose is a fairly literal translation of the original Python code into Rust. This includes using Python objects for the bit comparisons, argument lists and hashmaps, in places where we already have Rust-native types that can be used. We should rewrite the function to use the Rust-native types, and overhead the Python-space overhead of the conversions.
Also, as mentioned in https://github.com/Qiskit/qiskit/pull/12550#discussion_r1717605654, the version at the merge point was not a translation of the optimised version made in #12826. Moving to the Rust-native types should largely fix that concern automatically, though.
I'd have thought we'd be able to build up
qubit_map: Vec<Qubit>where the indices are the qubits inotherand the values are the correspondingQubitinself, and never need to involve Python space beyond converting the function arguments intoself'sQubitobjects (likequbits.iter()?.map(|bit| self.qubits.get(bit?)).collect::<PyResult<Vec<Qubit>>>()?or something).Let's not change it now, but we definitely should bring this more Rust-native in follow-ups; if nothing else, it'll make the file easier to read.
_Originally posted by @jakelishman in https://github.com/Qiskit/qiskit/pull/12550#discussion_r1717323668_
Following #12550,
DAGCircuit.composeis a fairly literal translation of the original Python code into Rust. This includes using Python objects for the bit comparisons, argument lists and hashmaps, in places where we already have Rust-native types that can be used. We should rewrite the function to use the Rust-native types, and overhead the Python-space overhead of the conversions.Also, as mentioned in https://github.com/Qiskit/qiskit/pull/12550#discussion_r1717605654, the version at the merge point was not a translation of the optimised version made in #12826. Moving to the Rust-native types should largely fix that concern automatically, though.