Skip to content

Commit

Permalink
Fix multi-subspace stepwise moves (#65)
Browse files Browse the repository at this point in the history
  • Loading branch information
dilpath authored Nov 20, 2023
1 parent 2bb3076 commit 7c71053
Showing 1 changed file with 21 additions and 12 deletions.
33 changes: 21 additions & 12 deletions petab_select/model_subspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand All @@ -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.
Expand All @@ -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
)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 7c71053

Please sign in to comment.