Merge pull request #505 from pybop-team/bugfixes-for-logposterior-tra…

…nsformations

Fixes for LogPosterior w/ GaussianLogLikelihood

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ## Bug Fixes
 
+- [#505](https://github.com/pybop-team/PyBOP/pull/505) - Bug fixes for `LogPosterior` with transformed `GaussianLogLikelihood` likelihood.
+
 ## Breaking Changes
 
 # [v24.9.0](https://github.com/pybop-team/PyBOP/tree/v24.9.0) - 2024-09-10

examples/scripts/spm_MAP.py

@@ -25,13 +25,15 @@
         bounds=[0.3, 0.8],
         initial_value=0.653,
         true_value=parameter_set["Negative electrode active material volume fraction"],
+        transformation=pybop.LogTransformation(),
     ),
     pybop.Parameter(
         "Positive electrode active material volume fraction",
         prior=pybop.Uniform(0.3, 0.8),
         bounds=[0.4, 0.7],
         initial_value=0.657,
         true_value=parameter_set["Positive electrode active material volume fraction"],
+        transformation=pybop.LogTransformation(),
     ),
 )
 
@@ -44,7 +46,7 @@
         ),
     ]
 )
-values = model.predict(initial_state={"Initial SoC": 0.7}, experiment=experiment)
+values = model.predict(initial_state={"Initial SoC": 0.5}, experiment=experiment)
 corrupt_values = values["Voltage [V]"].data + np.random.normal(
     0, sigma, len(values["Voltage [V]"].data)
 )
@@ -60,7 +62,7 @@
 
 # Generate problem, cost function, and optimisation class
 problem = pybop.FittingProblem(model, parameters, dataset)
-cost = pybop.LogPosterior(pybop.GaussianLogLikelihoodKnownSigma(problem, sigma0=sigma))
+cost = pybop.LogPosterior(pybop.GaussianLogLikelihood(problem))
 optim = pybop.IRPropMin(
     cost,
     sigma0=0.05,

pybop/costs/_likelihoods.py

@@ -141,7 +141,7 @@ def _add_single_sigma(self, index, value):
                 Parameter(
                     f"Sigma for output {index+1}",
                     initial_value=value,
-                    prior=Uniform(0.5 * value, 1.5 * value),
+                    prior=Uniform(1e-3 * value, 1e3 * value),
                     bounds=[1e-8, 3 * value],
                 )
             )
@@ -235,10 +235,12 @@ def __init__(
         super().__init__(problem=log_likelihood.problem)
         self.gradient_step = gradient_step
 
-        # Store the likelihood and prior
+        # Store the likelihood, prior, update parameters and transformation
+        self.join_parameters(log_likelihood.parameters)
         self._log_likelihood = log_likelihood
-        self._parameters = self._log_likelihood.parameters
-        self._has_separable_problem = self._log_likelihood.has_separable_problem
+
+        for attr in ["transformation", "_has_separable_problem"]:
+            setattr(self, attr, getattr(log_likelihood, attr))
 
         if log_prior is None:
             self._prior = JointLogPrior(*self._parameters.priors())

tests/integration/test_optimisation_options.py

@@ -13,8 +13,10 @@ class TestOptimisation:
 
     @pytest.fixture(autouse=True)
     def setup(self):
-        self.ground_truth = np.asarray([0.55, 0.55]) + np.random.normal(
-            loc=0.0, scale=0.05, size=2
+        self.ground_truth = np.clip(
+            np.asarray([0.55, 0.55]) + np.random.normal(loc=0.0, scale=0.05, size=2),
+            a_min=0.4,
+            a_max=0.75,
         )
 
     @pytest.fixture

tests/integration/test_transformation.py

@@ -68,6 +68,7 @@ def noise(self, sigma, values):
             pybop.SumofPower,
             pybop.Minkowski,
             pybop.LogPosterior,
+            pybop.LogPosterior,  # Second for GaussianLogLikelihood
         ]
     )
     def cost_cls(self, request):
@@ -90,13 +91,19 @@ def cost(self, model, parameters, init_soc, cost_cls):
         problem = pybop.FittingProblem(model, parameters, dataset)
 
         # Construct the cost
+        first_map = True
         if cost_cls is pybop.GaussianLogLikelihoodKnownSigma:
             return cost_cls(problem, sigma0=self.sigma0)
         elif cost_cls is pybop.GaussianLogLikelihood:
             return cost_cls(problem)
+        elif cost_cls is pybop.LogPosterior and first_map:
+            first_map = False
+            return cost_cls(log_likelihood=pybop.GaussianLogLikelihood(problem))
         elif cost_cls is pybop.LogPosterior:
             return cost_cls(
-                pybop.GaussianLogLikelihoodKnownSigma(problem, sigma0=self.sigma0)
+                log_likelihood=pybop.GaussianLogLikelihoodKnownSigma(
+                    problem, sigma0=self.sigma0
+                )
             )
         else:
             return cost_cls(problem)
@@ -114,7 +121,7 @@ def test_thevenin_transformation(self, optimiser, cost):
         optim = optimiser(
             cost=cost,
             sigma0=[0.03, 0.03, 1e-3]
-            if isinstance(cost, pybop.GaussianLogLikelihood)
+            if isinstance(cost, (pybop.GaussianLogLikelihood, pybop.LogPosterior))
             else [0.03, 0.03],
             max_unchanged_iterations=35,
             absolute_tolerance=1e-6,
@@ -125,7 +132,7 @@ def test_thevenin_transformation(self, optimiser, cost):
         x, final_cost = optim.run()
 
         # Add sigma0 to ground truth for GaussianLogLikelihood
-        if isinstance(optim.cost, pybop.GaussianLogLikelihood):
+        if isinstance(optim.cost, (pybop.GaussianLogLikelihood, pybop.LogPosterior)):
             self.ground_truth = np.concatenate(
                 (self.ground_truth, np.asarray([self.sigma0]))
             )

tests/unit/test_posterior.py

@@ -68,15 +68,13 @@ def prior(self):
     def test_log_posterior_construction(self, likelihood, prior):
         # Test log posterior construction
         posterior = pybop.LogPosterior(likelihood, prior)
+        keys = likelihood.parameters.keys()
 
         assert posterior._log_likelihood == likelihood
         assert posterior._prior == prior
-
-        # Test log posterior construction without parameters
-        likelihood.parameters.priors = None
-
-        with pytest.raises(TypeError, match="'NoneType' object is not callable"):
-            pybop.LogPosterior(likelihood, log_prior=None)
+        assert posterior.parameters[keys[0]] == likelihood.parameters[keys[0]]
+        assert posterior.has_separable_problem == likelihood.has_separable_problem
+        assert posterior.transformation == likelihood.transformation
 
     @pytest.mark.unit
     def test_log_posterior_construction_no_prior(self, likelihood):