Base pass manager bug fixes

qiskit/passmanager/base_tasks.py

@@ -19,7 +19,7 @@
 from collections.abc import Iterable, Callable, Generator
 from typing import Any
 
-from .compilation_status import RunState, PassManagerState
+from .compilation_status import RunState, PassManagerState, PropertySet
 
 logger = logging.getLogger(__name__)
 
@@ -62,6 +62,7 @@ class GenericPass(Task, ABC):
     """
 
     def __init__(self):
+        self.property_set = PropertySet()
         self.requires: Iterable[Task] = []
 
     def name(self) -> str:
@@ -77,6 +78,7 @@ def execute(
         # Overriding this method is not safe.
         # Pass subclass must keep current implementation.
         # Especially, task execution may break when method signature is modified.
+        self.property_set = state.property_set
 
         if self.requires:
             # pylint: disable=cyclic-import

qiskit/passmanager/flow_controllers.py

@@ -133,6 +133,8 @@ def iter_tasks(self, state: PassManagerState) -> Generator[Task, PassManagerStat
                 state = yield task
             if not self.do_while(state.property_set):
                 return
+            # Remove stored tasks from the completed task collection for next loop
+            state.workflow_status.completed_passes.difference_update(self.tasks)
         raise PassManagerError("Maximum iteration reached. max_iteration=%i" % max_iteration)
 
 

qiskit/passmanager/passmanager.py

@@ -212,7 +212,7 @@ def callback_func(**kwargs):
             return in_programs
 
         is_list = True
-        if not isinstance(in_programs, Sequence):
+        if not (isinstance(in_programs, Sequence) and not isinstance(in_programs, str)):
             in_programs = [in_programs]
             is_list = False
 

qiskit/transpiler/basepasses.py

@@ -74,7 +74,6 @@ class BasePass(GenericPass, metaclass=MetaPass):
     def __init__(self):
         super().__init__()
         self.preserves: Iterable[GenericPass] = []
-        self.property_set = PropertySet()
         self._hash = hash(None)
 
     def __hash__(self):
@@ -118,21 +117,6 @@ def is_analysis_pass(self):
         """
         return isinstance(self, AnalysisPass)
 
