From 7c71053b752840cc43c7b899ae1dacfd85f4f318 Mon Sep 17 00:00:00 2001 From: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> Date: Mon, 20 Nov 2023 12:06:22 +0100 Subject: [PATCH] Fix multi-subspace stepwise moves (#65) --- petab_select/model_subspace.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/petab_select/model_subspace.py b/petab_select/model_subspace.py index 8c0281d5..9b40f696 100644 --- a/petab_select/model_subspace.py +++ b/petab_select/model_subspace.py @@ -232,8 +232,10 @@ def continue_searching( if candidate_space.predecessor_model == VIRTUAL_INITIAL_MODEL: if candidate_space.method == Method.FORWARD: old_estimated_all = set() + old_fixed_all = set(self.parameters) elif candidate_space.method == Method.BACKWARD: old_estimated_all = set(self.parameters) + old_fixed_all = set() else: # Should already be handled elsewhere (e.g. # `self.check_compatibility_stepwise_method`). @@ -242,10 +244,16 @@ def continue_searching( ) else: old_estimated_all = set() + old_fixed_all = set() if isinstance(candidate_space.predecessor_model, Model): old_estimated_all = ( candidate_space.predecessor_model.get_estimated_parameter_ids_all() ) + old_fixed_all = [ + parameter_id + for parameter_id in self.parameters_all + if parameter_id not in old_estimated_all + ] # Parameters that are fixed in the candidate space # predecessor model but are necessarily estimated in this subspace. @@ -264,6 +272,7 @@ def continue_searching( # Parameters related to minimal changes compared to the predecessor model. old_estimated = set(old_estimated_all).intersection(self.can_estimate) + old_fixed = set(old_fixed_all).intersection(self.can_fix) new_must_estimate = set(new_must_estimate_all).intersection( self.parameters ) @@ -304,14 +313,14 @@ def continue_searching( new_must_estimate_all or candidate_space.predecessor_model == VIRTUAL_INITIAL_MODEL ): - # Consider minimal models that have all necessary parameters - # estimated. As this subspace necessarily has more estimated - # parameters than the predecessor model, all parameters that - # can be fixed, will be fixed. + # Consider minimal models that have all necessarily-estimated + # parameters. estimated_parameters = { parameter_id: ESTIMATE - for parameter_id in set(self.parameters).difference( - self.can_fix + for parameter_id in ( + set(self.parameters) + .difference(self.can_fix) + .union(old_estimated) ) } models = self.get_models( @@ -394,14 +403,14 @@ def continue_searching( new_must_fix_all or candidate_space.predecessor_model == VIRTUAL_INITIAL_MODEL ): - # Consider minimal models that have all necessarily fixed parameters. - # As this subspace necessarily has fewer estimated parameters than the - # predecessor model, all parameters that can be estimated, will be - # estimated. + # Consider minimal models that have all necessarily-fixed + # parameters. estimated_parameters = { parameter_id: ESTIMATE - for parameter_id in set(self.parameters).difference( - self.must_fix + for parameter_id in ( + set(self.parameters) + .difference(self.must_fix) + .difference(old_fixed) ) } models = self.get_models(