Remove use of BackendProperties (BackendV1) in transpiler pipeline
#13722
Conversation
|
One or more of the following people are relevant to this code:
|
BackendProperties (BackendV1) in transpiler pipeline
Pull Request Test Coverage Report for Build 13388802659Details
💛 - Coveralls |
…ests that depend on BackendV1.
f0958f1 to
25bf326
Compare
| def test_no_backend_properties(self): | ||
| """Test we raise at runtime if no properties are provided with a coupling graph.""" | ||
| qc = QuantumCircuit(2) | ||
| empty_pass = VF2PostLayout( | ||
| coupling_map=CouplingMap([(0, 1), (1, 2)]), strict_direction=False | ||
| ) | ||
| with self.assertRaises(TranspilerError): | ||
| empty_pass.run(circuit_to_dag(qc)) |
There was a problem hiding this comment.
Isn't this test still valid without backend properties? Shouldn't the pass fail if we provide a coupling map with no error rates to use?
There was a problem hiding this comment.
I thought about removing coupling_map from the VF2PostLayout pass because of this test, because it really cannot be used without backend properties. Then I went for not erroring when only a coupling map is provided, I think I checked that it simply didn't return a solution. I am now standing somewhere between the two, the only advantage of still accepting the argument I guess is some kind of compatibility angle?
There was a problem hiding this comment.
Yeah it'd mostly be a compatibility question. From a practical PoV VF2Layout works with just a coupling map and for the error heuristic it uses node degree in the connectivity graph as a proxy for error (so a more connected node is avoided). For VF2PostLayout that is unlikely to have an impact because most coupling maps are regular lattices so the degrees won't differ that much.
The one case I could see it making a difference is like if the interaction graph of the post routing circuit was a line you could maybe snake the line over boundary nodes in the lattice that have a smaller degree. Something like:
im graph:
0 - 1 - 2 - 3 -4
cmap:
0 - 1 - 2 - 3
| | | |
4 - 5 - 6 - 7
| | | |
8 - 9 - 10 -11
if you used the node degree the mapping would favor putting the line that maximized the number of nodes on 0, 3, 8, and 11 in the cmap. Whether this is good behavior for VF2PostLayout is debatable though.
There was a problem hiding this comment.
Hmm ok, even though that use case doesn't come up much in practice now, I think the most "behavior preserving" path regarding this removal is erroring on the coupling map.
…1-props-transpiler
…used if backend properties were present. After the removal of the latter, the coupling_map would always be overwritten by the target.
|
After a bit of back and forth between the different removal alternatives in |
| "A target must be specified or a coupling map and properties must be provided" | ||
| ) | ||
| if self.target is None: | ||
| raise TranspilerError("A target must be specified or a coupling map must be provided") |
There was a problem hiding this comment.
This threw me for a second until I remember these passes should be instantiable even if the required target isn't specified.
There was a problem hiding this comment.
Yes I also spent a long time thinking if I should make it a required argument.
Summary
This PR removes the following uses of
BackendPropertiesdeprecated in #13719:as well as the previously deprecated input arguments of
transpile,generate_preset_pass_manager.Details and comments
Currently on hold as it contains the changes from #13706, should be rebased after it's merged.
Contributes to #13708.