Merge branch 'develop' into fix_petab_importer_amici_validation

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.")

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):

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

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])