From f9a06d4c99aaa09f7bf48599fbc8fb33f9bb221b Mon Sep 17 00:00:00 2001 From: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> Date: Sun, 17 Dec 2023 18:06:06 +0100 Subject: [PATCH] Fix FAMoS termination; remove other switching methods; automatically calibrate uncalibrated predecessor models (#68) Co-authored-by: Doresic --- petab_select/candidate_space.py | 141 ++----------------- petab_select/constants.py | 28 ---- petab_select/criteria.py | 11 +- petab_select/model.py | 32 ++++- petab_select/ui.py | 53 +++++-- test/candidate_space/test_candidate_space.py | 136 +----------------- 6 files changed, 93 insertions(+), 308 deletions(-) diff --git a/petab_select/candidate_space.py b/petab_select/candidate_space.py index d1d60433..8a64af56 100644 --- a/petab_select/candidate_space.py +++ b/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,10 +1030,14 @@ 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] @@ -1137,7 +1045,7 @@ def switch_inner_candidate_space( # 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, ] diff --git a/petab_select/constants.py b/petab_select/constants.py index 908be6b7..ac0b2a73 100644 --- a/petab_select/constants.py +++ b/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, ] diff --git a/petab_select/criteria.py b/petab_select/criteria.py index d15c0f33..1f245a28 100644 --- a/petab_select/criteria.py +++ b/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 diff --git a/petab_select/model.py b/petab_select/model.py index c441f87f..ac0944df 100644 --- a/petab_select/model.py +++ b/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,19 +241,29 @@ 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 @@ -260,15 +271,24 @@ def compute_criterion(self, criterion: Criterion) -> TYPE_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, diff --git a/petab_select/ui.py b/petab_select/ui.py index 89a0e10b..9bb024af 100644 --- a/petab_select/ui.py +++ b/petab_select/ui.py @@ -1,3 +1,4 @@ +import copy from pathlib import Path from typing import Dict, List, Optional, Union @@ -84,10 +85,29 @@ def candidates( candidate_space.exclude_hashes(calibrated_models) # Set the predecessor model to the previous predecessor model. + predecessor_model = candidate_space.previous_predecessor_model + + # If the predecessor model has not yet been calibrated, then calibrate it. + if ( + predecessor_model.get_criterion( + criterion, + raise_on_failure=False, + ) + is None + ): + candidate_space.models = [copy.deepcopy(predecessor_model)] + # Dummy zero likelihood, which the predecessor model will + # improve on after it's actually calibrated. + predecessor_model.set_criterion(Criterion.LH, 0.0) + return candidate_space + + # Exclude the calibrated predecessor model. + if not candidate_space.excluded(predecessor_model): + candidate_space.exclude(predecessor_model) + # Set the new predecessor_model from the initial model or # by calling ui.best to find the best model to jump to if # this is not the first step of the search. - predecessor_model = candidate_space.previous_predecessor_model if newly_calibrated_models: predecessor_model = problem.get_best( newly_calibrated_models.values(), @@ -142,15 +162,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 diff --git a/test/candidate_space/test_candidate_space.py b/test/candidate_space/test_candidate_space.py index 9a993ef6..523f42d7 100644 --- a/test/candidate_space/test_candidate_space.py +++ b/test/candidate_space/test_candidate_space.py @@ -5,9 +5,13 @@ from more_itertools import one import petab_select -from petab_select.candidate_space import ( # BackwardCandidateSpace,; BruteForceCandidateSpace,; ForwardCandidateSpace,; ForwardAndBackwardCandidateSpace,; LateralCandidateSpace, - BidirectionalCandidateSpace, -) + +# from petab_select.candidate_space import ( +# BackwardCandidateSpace, +# BruteForceCandidateSpace, +# ForwardCandidateSpace, +# LateralCandidateSpace, +# ) from petab_select.constants import ( ESTIMATE, MODEL_SUBSPACE_ID, @@ -107,129 +111,3 @@ def model_space(calibrated_model_space) -> pd.DataFrame: df = get_model_space_df(df) model_space = ModelSpace.from_df(df) return model_space - - -def test_bidirectional( - model_space, calibrated_model_space, ordered_model_parameterizations -): - criterion = Criterion.AIC - model_id_length = one( - set([len(model_id) for model_id in calibrated_model_space]) - ) - - candidate_space = BidirectionalCandidateSpace() - calibrated_models = [] - - # Perform bidirectional search until no more models are found. - search_iterations = 0 - while True: - new_calibrated_models = [] - search_iterations += 1 - - # Get models. - model_space.search(candidate_space) - - # Calibrate models. - for model in candidate_space.models: - model_id = model.model_subspace_id[-model_id_length:] - model.set_criterion( - criterion=criterion, value=calibrated_model_space[model_id] - ) - new_calibrated_models.append(model) - - # End if no more models were found. - if not new_calibrated_models: - break - - # Get next predecessor model as best of new models. - best_new_model = None - for model in new_calibrated_models: - if best_new_model is None: - best_new_model = model - continue - if default_compare( - model0=best_new_model, model1=model, criterion=criterion - ): - best_new_model = model - - # Set next predecessor and exclusions. - calibrated_model_hashes = [ - model.get_hash() for model in calibrated_models - ] - candidate_space.reset( - predecessor_model=best_new_model, - exclusions=calibrated_model_hashes, - ) - - # exclude calibrated model hashes from model space too? - model_space.exclude_model_hashes(model_hashes=calibrated_model_hashes) - - # Check that expected models are found at each iteration. - ( - good_model_parameterizations_ascending, - bad_model_parameterizations, - ) = ordered_model_parameterizations - search_iteration = 0 - for history_item in candidate_space.method_history: - models = history_item[MODELS] - if not models: - continue - model_parameterizations = [ - model.model_subspace_id[-5:] for model in models - ] - - good_model_parameterization = good_model_parameterizations_ascending[ - search_iteration - ] - # The expected good model was found. - assert good_model_parameterization in model_parameterizations - model_parameterizations.remove(good_model_parameterization) - - if search_iteration == 0: - # All parameterizations have been correctly identified and removed. - assert not model_parameterizations - search_iteration += 1 - continue - - previous_model_parameterization = ( - good_model_parameterizations_ascending[search_iteration - 1] - ) - - # The expected bad model is also found. - # If a bad model is the same dimension and also represents a similar stepwise move away from the previous - # model parameterization, it should also be in the parameterizations. - for bad_model_parameterization in bad_model_parameterizations: - # Skip if different dimensions - if sum(map(int, bad_model_parameterization)) != sum( - map(int, good_model_parameterization) - ): - continue - # Skip if different distances from previous model parameterization - if sum( - [ - a != b - for a, b in zip( - bad_model_parameterization, - previous_model_parameterization, - ) - ] - ) != sum( - [ - a != b - for a, b in zip( - good_model_parameterization, - previous_model_parameterization, - ) - ] - ): - continue - assert bad_model_parameterization in model_parameterizations - model_parameterizations.remove(bad_model_parameterization) - - # All parameterizations have been correctly identified and removed. - assert not model_parameterizations - search_iteration += 1 - - # End test if all good models were found in the correct order. - if search_iteration >= len(good_model_parameterizations_ascending): - break