Merge branch 'refs/heads/develop' into 25.1

BradyPlanden · BradyPlanden · commit a29db68dd883 · 2025-02-04T15:48:29.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,7 +23,6 @@
 ## Optimisations
 
 - [#638](https://github.com/pybop-team/PyBOP/pull/638) - Allows the problem class to accept any domain name.
-- [#593](https://github.com/pybop-team/PyBOP/pull/593) - Enables `ParameterSet` to systematically return a `pybamm.ParameterValues` object within the model class.
 - [#618](https://github.com/pybop-team/PyBOP/pull/618) - Adds Mean Absolute Error (MAE) and Mean Squared Error (MSE) costs.
 - [#601](https://github.com/pybop-team/PyBOP/pull/601) - Deprecates `MultiOptimisationResult` by merging with `OptimisationResult`.
 - [#600](https://github.com/pybop-team/PyBOP/pull/600) - Removes repetitious functionality within the cost classes.
@@ -34,6 +33,9 @@
 
 ## Breaking Changes
 
+- [#656](https://github.com/pybop-team/PyBOP/pull/656) - Completes `ParameterSet` changes from #593 and aligns the simulation options in `model.predict` with the model properties such as the solver.
+- [#593](https://github.com/pybop-team/PyBOP/pull/593) - Enables `ParameterSet` to systematically return a `pybamm.ParameterValues` object within the model class.
+
 # [v24.12](https://github.com/pybop-team/PyBOP/tree/v24.12) - 2024-12-21
 
 ## Features
diff --git a/benchmarks/benchmark_model.py b/benchmarks/benchmark_model.py
@@ -23,7 +23,7 @@ def setup(self, model, parameter_set):
         set_random_seed()
 
         # Create model instance
-        self.model = model(parameter_set=pybop.ParameterSet.pybamm(parameter_set))
+        self.model = model(parameter_set=pybop.ParameterSet(parameter_set))
 
         # Define fitting parameters
         parameters = pybop.Parameters(
diff --git a/benchmarks/benchmark_optim_construction.py b/benchmarks/benchmark_optim_construction.py
@@ -25,7 +25,7 @@ def setup(self, model, parameter_set, optimiser):
         set_random_seed()
 
         # Create model instance
-        model_instance = model(parameter_set=pybop.ParameterSet.pybamm(parameter_set))
+        model_instance = model(parameter_set=pybop.ParameterSet(parameter_set))
 
         # Define fitting parameters
         parameters = pybop.Parameters(
diff --git a/benchmarks/benchmark_parameterisation.py b/benchmarks/benchmark_parameterisation.py
@@ -35,7 +35,7 @@ def setup(self, model, parameter_set, optimiser):
         set_random_seed()
 
         # Create model instance
-        params = pybop.ParameterSet.pybamm(parameter_set)
+        params = pybop.ParameterSet(parameter_set)
         params.update(
             {
                 "Negative electrode active material volume fraction": 0.63,
diff --git a/benchmarks/benchmark_track_parameterisation.py b/benchmarks/benchmark_track_parameterisation.py
@@ -35,7 +35,7 @@ def setup(self, model, parameter_set, optimiser):
         set_random_seed()
 
         # Create model instance
-        params = pybop.ParameterSet.pybamm(parameter_set)
+        params = pybop.ParameterSet(parameter_set)
         params.update(
             {
                 "Negative electrode active material volume fraction": 0.63,
diff --git a/examples/scripts/comparison_examples/gradient_descent.py b/examples/scripts/comparison_examples/gradient_descent.py
@@ -4,9 +4,8 @@
 import pybop
 
 # Define model and use high-performant solver for sensitivities
-solver = pybamm.IDAKLUSolver()
 parameter_set = pybop.ParameterSet("Chen2020")
-model = pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
+model = pybop.lithium_ion.SPM(parameter_set=parameter_set)
 
 # Fitting parameters
 parameters = pybop.Parameters(
@@ -36,6 +35,7 @@
 )
 
 # Generate problem, cost function, and optimisation class
+model.solver = pybamm.IDAKLUSolver()
 problem = pybop.FittingProblem(model, parameters, dataset)
 cost = pybop.RootMeanSquaredError(problem)
 optim = pybop.GradientDescent(
diff --git a/examples/scripts/comparison_examples/grouped_SPMe.py b/examples/scripts/comparison_examples/grouped_SPMe.py
@@ -41,6 +41,7 @@
     parameter_set=parameter_set,
     options=model_options,
 )
+time_domain_SPMe.solver = time_domain_SPMe.pybamm_model.default_solver
 simulation = time_domain_SPMe.predict(
     initial_state=initial_state, experiment=experiment
 )
diff --git a/examples/scripts/comparison_examples/scipy_minimize.py b/examples/scripts/comparison_examples/scipy_minimize.py
@@ -5,8 +5,7 @@
 
 # Define model
 parameter_set = pybop.ParameterSet("Chen2020")
-solver = pybamm.IDAKLUSolver()
-model = pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
+model = pybop.lithium_ion.SPM(parameter_set=parameter_set)
 
 # Fitting parameters
 parameters = pybop.Parameters(
@@ -36,6 +35,7 @@
 )
 
 # Generate problem, cost function, and optimisation class
+model.solver = pybamm.IDAKLUSolver()
 problem = pybop.FittingProblem(model, parameters, dataset)
 cost = pybop.SumSquaredError(problem)
 optim = pybop.SciPyMinimize(
diff --git a/examples/scripts/comparison_examples/simulated_annealing.py b/examples/scripts/comparison_examples/simulated_annealing.py
@@ -4,9 +4,8 @@
 import pybop
 
 # Define model and use high-performant solver for sensitivities
-solver = pybamm.IDAKLUSolver()
 parameter_set = pybop.ParameterSet.pybamm("Chen2020")
-model = pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
+model = pybop.lithium_ion.SPM(parameter_set=parameter_set)
 
 # Fitting parameters
 parameters = pybop.Parameters(
@@ -38,6 +37,7 @@
 )
 
 # Generate problem, cost function, and optimisation class
+model.solver = pybamm.IDAKLUSolver()
 problem = pybop.FittingProblem(model, parameters, dataset)
 cost = pybop.RootMeanSquaredError(problem)
 optim = pybop.SimulatedAnnealing(
diff --git a/examples/scripts/getting_started/multi_start_optimisation.py b/examples/scripts/getting_started/multi_start_optimisation.py
@@ -4,9 +4,8 @@
 import pybop
 
 # Define model and use the high-performant IDAKLU solver for sensitivities
-solver = pybamm.IDAKLUSolver(atol=1e-5, rtol=1e-5)
 parameter_set = pybop.ParameterSet("Chen2020")
-model = pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
+model = pybop.lithium_ion.SPM(parameter_set=parameter_set)
 
 # Fitting parameters
 parameters = pybop.Parameters(
@@ -36,6 +35,7 @@
 )
 
 # Generate problem, cost function classes
+model.solver = pybamm.IDAKLUSolver(atol=1e-5, rtol=1e-5)
 problem = pybop.FittingProblem(model, parameters, dataset)
 cost = pybop.RootMeanSquaredError(problem)
 
diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py
@@ -340,6 +340,7 @@ def clear(self):
         self._built_initial_soc = None
         self._mesh = None
         self._disc = None
+        self._geometry = self._unprocessed_model.default_geometry
 
     def classify_parameters(
         self, parameters: Optional[Parameters] = None, inputs: Optional[Inputs] = None
@@ -377,7 +378,7 @@ def classify_parameters(
         for key in standard_parameters.keys():
             self._parameter_set[key] = "[input]"
 
-        # Clear any built model, update the parameter set and geometry if rebuild required
+        # Clear any built model and update the parameter set if rebuild required
         if rebuild_parameters:
             self._sensitivities_available = False
             requires_rebuild = False
@@ -387,7 +388,6 @@ def classify_parameters(
                     requires_rebuild = True
             if requires_rebuild:
                 self.clear()
-                self._geometry = self.pybamm_model.default_geometry
                 # Update both the active and unprocessed parameter sets for consistency
                 self._parameter_set.update(rebuild_parameters)
                 self._unprocessed_parameter_set.update(rebuild_parameters)
@@ -733,17 +733,28 @@ def predict(
         ):
             raise ValueError("These parameter values are infeasible.")
 
+        simulation_args = {
+            "model": self._unprocessed_model,
+            "geometry": self._unprocessed_model.default_geometry,
+            "parameter_values": parameter_set,
+            "submesh_types": self._submesh_types,
+            "var_pts": self._var_pts,
+            "spatial_methods": self._spatial_methods,
+            "solver": self._solver,
+        }
+
         if experiment is not None:
             self._pybamm_solution = pybamm.Simulation(
-                model=self._unprocessed_model,
-                experiment=experiment,
-                parameter_values=parameter_set,
+                **simulation_args, experiment=experiment
             ).solve(initial_soc=initial_state)
         elif t_eval is not None:
-            self._pybamm_solution = pybamm.Simulation(
-                model=self._unprocessed_model,
-                parameter_values=parameter_set,
-            ).solve(t_eval=t_eval, initial_soc=initial_state)
+            self._pybamm_solution = pybamm.Simulation(**simulation_args).solve(
+                initial_soc=initial_state,
+                t_eval=[t_eval[0], t_eval[-1]]
+                if isinstance(self._solver, IDAKLUSolver)
+                else t_eval,
+                t_interp=t_eval if self._solver.supports_interp else None,
+            )
         else:
             raise ValueError(
                 "The predict method requires either an experiment or t_eval "
@@ -861,16 +872,20 @@ def new_copy(self):
         """
         model_class = type(self)
         if self.pybamm_model is None:
-            model_args = {"parameter_set": self._parameter_set.copy()}
+            model_args = {
+                "name": self.name,
+                "parameter_set": self._parameter_set.copy(),
+            }
         else:
             model_args = {
-                "options": self._unprocessed_model.options,
+                "name": self.name,
+                "options": copy.copy(self._unprocessed_model.options),
+                "geometry": self._unprocessed_model.default_geometry,
                 "parameter_set": self._unprocessed_parameter_set.copy(),
-                "geometry": self.pybamm_model.default_geometry.copy(),
-                "submesh_types": self.pybamm_model.default_submesh_types.copy(),
-                "var_pts": self.pybamm_model.default_var_pts.copy(),
-                "spatial_methods": self.pybamm_model.default_spatial_methods.copy(),
-                "solver": self.pybamm_model.default_solver.copy(),
+                "submesh_types": self.submesh_types.copy(),
+                "var_pts": self.var_pts.copy(),
+                "spatial_methods": self.spatial_methods.copy(),
+                "solver": self.solver.copy(),
                 "eis": copy.copy(self.eis),
             }
 
diff --git a/pybop/parameters/parameter_set.py b/pybop/parameters/parameter_set.py
@@ -1,3 +1,4 @@
+import copy
 import json
 import types
 from numbers import Number
@@ -229,6 +230,14 @@ def is_json_serializable(self, value):
         except (TypeError, OverflowError):
             return False
 
+    def copy(self):
+        new_copy = ParameterSet(
+            parameter_set=self.parameter_values.copy(),
+            json_path=copy.copy(self._json_path),
+            formation_concentrations=copy.copy(self.formation_concentrations),
+        )
+        return new_copy
+
     @classmethod
     def pybamm(cls, name):
         """
diff --git a/tests/integration/test_eis_parameterisation.py b/tests/integration/test_eis_parameterisation.py
@@ -22,7 +22,7 @@ def setup(self):
 
     @pytest.fixture
     def model(self):
-        parameter_set = pybop.ParameterSet.pybamm("Chen2020")
+        parameter_set = pybop.ParameterSet("Chen2020")
         x = self.ground_truth
         parameter_set.update(
             {
diff --git a/tests/integration/test_jax_parameterisations.py b/tests/integration/test_jax_parameterisations.py
@@ -1,6 +1,6 @@
 import numpy as np
-import pybamm
 import pytest
+from pybamm import IDAKLUSolver
 
 import pybop
 
@@ -21,16 +21,15 @@ def setup(self):
 
     @pytest.fixture
     def model(self):
-        parameter_set = pybop.ParameterSet.pybamm("Chen2020")
+        parameter_set = pybop.ParameterSet("Chen2020")
         x = self.ground_truth
         parameter_set.update(
             {
                 "Negative electrode active material volume fraction": x[0],
                 "Positive electrode active material volume fraction": x[1],
             }
         )
-        solver = pybamm.IDAKLUSolver()
-        return pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
+        return pybop.lithium_ion.SPM(parameter_set=parameter_set)
 
     @pytest.fixture
     def parameters(self):
@@ -96,6 +95,7 @@ def optim(self, optimiser, model, parameters, cost_cls, init_soc):
         )
 
         # Define the problem
+        model.solver = IDAKLUSolver()
         problem = pybop.FittingProblem(model, parameters, dataset)
 
         # Construct the cost
diff --git a/tests/integration/test_model_experiment_changes.py b/tests/integration/test_model_experiment_changes.py
@@ -39,7 +39,7 @@ def parameters(self, request):
     def test_changing_experiment(self, parameters):
         # Change the experiment and check that the results are different.
 
-        parameter_set = pybop.ParameterSet.pybamm("Chen2020")
+        parameter_set = pybop.ParameterSet("Chen2020")
         parameter_set.update(parameters.as_dict("true"))
         initial_state = {"Initial SoC": 0.5}
         model = pybop.lithium_ion.SPM(parameter_set=parameter_set)
@@ -69,7 +69,7 @@ def test_changing_experiment(self, parameters):
     def test_changing_model(self, parameters):
         # Change the model and check that the results are different.
 
-        parameter_set = pybop.ParameterSet.pybamm("Chen2020")
+        parameter_set = pybop.ParameterSet("Chen2020")
         parameter_set.update(parameters.as_dict("true"))
         initial_state = {"Initial SoC": 0.5}
         experiment = pybop.Experiment(["Charge at 1C until 4.1 V (2 seconds period)"])
@@ -109,7 +109,7 @@ def final_cost(self, solution, model, parameters):
         return results.final_cost
 
     def test_multi_fitting_problem(self):
-        parameter_set = pybop.ParameterSet.pybamm("Chen2020")
+        parameter_set = pybop.ParameterSet("Chen2020")
         parameters = pybop.Parameter(
             "Negative electrode active material volume fraction",
             prior=pybop.Gaussian(0.68, 0.05),
diff --git a/tests/integration/test_monte_carlo.py b/tests/integration/test_monte_carlo.py
@@ -1,6 +1,6 @@
 import numpy as np
-import pybamm
 import pytest
+from pybamm import IDAKLUSolver
 
 import pybop
 from pybop import (
@@ -31,16 +31,15 @@ def setup(self):
 
     @pytest.fixture
     def model(self):
-        parameter_set = pybop.ParameterSet.pybamm("Chen2020")
+        parameter_set = pybop.ParameterSet("Chen2020")
         x = self.ground_truth
         parameter_set.update(
             {
                 "Negative electrode active material volume fraction": x[0],
                 "Positive electrode active material volume fraction": x[1],
             }
         )
-        solver = pybamm.IDAKLUSolver()
-        return pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
+        return pybop.lithium_ion.SPM(parameter_set=parameter_set)
 
     @pytest.fixture
     def parameters(self):
@@ -80,6 +79,7 @@ def log_posterior(self, model, parameters, init_soc):
         )
 
         # Define the posterior to optimise
+        model.solver = IDAKLUSolver()
         problem = pybop.FittingProblem(model, parameters, dataset)
         likelihood = pybop.GaussianLogLikelihood(problem, sigma0=0.002 * 1.2)
         return pybop.LogPosterior(likelihood)
@@ -128,7 +128,7 @@ def test_sampling_spm(self, quick_sampler, log_posterior, map_estimate):
         # Assert both final sample and posterior mean
         x = np.mean(chains, axis=1)
         for i in range(len(x)):
-            np.testing.assert_allclose(x[i], self.ground_truth, atol=1.5e-2)
+            np.testing.assert_allclose(x[i], self.ground_truth, atol=1.6e-2)
 
     def get_data(self, model, init_soc):
         initial_state = {"Initial SoC": init_soc}
diff --git a/tests/integration/test_observer_parameterisation.py b/tests/integration/test_observer_parameterisation.py
@@ -1,6 +1,6 @@
 import numpy as np
-import pybamm
 import pytest
+from pybamm import IDAKLUSolver
 
 import pybop
 
@@ -22,14 +22,14 @@ def setup(self):
 
     @pytest.fixture
     def parameter_set(self):
-        return pybamm.ParameterValues(
+        return pybop.ParameterSet.to_pybamm(
             {"k": self.ground_truth[0], "y0": self.ground_truth[1]}
         )
 
     @pytest.fixture
     def model(self, parameter_set):
         return pybop.ExponentialDecayModel(
-            parameter_set=parameter_set, solver=pybamm.IDAKLUSolver, n_states=1
+            parameter_set=parameter_set, solver=IDAKLUSolver, n_states=1
         )
 
     @pytest.fixture
diff --git a/tests/integration/test_optimisation_options.py b/tests/integration/test_optimisation_options.py
@@ -23,7 +23,7 @@ def setup(self):
 
     @pytest.fixture
     def model(self):
-        parameter_set = pybop.ParameterSet.pybamm("Chen2020")
+        parameter_set = pybop.ParameterSet("Chen2020")
         x = self.ground_truth
         parameter_set.update(
             {
diff --git a/tests/integration/test_spm_parameterisations.py b/tests/integration/test_spm_parameterisations.py
diff --git a/tests/integration/test_thevenin_parameterisation.py b/tests/integration/test_thevenin_parameterisation.py
diff --git a/tests/integration/test_weighted_cost.py b/tests/integration/test_weighted_cost.py

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ def setup(self, model, parameter_set, optimiser):`
`35`	`35`	`set_random_seed()`
`36`	`36`
`37`	`37`	`# Create model instance`
`38`		`- params = pybop.ParameterSet.pybamm(parameter_set)`
	`38`	`+ params = pybop.ParameterSet(parameter_set)`
`39`	`39`	`params.update(`
`40`	`40`	`{`
`41`	`41`	`"Negative electrode active material volume fraction": 0.63,`
Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,7 @@`
`41`	`41`	`parameter_set=parameter_set,`
`42`	`42`	`options=model_options,`
`43`	`43`	`)`
	`44`	`+time_domain_SPMe.solver = time_domain_SPMe.pybamm_model.default_solver`
`44`	`45`	`simulation = time_domain_SPMe.predict(`
`45`	`46`	`initial_state=initial_state, experiment=experiment`
`46`	`47`	`)`