fix famos termination; remove other switching methods

petab_select/candidate_space.py

@@ -560,100 +560,100 @@ class BackwardCandidateSpace(ForwardCandidateSpace):
     direction = -1
 
 
-class BidirectionalCandidateSpace(ForwardCandidateSpace):
-    """The bidirectional method class.
-
-    Attributes:
-        method_history:
-            The history of models that were found at each search.
-            A list of dictionaries, where each dictionary contains keys for the `METHOD`
-            and the list of `MODELS`.
-    """
-
-    # TODO refactor method to inherit from governing_method if not specified
-    #      by constructor argument -- remove from here.
-    method = Method.BIDIRECTIONAL
-    governing_method = Method.BIDIRECTIONAL
-    retry_model_space_search_if_no_models = True
-
-    def __init__(
-        self,
-        *args,
-        initial_method: Method = Method.FORWARD,
-        **kwargs,
-    ):
-        super().__init__(*args, **kwargs)
-
-        # FIXME cannot access from CLI
-        # FIXME probably fine to replace `self.initial_method`
-        #       with `self.method` here. i.e.:
-        #       1. change `method` to `Method.FORWARD
-        #       2. change signature to `initial_method: Method = None`
-        #       3. change code here to `if initial_method is not None: self.method = initial_method`
-        self.initial_method = initial_method
-
-        self.method_history: List[Dict[str, Union[Method, List[Model]]]] = []
-
-    def update_method(self, method: Method):
-        if method == Method.FORWARD:
-            self.direction = 1
-        elif method == Method.BACKWARD:
-            self.direction = -1
-        else:
-            raise NotImplementedError(
-                f'Bidirectional direction must be either `Method.FORWARD` or `Method.BACKWARD`, not {method}.'
-            )
-
-        self.method = method
-
-    def switch_method(self):
-        if self.method == Method.FORWARD:
-            method = Method.BACKWARD
-        elif self.method == Method.BACKWARD:
-            method = Method.FORWARD
-
-        self.update_method(method=method)
-
-    def setup_before_model_subspaces_search(self):
-        # If previous search found no models, then switch method.
-        previous_search = (
-            None if not self.method_history else self.method_history[-1]
-        )
-        if previous_search is None:
-            self.update_method(self.initial_method)
-            return
-
-        self.update_method(previous_search[METHOD])
-        if not previous_search[MODELS]:
-            self.switch_method()
-            self.retry_model_space_search_if_no_models = False
-
-    def setup_after_model_subspaces_search(self):
-        current_search = {
-            METHOD: self.method,
-            MODELS: self.models,
-        }
-        self.method_history.append(current_search)
-        self.method = self.governing_method
-
-    def wrap_search_subspaces(self, search_subspaces):
-        def wrapper():
-            def iterate():
-                self.setup_before_model_subspaces_search()
-                search_subspaces()
-                self.setup_after_model_subspaces_search()
-
-            # Repeat until models are found or switching doesn't help.
-            iterate()
-            while (
-                not self.models and self.retry_model_space_search_if_no_models
-            ):
-                iterate()
-
-            # Reset flag for next time.
-            self.retry_model_space_search_if_no_models = True
-
-        return wrapper
+#class BidirectionalCandidateSpace(ForwardCandidateSpace):
+#    """The bidirectional method class.
+#
+#    Attributes:
+#        method_history:
+#            The history of models that were found at each search.
+#            A list of dictionaries, where each dictionary contains keys for the `METHOD`
+#            and the list of `MODELS`.
+#    """
+#
+#    # TODO refactor method to inherit from governing_method if not specified
+#    #      by constructor argument -- remove from here.
+#    method = Method.BIDIRECTIONAL
+#    governing_method = Method.BIDIRECTIONAL
+#    retry_model_space_search_if_no_models = True
+#
+#    def __init__(
+#        self,
+#        *args,
+#        initial_method: Method = Method.FORWARD,
+#        **kwargs,
+#    ):
+#        super().__init__(*args, **kwargs)
+#
+#        # FIXME cannot access from CLI
+#        # FIXME probably fine to replace `self.initial_method`
+#        #       with `self.method` here. i.e.:
+#        #       1. change `method` to `Method.FORWARD
+#        #       2. change signature to `initial_method: Method = None`
+#        #       3. change code here to `if initial_method is not None: self.method = initial_method`
+#        self.initial_method = initial_method
+#
+#        self.method_history: List[Dict[str, Union[Method, List[Model]]]] = []
+#
+#    def update_method(self, method: Method):
+#        if method == Method.FORWARD:
+#            self.direction = 1
+#        elif method == Method.BACKWARD:
+#            self.direction = -1
+#        else:
+#            raise NotImplementedError(
+#                f'Bidirectional direction must be either `Method.FORWARD` or `Method.BACKWARD`, not {method}.'
+#            )
+#
+#        self.method = method
+#
+#    def switch_method(self):
+#        if self.method == Method.FORWARD:
+#            method = Method.BACKWARD
+#        elif self.method == Method.BACKWARD:
+#            method = Method.FORWARD
+#
+#        self.update_method(method=method)
+#
+#    def setup_before_model_subspaces_search(self):
+#        # If previous search found no models, then switch method.
+#        previous_search = (
+#            None if not self.method_history else self.method_history[-1]
+#        )
+#        if previous_search is None:
+#            self.update_method(self.initial_method)
+#            return
+#
+#        self.update_method(previous_search[METHOD])
+#        if not previous_search[MODELS]:
+#            self.switch_method()
+#            self.retry_model_space_search_if_no_models = False
+#
+#    def setup_after_model_subspaces_search(self):
+#        current_search = {
+#            METHOD: self.method,
+#            MODELS: self.models,
+#        }
+#        self.method_history.append(current_search)
+#        self.method = self.governing_method
+#
+#    def wrap_search_subspaces(self, search_subspaces):
+#        def wrapper():
+#            def iterate():
+#                self.setup_before_model_subspaces_search()
+#                search_subspaces()
+#                self.setup_after_model_subspaces_search()
+#
+#            # Repeat until models are found or switching doesn't help.
+#            iterate()
+#            while (
+#                not self.models and self.retry_model_space_search_if_no_models
+#            ):
+#                iterate()
+#
+#            # Reset flag for next time.
+#            self.retry_model_space_search_if_no_models = True
+#
+#        return wrapper
 
 
 class FamosCandidateSpace(CandidateSpace):
