Migrate layout stage to plugins #10622
Conversation
This commit updates the preset pass manager construction to only use plugins for the layout stage. To accomplish this the previously hard coded built-in layout methods, trivial, dense, noise adpative, and sabre are migrated to be exposed as built-in plugins. Additionally, the special case of layout_method=None has been centralized into a standard default method plugin, as the pass construction in this case involved extra steps for each optimization level. This simplifies the preset pass manager construction as now the layout stage is solely built via plugins. Fixes Qiskit#9455
mtreinish
added
Changelog: API Change
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 5858335590
💛 - Coveralls |
| def pass_manager(self, pass_manager_config, optimization_level=None) -> PassManager: | ||
| _given_layout = SetLayout(pass_manager_config.initial_layout) | ||
|
|
||
| def _choose_layout_condition(property_set): | ||
| return not property_set["layout"] | ||
|
|
||
| def _layout_not_perfect(property_set): | ||
| """Return ``True`` if the first attempt at layout has been checked and found to be | ||
| imperfect. In this case, perfection means "does not require any swap routing".""" | ||
| return property_set["is_swap_mapped"] is not None and not property_set["is_swap_mapped"] |
There was a problem hiding this comment.
Are there any concerns about pickle-ability of the resulting pass manager now we're retuning method-local functions as part of the conditions?
(also, lol at using explicitly private names when defining method-internal functions - a clear copy-paste sign haha)
There was a problem hiding this comment.
There shouldn't be as it's passing the parallel transpiler tests which should be testing this.
| layout.append( | ||
| NoiseAdaptiveLayout(pass_manager_config.target), condition=_choose_layout_condition | ||
| ) | ||
| layout += common.generate_embed_passmanager(coupling_map) |
There was a problem hiding this comment.
With how we use this, I wonder a little if embed should be a separate stage in the transpiler pipeline. There's only ever a single version of it that would make sense, I guess, which reduces its importance, but it's a common usage pitfall to forget to run an embed step after what most people consider to be a complete "layout" stage, and it might be a clearer separation of concerns if "embed" is a separate stage.
That said, it'd probably play even weirder with the hybrid layout+routing manager we do in the defaults.
There was a problem hiding this comment.
Yeah, it potentially would have made sense when we originally did the stages. I was thinking also as a post-layout stage it would work instead of doing an explicit embed stage. But that layout ensure it is embedded is baked into the plugin interface now: https://qiskit.org/documentation/apidoc/transpiler_plugins.html#plugin-stages (under expectations for layouts). If we wanted to split it out now we'd have to condition the stage to only run if it's not already been embedded.
There was a problem hiding this comment.
This looks fine to me now, thanks. I guess my main problem with the optimization_level argument to PassManagerStagePlugin.pass_manager is that it's given a default of None by the interface, but it's actually required to be given and be an integer. That's beyond the scope of implementing plugins here, but imo it should either have not had a default value at all, or had a default of 1.
* Migrate layout stage to plugins This commit updates the preset pass manager construction to only use plugins for the layout stage. To accomplish this the previously hard coded built-in layout methods, trivial, dense, noise adpative, and sabre are migrated to be exposed as built-in plugins. Additionally, the special case of layout_method=None has been centralized into a standard default method plugin, as the pass construction in this case involved extra steps for each optimization level. This simplifies the preset pass manager construction as now the layout stage is solely built via plugins. Fixes Qiskit#9455 * Remove unnecessary if statement * Handle invalid optimization levels in plugins * Remove unused variable post-rebase
Summary
This commit updates the preset pass manager construction to only use plugins for the layout stage. To accomplish this the previously hard coded built-in layout methods, trivial, dense, noise adaptive, and sabre are migrated to be exposed as built-in plugins. Additionally, the special case of layout_method=None has been centralized into a standard default method plugin, as the pass construction in this case involved extra steps for each optimization level. This simplifies the preset pass manager construction as now the layout stage is solely built via plugins.
Details and comments
Fixes #9455