Fix FAMoS termination; remove other switching methods

petab_select/candidate_space.py

@@ -560,102 +560,6 @@ 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 FamosCandidateSpace(CandidateSpace):
     """The FAMoS method class.
 
@@ -1256,33 +1160,6 @@ 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
-
-
 class LateralCandidateSpace(CandidateSpace):
     """Find models with the same number of estimated parameters."""
 
@@ -1362,10 +1239,8 @@ def _consider_method(self, model):
 candidate_space_classes = [
     ForwardCandidateSpace,
     BackwardCandidateSpace,
-    BidirectionalCandidateSpace,
     LateralCandidateSpace,
     BruteForceCandidateSpace,
-    ForwardAndBackwardCandidateSpace,
     FamosCandidateSpace,
 ]
 

petab_select/constants.py

@@ -48,12 +48,6 @@
 # COMPARED_MODEL_ID = 'compared_'+MODEL_ID
 YAML_FILENAME = 'yaml'
 
-# FORWARD = 'forward'
-# BACKWARD = 'backward'
-# BIDIRECTIONAL = 'bidirectional'
-# LATERAL = 'lateral'
-
-
 # DISTANCES = {
 #    FORWARD: {
 #        'l1': 1,
@@ -70,20 +64,6 @@
 # }
 
 CRITERIA = 'criteria'
-# FIXME remove, change all uses to Enum below
-# AIC = 'AIC'
-# AICC = 'AICc'
-# BIC = 'BIC'
-# AKAIKE_INFORMATION_CRITERION = AIC
-# CORRECTED_AKAIKE_INFORMATION_CRITERION = AICC
-# BAYESIAN_INFORMATION_CRITERION = BIC
-# LH = 'LH'
-# LLH = 'LLH'
-# NLLH = 'NLLH'
-# LIKELIHOOD = LH
-# LOG_LIKELIHOOD = LLH
-# NEGATIVE_LOG_LIKELIHOOD = NLLH
-
 
 PARAMETERS = 'parameters'
 # PARAMETER_ESTIMATE = 'parameter_estimate'
@@ -119,11 +99,9 @@ class Method(str, Enum):
     """String literals for model selection methods."""
 
     BACKWARD = 'backward'
-    BIDIRECTIONAL = 'bidirectional'
     BRUTE_FORCE = 'brute_force'
     FAMOS = 'famos'
     FORWARD = 'forward'
-    FORWARD_AND_BACKWARD = 'forward_and_backward'
     LATERAL = 'lateral'
     MOST_DISTANT = 'most_distant'
 
@@ -142,25 +120,19 @@ class Criterion(str, Enum):
 # Methods that move through model space by taking steps away from some model.
 STEPWISE_METHODS = [
     Method.BACKWARD,
-    Method.BIDIRECTIONAL,
     Method.FORWARD,
-    Method.FORWARD_AND_BACKWARD,
     Method.LATERAL,
 ]
 # Methods that require an initial model.
 INITIAL_MODEL_METHODS = [
     Method.BACKWARD,
-    Method.BIDIRECTIONAL,
     Method.FORWARD,
-    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.FORWARD,
-    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