diff --git a/pypesto/profile/options.py b/pypesto/profile/options.py index 21cd0b858..f3784db7d 100644 --- a/pypesto/profile/options.py +++ b/pypesto/profile/options.py @@ -66,6 +66,8 @@ def __init__( self.magic_factor_obj_value = magic_factor_obj_value self.whole_path = whole_path + self.validate() + def __getattr__(self, key): """Allow usage of keys like attributes.""" try: @@ -91,3 +93,24 @@ def create_instance( return maybe_options options = ProfileOptions(**maybe_options) return options + + def validate(self): + """Check if options are valid. + + Raises ``ValueError`` if current settings aren't valid. + """ + if self.min_step_size <= 0: + raise ValueError("min_step_size must be > 0.") + if self.max_step_size <= 0: + raise ValueError("max_step_size must be > 0.") + if self.min_step_size > self.max_step_size: + raise ValueError("min_step_size must be <= max_step_size.") + if self.default_step_size <= 0: + raise ValueError("default_step_size must be > 0.") + if self.default_step_size > self.max_step_size: + raise ValueError("default_step_size must be <= max_step_size.") + if self.default_step_size < self.min_step_size: + raise ValueError("default_step_size must be >= min_step_size.") + + if self.magic_factor_obj_value < 0 or self.magic_factor_obj_value >= 1: + raise ValueError("magic_factor_obj_value must be >= 0 and < 1.") diff --git a/pypesto/profile/profile.py b/pypesto/profile/profile.py index bdbaf7ab7..116ef21d8 100644 --- a/pypesto/profile/profile.py +++ b/pypesto/profile/profile.py @@ -89,6 +89,7 @@ def parameter_profile( if profile_options is None: profile_options = ProfileOptions() profile_options = ProfileOptions.create_instance(profile_options) + profile_options.validate() # create a function handle that will be called later to get the next point if isinstance(next_guess_method, str): diff --git a/pypesto/profile/walk_along_profile.py b/pypesto/profile/walk_along_profile.py index 909863cd8..729bb2c35 100644 --- a/pypesto/profile/walk_along_profile.py +++ b/pypesto/profile/walk_along_profile.py @@ -62,14 +62,15 @@ def walk_along_profile( x_now = current_profile.x_path[:, -1] # check if the next profile point needs to be computed - if options.whole_path: - stop_profile = ( - x_now[i_par] * par_direction >= problem.ub_full[[i_par]] - ) + # ... check bounds + if par_direction == -1: + stop_profile = x_now[i_par] <= problem.lb_full[[i_par]] + elif par_direction == 1: + stop_profile = x_now[i_par] >= problem.ub_full[[i_par]] else: - stop_profile = ( - x_now[i_par] * par_direction >= problem.ub_full[[i_par]] - ) or (current_profile.ratio_path[-1] < options.ratio_min) + raise AssertionError("par_direction must be -1 or 1") + if not options.whole_path: + stop_profile |= current_profile.ratio_path[-1] < options.ratio_min if stop_profile: break diff --git a/test/profile/test_profile.py b/test/profile/test_profile.py index 855af1ada..a1371c948 100644 --- a/test/profile/test_profile.py +++ b/test/profile/test_profile.py @@ -7,6 +7,7 @@ from copy import deepcopy import numpy as np +import pytest from numpy.testing import assert_almost_equal import pypesto @@ -412,3 +413,73 @@ def test_approximate_ci(): # bound value assert np.isclose(lb, -3) assert np.isclose(ub, 9) + + +def test_options_valid(): + """Test ProfileOptions validity checks.""" + # default settings are valid + profile.ProfileOptions() + + # try to set invalid values + with pytest.raises(ValueError): + profile.ProfileOptions(default_step_size=-1) + with pytest.raises(ValueError): + profile.ProfileOptions(default_step_size=1, min_step_size=2) + with pytest.raises(ValueError): + profile.ProfileOptions( + default_step_size=2, + min_step_size=1, + ) + with pytest.raises(ValueError): + profile.ProfileOptions( + min_step_size=2, + max_step_size=1, + ) + + +@pytest.mark.parametrize( + "lb,ub", + [(6 * np.ones(5), 10 * np.ones(5)), (-4 * np.ones(5), 1 * np.ones(5))], +) +def test_gh1165(lb, ub): + """Regression test for https://github.com/ICB-DCM/pyPESTO/issues/1165 + + Check profiles with non-symmetric bounds and whole_path=True span the full parameter domain. + """ + obj = rosen_for_sensi(max_sensi_order=1)['obj'] + + problem = pypesto.Problem( + objective=obj, + lb=lb, + ub=ub, + ) + + optimizer = optimize.ScipyOptimizer(options={'maxiter': 10}) + result = optimize.minimize( + problem=problem, + optimizer=optimizer, + n_starts=2, + progress_bar=False, + ) + # just any parameter + par_idx = 1 + profile.parameter_profile( + problem=problem, + result=result, + optimizer=optimizer, + next_guess_method='fixed_step', + profile_index=[par_idx], + progress_bar=False, + profile_options=profile.ProfileOptions( + min_step_size=0.1, + delta_ratio_max=0.05, + default_step_size=0.5, + ratio_min=0.01, + whole_path=True, + ), + ) + # parameter value of the profiled parameter + x_path = result.profile_result.list[0][par_idx]['x_path'][par_idx, :] + # ensure we cover lb..ub + assert x_path[0] == lb[par_idx], (x_path.min(), lb[par_idx]) + assert x_path[-1] == ub[par_idx], (x_path.max(), ub[par_idx])