-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update transpile()
and generate_preset_pass_manager
to convert loose input of constraints to a Target
with Target.from_configuration()
#12185
Conversation
…on to individual passes.
…s where num_qubits and/or basis_gates is None.
… and comment out failing cases (temporary).
I have comments on 1. and 4. :-) 4.: I think if we can schedule with the provided user-input, we should just schedule it. :-) This may be relevant for work flows where an user uses external compilers to generate a physical circuit and then just wants to use qiskit for scheduling and so on. If I understand it correctly, we can schedule if the |
Thanks @sbrandhsn. I see your that your points come from a "transpiler pipeline usability" perspective, which I also believe is important. I guess that my question here is how could we realize these behaviors given the current 1.: How could we actually implement this behavior? The initial way I see is to allow for 4.: Unlike the previous example, I think that this one could be implemented by navigating the gray area created by The way things are now, we can create a target with no connectivity constraints, that is not an issue. However, this results in the target's
Your resulting target will contain 2 instructions (pseudocode):
As you can see, if I wonder how that would affect cases where targets are constructed to deliberately contain global instructions, and for example, what would we want to do in cases where information doesn't match, let's say |
Sure! I see your point. :-) I guess we could also fail the user when we detect an underspecified |
Pull Request Test Coverage Report for Build 9317432966Details
💛 - Coveralls |
transpile()
to convert loose input of constraints to a Target
with Target.from_configuration()
transpile()
to convert loose input of constraints to a Target
with Target.from_configuration()
One or more of the the following people are requested to review this:
|
qiskit/compiler/transpiler.py
Outdated
basis_gates=basis_gates if _skip_target else None, | ||
coupling_map=coupling_map if _skip_target else None, | ||
instruction_durations=instruction_durations if _skip_target else None, | ||
backend_properties=backend_properties if _skip_target else None, | ||
timing_constraints=timing_constraints if _skip_target else None, | ||
inst_map=inst_map if _skip_target else None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not strictly necessary but I added it for debugging (to make sure that the target was actually being used when I wanted it to) and then thought it didn't hurt to have as an additional check. Let me know if it doesn't convince you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this would be simpler if we moved this logic down into generate_preset_pass_manager()
itself. We already expose skip_target there so internally we can change that function to generate the target instead of doing it in transpile. (it might also require moving #11996 to generate_preset_pass_manager()
too).
@@ -1329,9 +1329,9 @@ def setUpClass(cls): | |||
super().setUpClass() | |||
cls.backend = Fake27QPulseV1() | |||
cls.backend.configuration().coupling_map = MUMBAI_CMAP | |||
cls.backend.configuration().basis_gates += ["for_loop", "while_loop", "if_else"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized that this test wasn't actually updating the backend basis gates, and this made the new target-based path fail with a "control flow gate no found" error. One of the oversights from the old model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
@@ -985,7 +987,8 @@ def instruction_properties(self, index): | |||
|
|||
def _build_coupling_graph(self): | |||
self._coupling_graph = rx.PyDiGraph(multigraph=False) | |||
self._coupling_graph.add_nodes_from([{} for _ in range(self.num_qubits)]) | |||
if self.num_qubits is not None: | |||
self._coupling_graph.add_nodes_from([{} for _ in range(self.num_qubits)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these changes when I was trying to support the edge case with no coupling_map
. This is no longer necessary to support the edge cases because we are skipping the target, but given that Target
does technically suport num_qubits=None
, should we keep these extra checks?
transpile()
to convert loose input of constraints to a Target
with Target.from_configuration()
transpile()
and generate_preset_pass_manager
to convert loose input of constraints to a Target
with Target.from_configuration()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM now. I like how we've moved all the input handling to one place. I think we should be good from a backwards compat PoV, but if there is another edge case from the extra input normalization in generate_preset_pass_manager
we can handle that as part of the normal 1.2 cycle.
…e not migrated from to with the others.
…e not migrated from transpile to generate_preset_pm with the others.
…e not migrated from transpile to generate_preset_pm with the others.
…e not migrated from transpile to generate_preset_pm with the others.
* Fix oversight from Qiskit#12185 where two input parsing functions were not migrated from transpile to generate_preset_pm with the others. * Add fix to reno from Qiskit#12185
Summary
This PR is a step towards a full target-based transpiler pipeline as specified in #9256. It builds up on #11996 and #12183 (should be merged first). The main difficulty of this PR is handling the edge cases found in the legacy model that don't translate well to the target-based model, documented below. For these edge cases, backwards compatibility in the near term is ensured with the "
_skip_target
" flag, but long term support would require in many cases changes in the target model. The cases are documented in detail for future reference.Details and comments
Migration to target-based transpiler. Edge Cases.
1. No basis gates but a coupling map
_skip_target
test_symmetric_coupling_map
intest/python/transpiler/test_preset_passmanagers.py
:2. No coupling map but basis gates
Target
to skip the coupling map build if num_qubits isNone
, instead of failing (here and here).test_no_coupling_map
intest/python/transpiler/test_preset_passmanagers.py
:test_layout_3239
intest/python/transpiler/test_preset_passmanagers.py
:test_unitary_is_preserved_if_in_basis
intest/python/transpiler/test_preset_passmanagers.py
:3. No coupling map but basis gates
_skip_target
.test_no_coupling_map_with_sabre
intest/python/transpiler/test_preset_passmanagers.py
test_no_basis_gates
intest/python/transpiler/test_preset_passmanagers.py
test_level0_keeps_reset
intest/python/transpiler/test_preset_passmanagers.py
:test_initial_layout_fully_connected_cm
intest/python/transpiler/test_preset_passmanagers.py
:test_partial_layout_fully_connected_cm
intest/python/transpiler/test_preset_passmanagers.py
:Other Edge Cases
1. Unitary synthesis with parametrized gates
Description: Unitary synthesis has separate methods for when target is provided/not provided. When target is provided, it requires gate re-parametrization, but so far it only re-parametrizes RXX and RZX gates.
Workaround: This issue was a consequence of the previously proposed workaround. "Fixed" after implementation of
_skip_target
.(snippet from unitary_synthesis.py line 693)
Examples:
test_translation_method_synthesis
intest/python/compiler/test_transpiler.py
:Output:
Similar cases:
test_translate_ecr_basis
intest/python/compiler/test_transpiler.py
test_unitary_is_preserved_if_basis_is_None
intest/python/transpiler/test_preset_passmanagers.py
:test_unitary_is_preserved_if_basis_is_None_synthesis_transltion
intest/python/transpiler/test_preset_passmanagers.py
:2. Custom pulse gates
Description: Custom 2q+ pulse gates that are directional may be flipped during the layout step if gate errors are provided (i.e. backend properties), triggering errors in
HighLevelSynthesis
orGateDirection
(as it doesn't know how to flip them back).Workaround:
_skip_target
and not providing backend properties when we are skipping the target.Examples:
test_backend_and_custom_gate
intest/python/compiler/test_transpiler.py
:Output:
3. [FIXED] Control Flow:
Description: Should we always add control flow to created target?
Workaround: always add control flow instructions
Examples:
test_transpile_control_flow_no_backend
intest/python/compiler/test_transpiler.py
:Output:
(raised by passmanager)
qiskit.transpiler.exceptions.TranspilerError: "The control-flow constructs ['for_loop', 'if_else', 'while_loop', 'switch_case'] are not supported by the backend."
4. Scheduling with no coupling map:
Description:
Target.from_configuration()
currently skips instruction durations if no coupling map is provided, which causes issues with some scheduling tests.Workaround:
_skip_target
Examples:
test_circuit_with_delay
intest/python/compiler/test_transpiler.py
:Output:
qiskit.transpiler.exceptions.TranspilerError: 'Duration of cx on qubits [0, 1] is not found.'
5. Backend with custom target & asymmetric coupling map
Description: The current way we resolve the custom constraints vs. backend priorities require extracting backend (target) properties, resolving the priorities, and re-building a new target. Some special constraints like asymmetric coupling maps are lost in this conversion.
Workaround: added
_no_loose_constraints
flag, ifTrue
, the target isn't built from config but directly taken from the backend.Examples:
test_transpile
intest/python/providers/test_backend_v2.py
: