From 1b43c89be05d0967557eee8a2d2f32bd529627d1 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Thu, 6 Mar 2025 02:01:25 +0000 Subject: [PATCH] Delegate `BasePass.__call__` to `PassManager.run` (#13820) * Delegate `BasePass.__call__` to `PassManager.run` This ensures that calling a pass directly will follow the same execution logic as a regular pass-manager construction. This particularly matters for passes that have `requires`. It also ensures that the conversion from a circuit and back to a DAG will follow the same rules, which is expected since it is a shorthand notation, but liable to get out of sync as they are complex. * Correctly propagate exceptions through `PassManager.run` * Fix lint --- qiskit/passmanager/passmanager.py | 34 ++++++++-- qiskit/transpiler/basepasses.py | 66 ++++++------------- qiskit/transpiler/passmanager.py | 13 ++++ ...-call-as-passmanager-7f874917b9b303f0.yaml | 24 +++++++ .../transpiler/test_elide_permutations.py | 3 +- test/python/transpiler/test_pass_call.py | 31 ++++++++- test/python/transpiler/test_passmanager.py | 19 +++++- .../visualization/test_circuit_text_drawer.py | 8 +-- 8 files changed, 136 insertions(+), 62 deletions(-) create mode 100644 releasenotes/notes/pass-call-as-passmanager-7f874917b9b303f0.yaml diff --git a/qiskit/passmanager/passmanager.py b/qiskit/passmanager/passmanager.py index 85f422f181b9..ae62bae1b2c6 100644 --- a/qiskit/passmanager/passmanager.py +++ b/qiskit/passmanager/passmanager.py @@ -174,6 +174,8 @@ def run( in_programs: Any | list[Any], callback: Callable = None, num_processes: int = None, + *, + property_set: dict[str, object] | None = None, **kwargs, ) -> Any: """Run all the passes on the specified ``in_programs``. @@ -211,7 +213,11 @@ def callback_func(**kwargs): execution is enabled. This argument overrides ``num_processes`` in the user configuration file, and the ``QISKIT_NUM_PROCS`` environment variable. If set to ``None`` the system default or local user configuration will be used. - + property_set: If given, the initial value to use as the :class:`.PropertySet` for the + pass manager pipeline. This can be used to persist analysis from one run to + another, in cases where you know the analysis is safe to share. Beware that some + analysis will be specific to the input circuit and the particular :class:`.Target`, + so you should take a lot of care when using this argument. kwargs: Arbitrary arguments passed to the compiler frontend and backend. Returns: @@ -229,7 +235,13 @@ def callback_func(**kwargs): # ourselves, since that can be quite expensive. if len(in_programs) == 1 or not should_run_in_parallel(num_processes): out = [ - _run_workflow(program=program, pass_manager=self, callback=callback, **kwargs) + _run_workflow( + program=program, + pass_manager=self, + callback=callback, + initial_property_set=property_set, + **kwargs, + ) for program in in_programs ] if len(in_programs) == 1 and not is_list: @@ -246,7 +258,10 @@ def callback_func(**kwargs): return parallel_map( _run_workflow_in_new_process, values=in_programs, - task_kwargs={"pass_manager_bin": dill.dumps(self)}, + task_kwargs={ + "pass_manager_bin": dill.dumps(self), + "initial_property_set": property_set, + }, num_processes=num_processes, ) @@ -270,6 +285,8 @@ def _flatten_tasks(self, elements: Iterable | Task) -> Iterable: def _run_workflow( program: Any, pass_manager: BasePassManager, + *, + initial_property_set: dict[str, object] | None = None, **kwargs, ) -> Any: """Run single program optimization with a pass manager. @@ -289,12 +306,12 @@ def _run_workflow( input_program=program, **kwargs, ) + property_set = ( + PropertySet() if initial_property_set is None else PropertySet(initial_property_set) + ) passmanager_ir, final_state = flow_controller.execute( passmanager_ir=passmanager_ir, - state=PassManagerState( - workflow_status=initial_status, - property_set=PropertySet(), - ), + state=PassManagerState(workflow_status=initial_status, property_set=property_set), callback=kwargs.get("callback", None), ) # The `property_set` has historically been returned as a mutable attribute on `PassManager` @@ -317,6 +334,8 @@ def _run_workflow( def _run_workflow_in_new_process( program: Any, pass_manager_bin: bytes, + *, + initial_property_set: dict[str, object] | None, ) -> Any: """Run single program optimization in new process. @@ -330,4 +349,5 @@ def _run_workflow_in_new_process( return _run_workflow( program=program, pass_manager=dill.loads(pass_manager_bin), + initial_property_set=initial_property_set, ) diff --git a/qiskit/transpiler/basepasses.py b/qiskit/transpiler/basepasses.py index 1e51e4165457..d38d750673aa 100644 --- a/qiskit/transpiler/basepasses.py +++ b/qiskit/transpiler/basepasses.py @@ -19,13 +19,11 @@ from inspect import signature from qiskit.circuit import QuantumCircuit -from qiskit.converters import circuit_to_dag, dag_to_circuit from qiskit.dagcircuit import DAGCircuit from qiskit.passmanager.base_tasks import GenericPass, PassManagerIR from qiskit.passmanager.compilation_status import PropertySet, RunState, PassManagerState from .exceptions import TranspilerError -from .layout import TranspileLayout class MetaPass(abc.ABCMeta): @@ -126,57 +124,31 @@ def __call__( Args: circuit: The dag on which the pass is run. - property_set: Input/output property set. An analysis pass - might change the property set in-place. + property_set: Input/output property set. An analysis pass might change the property set + in-place. If not given, the existing ``property_set`` attribute of the pass will + be used (if set). Returns: If on transformation pass, the resulting QuantumCircuit. If analysis pass, the input circuit. """ - property_set_ = None - if isinstance(property_set, dict): # this includes (dict, PropertySet) - property_set_ = PropertySet(property_set) - - if isinstance(property_set_, PropertySet): - # pylint: disable=attribute-defined-outside-init - self.property_set = property_set_ - - result = self.run(circuit_to_dag(circuit)) - - result_circuit = circuit - - if isinstance(property_set, dict): # this includes (dict, PropertySet) + from qiskit.transpiler import PassManager # pylint: disable=cyclic-import + + pm = PassManager([self]) + # Previous versions of the `__call__` function would not construct a `PassManager`, but just + # call `self.run` directly (this caused issues with `requires`). It only overrode + # `self.property_set` if the input was not `None`, which some users might have been relying + # on (as our test suite was). + if property_set is None: + property_set = self.property_set + out = pm.run(circuit, property_set=property_set) + if property_set is not None and property_set is not pm.property_set: + # When this `__call__` was first added, it contained this behaviour of mutating the + # input `property_set` in-place, but didn't use the `PassManager` infrastructure. This + # preserves the output-variable nature of the `property_set` parameter. property_set.clear() - property_set.update(self.property_set) - - if isinstance(result, DAGCircuit): - result_circuit = dag_to_circuit(result, copy_operations=False) - elif result is None: - result_circuit = circuit.copy() - - if self.property_set["layout"]: - result_circuit._layout = TranspileLayout( - initial_layout=self.property_set["layout"], - input_qubit_mapping=self.property_set["original_qubit_indices"], - final_layout=self.property_set["final_layout"], - _input_qubit_count=len(circuit.qubits), - _output_qubit_list=result_circuit.qubits, - ) - if self.property_set["clbit_write_latency"] is not None: - result_circuit._clbit_write_latency = self.property_set["clbit_write_latency"] - if self.property_set["conditional_latency"] is not None: - result_circuit._conditional_latency = self.property_set["conditional_latency"] - if self.property_set["node_start_time"]: - # This is dictionary keyed on the DAGOpNode, which is invalidated once - # dag is converted into circuit. So this schedule information is - # also converted into list with the same ordering with circuit.data. - topological_start_times = [] - start_times = self.property_set["node_start_time"] - for dag_node in result.topological_op_nodes(): - topological_start_times.append(start_times[dag_node]) - result_circuit._op_start_times = topological_start_times - - return result_circuit + property_set.update(pm.property_set) + return out class AnalysisPass(BasePass): # pylint: disable=abstract-method diff --git a/qiskit/transpiler/passmanager.py b/qiskit/transpiler/passmanager.py index c905d6142144..391b35c22fc9 100644 --- a/qiskit/transpiler/passmanager.py +++ b/qiskit/transpiler/passmanager.py @@ -174,6 +174,8 @@ def run( # pylint:disable=arguments-renamed output_name: str | None = None, callback: Callable = None, num_processes: int = None, + *, + property_set: dict[str, object] | None = None, ) -> _CircuitsT: """Run all the passes on the specified ``circuits``. @@ -216,6 +218,11 @@ def callback_func(**kwargs): execution is enabled. This argument overrides ``num_processes`` in the user configuration file, and the ``QISKIT_NUM_PROCS`` environment variable. If set to ``None`` the system default or local user configuration will be used. + property_set: If given, the initial value to use as the :class:`.PropertySet` for the + pass manager pipeline. This can be used to persist analysis from one run to + another, in cases where you know the analysis is safe to share. Beware that some + analysis will be specific to the input circuit and the particular :class:`.Target`, + so you should take a lot of care when using this argument. Returns: The transformed circuit(s). @@ -228,6 +235,7 @@ def callback_func(**kwargs): callback=callback, output_name=output_name, num_processes=num_processes, + property_set=property_set, ) def draw(self, filename=None, style=None, raw=False): @@ -436,6 +444,8 @@ def run( output_name: str | None = None, callback: Callable | None = None, num_processes: int = None, + *, + property_set: dict[str, object] | None = None, ) -> _CircuitsT: self._update_passmanager() return super().run(circuits, output_name, callback, num_processes=num_processes) @@ -462,6 +472,9 @@ def _replace_error(meth): def wrapper(*meth_args, **meth_kwargs): try: return meth(*meth_args, **meth_kwargs) + except TranspilerError: + # If it's already a `TranspilerError` subclass, don't erase the extra information. + raise except PassManagerError as ex: raise TranspilerError(ex.message) from ex diff --git a/releasenotes/notes/pass-call-as-passmanager-7f874917b9b303f0.yaml b/releasenotes/notes/pass-call-as-passmanager-7f874917b9b303f0.yaml new file mode 100644 index 000000000000..a927cbbd903a --- /dev/null +++ b/releasenotes/notes/pass-call-as-passmanager-7f874917b9b303f0.yaml @@ -0,0 +1,24 @@ +--- +features_transpiler: + - | + :meth:`.PassManager.run` now accepts a ``property_set`` argument, which can be set to a + :class:`~collections.abc.Mapping`-like object to provide the initial values of the pipeline's + :class:`.PropertySet`. This can be used to recommence a partially applied compilation, or to + reuse certain analysis from a prior compilation in a new place. +upgrade_transpiler: + - | + The keyword argument ``property_set`` is now reserved in :meth:`.BasePassManager.run`, and + cannot be used as a ``kwarg`` that will be forwarded to the subclass's conversion from the + front-end representation to the internal representation. +fixes: + - | + Calling an :class:`.AnalysisPass` or a :class:`.TransformationPass` like a function (as in + ``pass_ = MyPass(); pass_(qc)``) will now respect any requirements that the pass might have. + For example, scheduling passes such as :class:`.ALAPScheduleAnalysis` require that + :class:`.TimeUnitConversion` runs before them. Running the pass via a :class:`.PassManager` + always respected this requirement, but until this note, it was not respected by calling the + pass directly. + - | + When a :exc:`.TranspilerError` subclass is raised by a pass inside a call to + :meth:`.PassManger.run`, the exception will now be propagated through losslessly, rather than + becoming a chained exception with an erased type. diff --git a/test/python/transpiler/test_elide_permutations.py b/test/python/transpiler/test_elide_permutations.py index 6aa2d7804b80..dd59903db5ef 100644 --- a/test/python/transpiler/test_elide_permutations.py +++ b/test/python/transpiler/test_elide_permutations.py @@ -225,8 +225,7 @@ def test_partial_permutation_in_middle(self): # Make sure that the transpiled circuit *with* the final permutation # is equivalent to the original circuit - perm = pass_.property_set["virtual_permutation_layout"].to_permutation(qc.qubits) - res.append(PermutationGate(perm), [0, 1, 2, 3, 4]) + res.append(PermutationGate(res.layout.routing_permutation()), [0, 1, 2, 3, 4]) self.assertEqual(Operator(res), Operator(qc)) def test_permutation_at_beginning(self): diff --git a/test/python/transpiler/test_pass_call.py b/test/python/transpiler/test_pass_call.py index a8478f67330f..86cd9e9a3773 100644 --- a/test/python/transpiler/test_pass_call.py +++ b/test/python/transpiler/test_pass_call.py @@ -13,7 +13,7 @@ """Test calling passes (passmanager-less)""" from qiskit import QuantumRegister, QuantumCircuit -from qiskit.transpiler import PropertySet +from qiskit.transpiler import PropertySet, TransformationPass, AnalysisPass from ._dummy_passes import PassD_TP_NR_NP, PassE_AP_NR_NP, PassN_AP_NR_NP from test import QiskitTestCase # pylint: disable=wrong-import-order @@ -90,3 +90,32 @@ def test_analysis_pass_remove_property(self): self.assertEqual(property_set, PropertySet({"to none": None})) self.assertIsInstance(property_set, dict) self.assertEqual(circuit, result) + + def test_pass_requires(self): + """The 'requires' field of a pass should be respected when called.""" + name = "my_property" + value = "hello, world" + + assert_equal = self.assertEqual + + class Analyse(AnalysisPass): + """Dummy pass to set a property.""" + + def run(self, dag): + self.property_set[name] = value + return dag + + class Execute(TransformationPass): + """Dummy pass to assert that its required pass ran.""" + + def __init__(self): + super().__init__() + self.requires.append(Analyse()) + + def run(self, dag): + assert_equal(self.property_set[name], value) + return dag + + pass_ = Execute() + pass_(QuantumCircuit()) + self.assertEqual(pass_.property_set[name], value) diff --git a/test/python/transpiler/test_passmanager.py b/test/python/transpiler/test_passmanager.py index 6a72ffb16a86..8a0faf0edc5c 100644 --- a/test/python/transpiler/test_passmanager.py +++ b/test/python/transpiler/test_passmanager.py @@ -26,7 +26,7 @@ ConditionalController, DoWhileController, ) -from qiskit.transpiler import PassManager, PropertySet, TransformationPass +from qiskit.transpiler import PassManager, PropertySet, TransformationPass, AnalysisPass from qiskit.transpiler.passes import Optimize1qGates, BasisTranslator, ResourceEstimation from qiskit.circuit.library.standard_gates.equivalence_library import ( StandardEquivalenceLibrary as std_eqlib, @@ -191,3 +191,20 @@ def callback(pass_, **_): "third 4", ] self.assertEqual(calls, expected) + + def test_override_initial_property_set(self): + """Test that the ``property_set`` argument allows seeding the base analysis.""" + input_name = "my_property" + output_name = "output_property" + + class Analyse(AnalysisPass): + def run(self, dag): + self.property_set[output_name] = self.property_set[input_name] + return dag + + pm = PassManager([Analyse()]) + pm.run(QuantumCircuit(), property_set={input_name: "hello, world"}) + self.assertEqual(pm.property_set[output_name], "hello, world") + + pm.run(QuantumCircuit(), property_set={input_name: "a different string"}) + self.assertEqual(pm.property_set[output_name], "a different string") diff --git a/test/python/visualization/test_circuit_text_drawer.py b/test/python/visualization/test_circuit_text_drawer.py index 722f1bb2bd7b..33523742b243 100644 --- a/test/python/visualization/test_circuit_text_drawer.py +++ b/test/python/visualization/test_circuit_text_drawer.py @@ -3275,8 +3275,8 @@ def test_mixed_layout(self): circuit.h(qr) pass_ = ApplyLayout() - pass_.property_set["layout"] = Layout({qr[0]: 0, ancilla[1]: 1, ancilla[0]: 2, qr[1]: 3}) - circuit_with_layout = pass_(circuit) + layout = Layout({qr[0]: 0, ancilla[1]: 1, ancilla[0]: 2, qr[1]: 3}) + circuit_with_layout = pass_(circuit, property_set={"layout": layout}) self.assertEqual( str(circuit_drawer(circuit_with_layout, output="text", initial_state=True)), expected @@ -3337,8 +3337,8 @@ def test_with_classical_regs(self): circuit.measure(qr2[1], cr[1]) pass_ = ApplyLayout() - pass_.property_set["layout"] = Layout({qr1[0]: 0, qr1[1]: 1, qr2[0]: 2, qr2[1]: 3}) - circuit_with_layout = pass_(circuit) + layout = Layout({qr1[0]: 0, qr1[1]: 1, qr2[0]: 2, qr2[1]: 3}) + circuit_with_layout = pass_(circuit, property_set={"layout": layout}) self.assertEqual( str(circuit_drawer(circuit_with_layout, output="text", initial_state=True)), expected