@@ -1256,31 +1256,31 @@ def wrapper():
         return wrapper
 
 
-# TODO rewrite so BidirectionalCandidateSpace inherits from ForwardAndBackwardCandidateSpace
-#      instead
-class ForwardAndBackwardCandidateSpace(BidirectionalCandidateSpace):
-    method = Method.FORWARD_AND_BACKWARD
-    governing_method = Method.FORWARD_AND_BACKWARD
-    retry_model_space_search_if_no_models = False
-
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs, initial_method=None)
-
-    def wrap_search_subspaces(self, search_subspaces):
-        def wrapper():
-            for method in [Method.FORWARD, Method.BACKWARD]:
-                self.update_method(method=method)
-                search_subspaces()
-                self.setup_after_model_subspaces_search()
-
-        return wrapper
-
-    # Disable unused interface
-    setup_before_model_subspaces_search = None
-    switch_method = None
-
-    def setup_after_model_space_search(self):
-        pass
+## TODO rewrite so BidirectionalCandidateSpace inherits from ForwardAndBackwardCandidateSpace
+##      instead
+#class ForwardAndBackwardCandidateSpace(BidirectionalCandidateSpace):
+#    method = Method.FORWARD_AND_BACKWARD
+#    governing_method = Method.FORWARD_AND_BACKWARD
+#    retry_model_space_search_if_no_models = False
+#
+#    def __init__(self, *args, **kwargs):
+#        super().__init__(*args, **kwargs, initial_method=None)
+#
+#    def wrap_search_subspaces(self, search_subspaces):
+#        def wrapper():
+#            for method in [Method.FORWARD, Method.BACKWARD]:
+#                self.update_method(method=method)
+#                search_subspaces()
+#                self.setup_after_model_subspaces_search()
+#
+#        return wrapper
+#
+#    # Disable unused interface
+#    setup_before_model_subspaces_search = None
+#    switch_method = None
+#
+#    def setup_after_model_space_search(self):
+#        pass
 
 
 class LateralCandidateSpace(CandidateSpace):
