NoiseAdaptiveLayout to consider inconsistencies in backend #10859
Conversation
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 6227804167
💛 - Coveralls |
| """NoiseAdaptiveLayout initializer. | ||
|
|
||
| Args: | ||
| backend_prop (Union[BackendProperties, Target]): backend properties object | ||
| coupling_map (CouplingMap): Optional. To filter the backend_prop qubits/gates. |
There was a problem hiding this comment.
I would add a note here that this argument is ignored if backend_prop is a Target object and this is only needed to deal with inconsistencies in BackendProperties objects.
There was a problem hiding this comment.
Extending it in 29bc007
| # inconsistency internally). For the non-target path, this is a possible solution. | ||
| # See https://github.com/Qiskit/qiskit/issues/7677 | ||
| self.backend_prop.gates = filter( | ||
| lambda ginfo: ginfo.gate == "cx" |
There was a problem hiding this comment.
What about backends that use different entangling gates besides cx?
There was a problem hiding this comment.
i think the pass is already cx-specific
. But it is easy to fix by just removing that condition, in 6d9c214| # See https://github.com/Qiskit/qiskit/issues/7677 | ||
| self.backend_prop.gates = filter( | ||
| lambda ginfo: ginfo.gate == "cx" | ||
| and tuple(ginfo.qubits) in coupling_map.graph.edge_list(), |
There was a problem hiding this comment.
For performance it might make sense to collect the edge_list into a set(). Event though edge_list() returns a rust container it's a sequence type based on a rust Vec, so there is an O(n^2) overhead here because we're iterating over all edges for each edge in the backend.
There was a problem hiding this comment.
Good call! easy optimization 1e52ac8
| and tuple(ginfo.qubits) in coupling_map.graph.edge_list(), | ||
| backend_prop.gates, | ||
| ) | ||
| self.backend_prop.qubits = backend_prop.qubits[ |
There was a problem hiding this comment.
Does this modify the BackendProperties object for other passes that are using it, do we need a copy here before modifying? For the default preset pass managers, I don't think it really matters because the only things after this that access the backend properties are potentially VF2PostLayout, Optimize1qGatesDecomposition, and UnitarySynthesis (I might have missed one from memory) and those only look up error rates in the properties.
There was a problem hiding this comment.
Good point. I made the copy only under this condition, so I dont penalize the most general case. Done in 1a375df
|
|
||
| result = transpile(qc, backend, layout_method="noise_adaptive", optimization_level=level) | ||
|
|
||
| self.assertIsInstance(result, QuantumCircuit) |
There was a problem hiding this comment.
Do you want to assert anything about the properties of the output circuit, like number of qubits?
There was a problem hiding this comment.
sure. I did the same with test_no_coupling_map in this very same file in 960c20c
…h 'main' of github.com:Qiskit/qiskit-terra into fixes/7677/1
Fixes #7677
The pass
NoiseAdaptiveLayoutnow takes a CouplingMap as an optional argument. This is used by the plugin to control on configuration/properties inconsistency, like in the case of FakeMelbourne