-    def execute(
-        self,
-        passmanager_ir: PassManagerIR,
-        state: PassManagerState,
-        callback: Callable = None,
-    ) -> tuple[PassManagerIR, PassManagerState]:
-        # For backward compatibility.
-        # Circuit passes access self.property_set.
-        self.property_set = state.property_set
-        return super().execute(
-            passmanager_ir=passmanager_ir,
-            state=state,
-            callback=callback,
-        )
-
     def __call__(
         self,
         circuit: QuantumCircuit,

test/python/passmanager/__init__.py

@@ -0,0 +1,77 @@
+# This code is part of Qiskit.
+#
+# (C) Copyright IBM 2023
+#
+# This code is licensed under the Apache License, Version 2.0. You may
+# obtain a copy of this license in the LICENSE.txt file in the root directory
+# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# Any modifications or derivative works of this code must retain this
+# copyright notice, and modified files need to carry a notice indicating
+# that they have been altered from the originals.
+
+"""Pass manager test cases."""
+
+import io
+import re
+from itertools import zip_longest
+from logging import StreamHandler, getLogger
+
+from qiskit.passmanager import PassManagerState, WorkflowStatus, PropertySet
+from qiskit.test import QiskitTestCase
+
+
+class PassManagerTestCase(QiskitTestCase):
+    """Test case for the pass manager module."""
+
+    def setUp(self):
+        super().setUp()
+
+        self.output = io.StringIO()
+        self.state = PassManagerState(
+            workflow_status=WorkflowStatus(),
+            property_set=PropertySet(),
+        )
+
+        logger = getLogger()
+        self.addCleanup(logger.setLevel, logger.level)
+
+        logger.setLevel("DEBUG")
+        logger.addHandler(StreamHandler(self.output))
+
+    def assertLogEqual(self, func, expected_lines, *args, exception_type=None):
+        """Execute provided function and verify logger.
+
+        Args:
+            func (Callable): Function to test.
+            expected_lines (List[str]): Expected log output.
+            args (Any): Arguments to the function.
+            exception_type (Type): Optional. Expected exception for error handling.
+
+        Returns:
+            Any: Output values from the function.
+        """
+        if exception_type:
+            self.assertRaises(exception_type, func, *args)
+            out = None
+        else:
+            out = func(*args)
+
+        self.output.seek(0)
+        recorded_lines = [line.rstrip() for line in self.output.readlines()]
+        for i, (expected, recorded) in enumerate(zip_longest(expected_lines, recorded_lines)):
+            expected = expected or ""
+            recorded = recorded or ""
+            if not re.fullmatch(expected, recorded):
+                raise AssertionError(
+                    f"Log didn't match. Mismatch found at line #{i}.\n\n"
+                    f"Expected:\n{self._format_log(expected_lines)}\n"
+                    f"Recorded:\n{self._format_log(recorded_lines)}"
+                )
+        return out
+
+    def _format_log(self, lines):
+        out = ""
+        for i, line in enumerate(lines):
+            out += f"#{i:02d}: {line}\n"
+        return out

test/python/passmanager/test_generic_pass.py

@@ -0,0 +1,126 @@
+# This code is part of Qiskit.
+#
+# (C) Copyright IBM 2023
+#
+# This code is licensed under the Apache License, Version 2.0. You may
+# obtain a copy of this license in the LICENSE.txt file in the root directory
+# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# Any modifications or derivative works of this code must retain this
+# copyright notice, and modified files need to carry a notice indicating
+# that they have been altered from the originals.
+# pylint: disable=missing-class-docstring
+
+"""Pass manager test cases."""
+
+from test.python.passmanager import PassManagerTestCase
+
+from logging import getLogger
+
+from qiskit.passmanager import GenericPass
+from qiskit.passmanager.compilation_status import RunState
+
+
+class TestGenericPass(PassManagerTestCase):
+    """Tests for the GenericPass subclass."""
+
+    def test_run_task(self):
+        """Test case: Simple successful task execution."""
+
+        class Task(GenericPass):
+            def run(self, passmanager_ir):
+                return passmanager_ir
+
+        task = Task()
+        data = "test_data"
+        expected = [r"Pass: Task - (\d*\.)?\d+ \(ms\)"]
+        self.assertLogEqual(task.execute, expected, data, self.state)
+        self.assertEqual(self.state.workflow_status.count, 1)
+        self.assertIn(task, self.state.workflow_status.completed_passes)
+        self.assertEqual(self.state.workflow_status.previous_run, RunState.SUCCESS)
+
+    def test_failure_task(self):
+        """Test case: Log is created regardless of success."""
+
+        class TestError(Exception):
+            pass
+
+        class RaiseError(GenericPass):
+            def run(self, passmanager_ir):
+                raise TestError()
+
+        task = RaiseError()
+        data = "test_data"
+        expected = [r"Pass: RaiseError - (\d*\.)?\d+ \(ms\)"]
+        self.assertLogEqual(task.execute, expected, data, self.state, exception_type=TestError)
+        self.assertEqual(self.state.workflow_status.count, 0)
+        self.assertNotIn(task, self.state.workflow_status.completed_passes)
+        self.assertEqual(self.state.workflow_status.previous_run, RunState.FAIL)
+
+    def test_requires(self):
+        """Test cate: Dependency tasks are run in advance to user provided task."""
+
+        class TaskA(GenericPass):
+            def run(self, passmanager_ir):
+                return passmanager_ir
+
+        class TaskB(GenericPass):
+            def __init__(self):
+                super().__init__()
+                self.requires = [TaskA()]
+
+            def run(self, passmanager_ir):
+                return passmanager_ir
+
+        task = TaskB()
+        data = "test_data"
+        expected = [
+            r"Pass: TaskA - (\d*\.)?\d+ \(ms\)",
+            r"Pass: TaskB - (\d*\.)?\d+ \(ms\)",
+        ]
+        self.assertLogEqual(task.execute, expected, data, self.state)
+        self.assertEqual(self.state.workflow_status.count, 2)
+
+    def test_requires_in_list(self):
+        """Test cate: Dependency tasks are not executed multiple times."""
+
+        class TaskA(GenericPass):
+            def run(self, passmanager_ir):
+                return passmanager_ir
+
+        class TaskB(GenericPass):
+            def __init__(self):
+                super().__init__()
+                self.requires = [TaskA()]
+
+            def run(self, passmanager_ir):
+                return passmanager_ir
+
+        task = TaskB()
+        data = "test_data"
+        expected = [
+            r"Pass: TaskB - (\d*\.)?\d+ \(ms\)",
+        ]
+        self.state.workflow_status.completed_passes.add(task.requires[0])  # already done
+        self.assertLogEqual(task.execute, expected, data, self.state)
+        self.assertEqual(self.state.workflow_status.count, 1)
+
+    def test_run_with_callable(self):
+        """Test case: Callable is called after generic pass is run."""
+
+        # pylint: disable=unused-argument
+        def test_callable(task, passmanager_ir, property_set, running_time, count):
+            logger = getLogger()
+            logger.info("%s is running on %s", task.name(), passmanager_ir)
+
+        class Task(GenericPass):
+            def run(self, passmanager_ir):
+                return passmanager_ir
+
+        task = Task()
+        data = "test_data"
+        expected = [
+            r"Pass: Task - (\d*\.)?\d+ \(ms\)",
+            r"Task is running on test_data",
+        ]
+        self.assertLogEqual(task.execute, expected, data, self.state, test_callable)