@@ -1362,10 +1362,10 @@ def _consider_method(self, model):
 candidate_space_classes = [
     ForwardCandidateSpace,
     BackwardCandidateSpace,
-    BidirectionalCandidateSpace,
+    #BidirectionalCandidateSpace,
     LateralCandidateSpace,
     BruteForceCandidateSpace,
-    ForwardAndBackwardCandidateSpace,
+    #ForwardAndBackwardCandidateSpace,
     FamosCandidateSpace,
 ]
 

petab_select/constants.py

@@ -119,11 +119,11 @@ class Method(str, Enum):
     """String literals for model selection methods."""
 
     BACKWARD = 'backward'
-    BIDIRECTIONAL = 'bidirectional'
+    #BIDIRECTIONAL = 'bidirectional'
     BRUTE_FORCE = 'brute_force'
     FAMOS = 'famos'
     FORWARD = 'forward'
-    FORWARD_AND_BACKWARD = 'forward_and_backward'
+    #FORWARD_AND_BACKWARD = 'forward_and_backward'
     LATERAL = 'lateral'
     MOST_DISTANT = 'most_distant'
 
@@ -142,25 +142,25 @@ class Criterion(str, Enum):
 # Methods that move through model space by taking steps away from some model.
 STEPWISE_METHODS = [
     Method.BACKWARD,
-    Method.BIDIRECTIONAL,
+    #Method.BIDIRECTIONAL,
     Method.FORWARD,
-    Method.FORWARD_AND_BACKWARD,
+    #Method.FORWARD_AND_BACKWARD,
     Method.LATERAL,
 ]
 # Methods that require an initial model.
 INITIAL_MODEL_METHODS = [
     Method.BACKWARD,
-    Method.BIDIRECTIONAL,
+    #Method.BIDIRECTIONAL,
     Method.FORWARD,
-    Method.FORWARD_AND_BACKWARD,
+    #Method.FORWARD_AND_BACKWARD,
     Method.LATERAL,
 ]
 
 # Virtual initial models can be used to initialize some initial model methods.
 VIRTUAL_INITIAL_MODEL = 'virtual_initial_model'
 VIRTUAL_INITIAL_MODEL_METHODS = [
     Method.BACKWARD,
-    Method.BIDIRECTIONAL,
+    #Method.BIDIRECTIONAL,
     Method.FORWARD,
-    Method.FORWARD_AND_BACKWARD,
+    #Method.FORWARD_AND_BACKWARD,
 ]

petab_select/ui.py

@@ -134,15 +134,32 @@ def candidates(
     problem.model_space.exclude_model_hashes(
         model_hashes=excluded_model_hashes
     )
-    if do_search:
+    while do_search:
         problem.model_space.search(candidate_space, limit=limit_sent)
 
-    write_summary_tsv(
-        problem=problem,
-        candidate_space=candidate_space,
-        previous_predecessor_model=candidate_space.previous_predecessor_model,
-        predecessor_model=predecessor_model,
-    )
+        write_summary_tsv(
+            problem=problem,
+            candidate_space=candidate_space,
+            previous_predecessor_model=candidate_space.previous_predecessor_model,
+            predecessor_model=predecessor_model,
+        )
+
+        if candidate_space.models:
+            break
+
+        # No models were found. Repeat the search with the same candidate space,
+        # if the candidate space is able to switch methods.
+        # N.B.: candidate spaces that switch methods must raise `StopIteration`
+        # when they stop switching.
+        if candidate_space.governing_method == Method.FAMOS:
+            try:
+                candidate_space.update_after_calibration(
+                    calibrated_models=calibrated_models,
+                    newly_calibrated_models={},
+                    criterion=criterion,
+                )
+            except StopIteration:
+                break
 
     candidate_space.previous_predecessor_model = predecessor_model