fix: adds logic to ensure bound transformation on log-space is finite (…

…#595)
pybop-team · Dec 13, 2024 · b712628 · b712628
1 parent 1095016
commit b712628
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,7 @@
 
 ## Bug Fixes
 
+- [#595](https://github.com/pybop-team/PyBOP/pull/595) - Fixes non-finite LogTransformed bounds for indices of zero.
 - [#561](https://github.com/pybop-team/PyBOP/pull/561) - Bug fixes the sign of the SciPy cost logs for maximised costs.
 - [#505](https://github.com/pybop-team/PyBOP/pull/505) - Bug fixes for `LogPosterior` with transformed `GaussianLogLikelihood` likelihood.
 

diff --git a/pybop/parameters/parameter.py b/pybop/parameters/parameter.py
@@ -4,7 +4,7 @@
 
 import numpy as np
 
-from pybop import ComposedTransformation, IdentityTransformation
+from pybop import ComposedTransformation, IdentityTransformation, LogTransformation
 from pybop._utils import is_numeric
 
 Inputs = dict[str, float]
@@ -57,8 +57,8 @@ def __init__(
         self.transformation = transformation
         self.applied_prior_bounds = False
         self.bounds = None
-        self.lower_bounds = None
-        self.upper_bounds = None
+        self.lower_bound = None
+        self.upper_bound = None
         self.set_bounds(bounds)
         self.margin = 1e-4
 
@@ -340,19 +340,26 @@ def get_bounds(self, apply_transform: bool = False) -> dict:
         """
         bounds = {"lower": [], "upper": []}
         for param in self.param.values():
-            if param.bounds is None:
-                lower, upper = -np.inf, np.inf
-            else:
-                lower, upper = param.bounds
-                if apply_transform and param.transformation is not None:
-                    lower = float(param.transformation.to_search(param.bounds[0]))
-                    upper = float(param.transformation.to_search(param.bounds[1]))
-                    if np.isnan(lower) or np.isnan(upper):
-                        raise ValueError(
-                            "Transformed bounds resulted in NaN values.\n"
-                            "If you've not applied bounds, this is due to the defaults applied from the prior distribution,\n"
-                            "consider bounding the parameters to avoid this error."
-                        )
+            lower, upper = param.bounds or (-np.inf, np.inf)
+
+            if (
+                apply_transform
+                and param.bounds is not None
+                and param.transformation is not None
+            ):
+                if isinstance(param.transformation, LogTransformation):
+                    param.bounds = [
+                        b + np.finfo(float).eps if b == 0 else b for b in param.bounds
+                    ]
+                lower = float(param.transformation.to_search(param.bounds[0]))
+                upper = float(param.transformation.to_search(param.bounds[1]))
+
+                if np.isnan(lower) or np.isnan(upper):
+                    raise ValueError(
+                        "Transformed bounds resulted in NaN values.\n"
+                        "If you've not applied bounds, this is due to the defaults applied from the prior distribution,\n"
+                        "consider bounding the parameters to avoid this error."
+                    )
 
             bounds["lower"].append(lower)
             bounds["upper"].append(upper)

diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py
@@ -203,6 +203,37 @@ def test_parameters_naming(self, parameter):
 
     @pytest.mark.unit
     def test_parameters_transformation(self):
+        # Construct params
+        params = pybop.Parameters(
+            pybop.Parameter(
+                "LogParam",
+                bounds=[0, 1],
+                transformation=pybop.LogTransformation(),
+            ),
+            pybop.Parameter(
+                "ScaledParam",
+                bounds=[0, 1],
+                transformation=pybop.ScaledTransformation(1, 0.5),
+            ),
+            pybop.Parameter(
+                "IdentityParam",
+                bounds=[0, 1],
+                transformation=pybop.IdentityTransformation(),
+            ),
+            pybop.Parameter(
+                "UnitHyperParam",
+                bounds=[0, 1],
+                transformation=pybop.UnitHyperCube(1, 2),
+            ),
+        )
+
+        # Test transformed bounds
+        bounds = params.get_bounds(apply_transform=True)
+        np.testing.assert_allclose(
+            bounds["lower"], [np.log(np.finfo(float).eps), 0.5, 0, -1]
+        )
+        np.testing.assert_allclose(bounds["upper"], [np.log(1), 1.5, 1, 0])
+
         # Test unbounded transformation causes ValueError due to NaN
         params = pybop.Parameters(
             pybop.Parameter(