Fix FAMoS termination; remove other switching methods; automatically …

…calibrate uncalibrated predecessor models (#68)

Co-authored-by: Doresic <[email protected]>

petab_select/candidate_space.py

@@ -569,102 +569,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.
 
@@ -896,7 +800,7 @@ def update_after_calibration(
             logging.info("Switching method")
             self.switch_method(calibrated_models=calibrated_models)
             self.switch_inner_candidate_space(
-                calibrated_models=calibrated_models,
+                exclusions=list(calibrated_models),
             )
             logging.info(
                 "Method switched to ", self.inner_candidate_space.method
@@ -911,7 +815,7 @@ def update_from_newly_calibrated_models(
     ) -> bool:
         """Update ``self.best_models`` with the latest ``newly_calibrated_models``
         and determine if there was a new best model. If so, return
-        ``True``. ``False`` otherwise."""
+        ``False``. ``True`` otherwise."""
 
         go_into_switch_method = True
         for newly_calibrated_model in newly_calibrated_models.values():
@@ -1126,18 +1030,22 @@ def update_method(self, method: Method):
 
     def switch_inner_candidate_space(
         self,
-        calibrated_models: Dict[str, Model],
+        exclusions: List[str],
     ):
-        """Switch ``self.inner_candidate_space`` to the candidate space of
-        the current self.method."""
+        """Switch the inner candidate space to match the current method.
+
+        Args:
+            exclusions:
+                Hashes of excluded models.
+        """
 
         # if self.method != Method.MOST_DISTANT:
         self.inner_candidate_space = self.inner_candidate_spaces[self.method]
         # reset the next inner candidate space with the current history of all
         # calibrated models
         self.inner_candidate_space.reset(
             predecessor_model=self.predecessor_model,
-            exclusions=calibrated_models,
+            exclusions=exclusions,
         )
 
     def jump_to_most_distant(
@@ -1265,33 +1173,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."""
 
@@ -1371,10 +1252,8 @@ def _consider_method(self, model):
 candidate_space_classes = [
     ForwardCandidateSpace,
     BackwardCandidateSpace,
-    BidirectionalCandidateSpace,
     LateralCandidateSpace,
     BruteForceCandidateSpace,
-    ForwardAndBackwardCandidateSpace,
     FamosCandidateSpace,
 ]
 

petab_select/constants.py

@@ -49,12 +49,6 @@
 # COMPARED_MODEL_ID = 'compared_'+MODEL_ID
 YAML_FILENAME = 'yaml'
 
-# FORWARD = 'forward'
-# BACKWARD = 'backward'
-# BIDIRECTIONAL = 'bidirectional'
-# LATERAL = 'lateral'
-
-
 # DISTANCES = {
 #    FORWARD: {
 #        'l1': 1,
@@ -71,20 +65,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'
@@ -121,14 +101,12 @@ class Method(str, Enum):
 
     #: The backward stepwise method.
     BACKWARD = 'backward'
-    BIDIRECTIONAL = 'bidirectional'
     #: The brute-force method.
     BRUTE_FORCE = 'brute_force'
     #: The FAMoS method.
     FAMOS = 'famos'
     #: The forward stepwise method.
     FORWARD = 'forward'
-    FORWARD_AND_BACKWARD = 'forward_and_backward'
     #: The lateral, or swap, method.
     LATERAL = 'lateral'
     #: The jump-to-most-distant-model method.
@@ -155,17 +133,13 @@ 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,
 ]
 
@@ -174,9 +148,7 @@ class Criterion(str, Enum):
 #: Methods that are compatible with a virtual initial model.
 VIRTUAL_INITIAL_MODEL_METHODS = [
     Method.BACKWARD,
-    Method.BIDIRECTIONAL,
     Method.FORWARD,
-    Method.FORWARD_AND_BACKWARD,
 ]
 
 

petab_select/criteria.py

@@ -1,7 +1,6 @@
 """Implementations of model selection criteria."""
 
-import math
-
+import numpy as np
 import petab
 from petab.C import OBJECTIVE_PRIOR_PARAMETERS, OBJECTIVE_PRIOR_TYPE
 
@@ -93,7 +92,7 @@ def get_llh(self) -> float:
         """Get the log-likelihood."""
         llh = self.model.get_criterion(Criterion.LLH, compute=False)
         if llh is None:
-            llh = math.log(self.get_lh())
+            llh = np.log(self.get_lh())
         return llh
 
     def get_lh(self) -> float:
@@ -105,9 +104,9 @@ def get_lh(self) -> float:
         if lh is not None:
             return lh
         elif llh is not None:
-            return math.exp(llh)
+            return np.exp(llh)
         elif nllh is not None:
-            return math.exp(-1 * nllh)
+            return np.exp(-1 * nllh)
 
         raise ValueError(
             'Please supply the likelihood (LH) or a compatible transformation. Compatible transformations: log(LH), -log(LH).'
@@ -237,4 +236,4 @@ def calculate_bic(
     Returns:
         The BIC value.
     """
-    return n_estimated * math.log(n_measurements + n_priors) + 2 * nllh
+    return n_estimated * np.log(n_measurements + n_priors) + 2 * nllh

petab_select/model.py

@@ -229,6 +229,7 @@ def get_criterion(
         self,
         criterion: Criterion,
         compute: bool = True,
+        raise_on_failure: bool = True,
     ) -> Union[TYPE_CRITERION, None]:
         """Get a criterion value for the model.
 
@@ -240,35 +241,54 @@ def get_criterion(
                 attributes. For example, if the ``'AIC'`` criterion is requested, this
                 can be computed from a predetermined model likelihood and its
                 number of estimated parameters.
+            raise_on_failure:
+                Whether to raise a `ValueError` if the criterion could not be
+                computed. If `False`, `None` is returned.
 
         Returns:
             The criterion value, or `None` if it is not available.
             TODO check for previous use of this method before `.get` was used
         """
         if criterion not in self.criteria and compute:
-            self.compute_criterion(criterion=criterion)
+            self.compute_criterion(
+                criterion=criterion,
+                raise_on_failure=raise_on_failure,
+            )
             # value = self.criterion_computer(criterion=id)
             # self.set_criterion(id=id, value=value)
 
         return self.criteria.get(criterion, None)
 
-    def compute_criterion(self, criterion: Criterion) -> TYPE_CRITERION:
+    def compute_criterion(
+        self,
+        criterion: Criterion,
+        raise_on_failure: bool = True,
+    ) -> TYPE_CRITERION:
         """Compute a criterion value for the model.
 
         The value will also be stored, which will overwrite any previously stored value
         for the criterion.
 
         Args:
             criterion:
-                The criterion to compute
+                The ID of the criterion
                 (e.g. :obj:`petab_select.constants.Criterion.AIC`).
+            raise_on_failure:
+                Whether to raise a `ValueError` if the criterion could not be
+                computed. If `False`, `None` is returned.
 
         Returns:
             The criterion value.
         """
-        criterion_value = self.criterion_computer(criterion)
-        self.set_criterion(criterion, criterion_value)
-        return criterion_value
+        try:
+            criterion_value = self.criterion_computer(criterion)
+            self.set_criterion(criterion, criterion_value)
+            result = criterion_value
+        except ValueError:
+            if raise_on_failure:
+                raise
+            result = None
+        return result
 
     def set_estimated_parameters(
         self,