From c7ef869980b84f01ffdb912a65b187403ce4933b Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Mon, 8 Apr 2024 17:45:00 +0100 Subject: [PATCH 1/4] #251 increment pybamm>23.5, fix examples incorrect model, remove non-integration tests in parameterisation tests --- examples/scripts/spm_adam.py | 2 +- examples/scripts/spm_descent.py | 2 +- pyproject.toml | 2 +- scripts/ci/build_matrix.sh | 2 +- tests/integration/test_parameterisations.py | 45 +++++---------------- 5 files changed, 14 insertions(+), 39 deletions(-) diff --git a/examples/scripts/spm_adam.py b/examples/scripts/spm_adam.py index e0796744c..f6a75354b 100644 --- a/examples/scripts/spm_adam.py +++ b/examples/scripts/spm_adam.py @@ -4,7 +4,7 @@ # Parameter set and model definition parameter_set = pybop.ParameterSet.pybamm("Chen2020") -model = pybop.lithium_ion.SPMe(parameter_set=parameter_set) +model = pybop.lithium_ion.SPM(parameter_set=parameter_set) # Fitting parameters parameters = [ diff --git a/examples/scripts/spm_descent.py b/examples/scripts/spm_descent.py index 3fe078689..5a4568f00 100644 --- a/examples/scripts/spm_descent.py +++ b/examples/scripts/spm_descent.py @@ -4,7 +4,7 @@ # Parameter set and model definition parameter_set = pybop.ParameterSet.pybamm("Chen2020") -model = pybop.lithium_ion.SPMe(parameter_set=parameter_set) +model = pybop.lithium_ion.SPM(parameter_set=parameter_set) # Fitting parameters parameters = [ diff --git a/pyproject.toml b/pyproject.toml index 3663ba670..be1d6d938 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,7 +27,7 @@ classifiers = [ ] requires-python = ">=3.8, <3.13" dependencies = [ - "pybamm>=23.5", + "pybamm>23.5", "numpy>=1.16", "scipy>=1.3", "pints>=0.5", diff --git a/scripts/ci/build_matrix.sh b/scripts/ci/build_matrix.sh index 5e580303d..acc9578f2 100755 --- a/scripts/ci/build_matrix.sh +++ b/scripts/ci/build_matrix.sh @@ -12,7 +12,7 @@ python_version=("3.8" "3.9" "3.10" "3.11" "3.12") os=("ubuntu-latest" "windows-latest" "macos-latest") # This command fetches the last three PyBaMM versions excluding release candidates from PyPI -pybamm_version=($(curl -s https://pypi.org/pypi/pybamm/json | jq -r '.releases | keys[]' | grep -v rc | tail -n 3 | paste -sd " " -)) +pybamm_version=($(curl -s https://pypi.org/pypi/pybamm/json | jq -r '.releases | keys[]' | grep -v rc | tail -n 2 | paste -sd " " -)) # open dict json='{ diff --git a/tests/integration/test_parameterisations.py b/tests/integration/test_parameterisations.py index 4da45fd4d..ae5748235 100644 --- a/tests/integration/test_parameterisations.py +++ b/tests/integration/test_parameterisations.py @@ -94,7 +94,9 @@ def spm_costs(self, model, parameters, cost_class, init_soc): def test_spm_optimisers(self, optimiser, spm_costs): # Some optimisers require a complete set of bounds if optimiser in [pybop.SciPyDifferentialEvolution, pybop.PSO]: - spm_costs.problem.parameters[1].set_bounds([0.375, 0.75]) + spm_costs.problem.parameters[1].set_bounds( + [0.3, 0.8] + ) # Large range to ensure IC within bounds bounds = {"lower": [], "upper": []} for param in spm_costs.problem.parameters: bounds["lower"].append(param.bounds[0]) @@ -106,38 +108,16 @@ def test_spm_optimisers(self, optimiser, spm_costs): parameterisation = pybop.Optimisation( cost=spm_costs, optimiser=optimiser, sigma0=0.05 ) - parameterisation.set_max_unchanged_iterations(iterations=35, threshold=5e-4) + parameterisation.set_max_unchanged_iterations(iterations=35, threshold=1e-5) parameterisation.set_max_iterations(125) initial_cost = parameterisation.cost(spm_costs.x0) - if optimiser in [pybop.CMAES]: - parameterisation.set_f_guessed_tracking(True) - parameterisation.cost.problem.model.allow_infeasible_solutions = False - assert parameterisation._use_f_guessed is True - parameterisation.set_max_iterations(1) - x, final_cost = parameterisation.run() - - parameterisation.set_f_guessed_tracking(False) - parameterisation.set_max_iterations(125) - - x, final_cost = parameterisation.run() - assert parameterisation._max_iterations == 125 - - elif optimiser in [pybop.GradientDescent]: + if optimiser in [pybop.GradientDescent]: if isinstance(spm_costs, pybop.GaussianLogLikelihoodKnownSigma): parameterisation.optimiser.set_learning_rate(1.8e-5) - parameterisation.set_min_iterations(150) else: parameterisation.optimiser.set_learning_rate(0.02) - parameterisation.set_max_iterations(150) - x, final_cost = parameterisation.run() - - elif optimiser in [pybop.SciPyDifferentialEvolution]: - with pytest.raises(ValueError): - parameterisation.optimiser.set_population_size(-5) - - parameterisation.optimiser.set_population_size(5) x, final_cost = parameterisation.run() elif optimiser in [pybop.SciPyMinimize]: @@ -192,7 +172,9 @@ def spm_two_signal_cost(self, parameters, model, cost_class): def test_multiple_signals(self, multi_optimiser, spm_two_signal_cost): # Some optimisers require a complete set of bounds if multi_optimiser in [pybop.SciPyDifferentialEvolution]: - spm_two_signal_cost.problem.parameters[1].set_bounds([0.375, 0.75]) + spm_two_signal_cost.problem.parameters[1].set_bounds( + [0.3, 0.8] + ) # Large range to ensure IC within bounds bounds = {"lower": [], "upper": []} for param in spm_two_signal_cost.problem.parameters: bounds["lower"].append(param.bounds[0]) @@ -208,10 +190,6 @@ def test_multiple_signals(self, multi_optimiser, spm_two_signal_cost): parameterisation.set_max_iterations(125) initial_cost = parameterisation.cost(spm_two_signal_cost.x0) - - if multi_optimiser in [pybop.SciPyDifferentialEvolution]: - parameterisation.optimiser.set_population_size(5) - x, final_cost = parameterisation.run() # Assertions @@ -224,7 +202,7 @@ def test_model_misparameterisation(self, parameters, model, init_soc): # Define two different models with different parameter sets # The optimisation should fail as the models are not the same second_parameter_set = pybop.ParameterSet.pybamm("Ecker2015") - second_model = pybop.lithium_ion.SPM(parameter_set=second_parameter_set) + second_model = pybop.lithium_ion.SPMe(parameter_set=second_parameter_set) # Form dataset solution = self.getdata(second_model, self.ground_truth, init_soc) @@ -237,10 +215,7 @@ def test_model_misparameterisation(self, parameters, model, init_soc): ) # Define the cost to optimise - signal = ["Voltage [V]"] - problem = pybop.FittingProblem( - model, parameters, dataset, signal=signal, init_soc=init_soc - ) + problem = pybop.FittingProblem(model, parameters, dataset, init_soc=init_soc) cost = pybop.RootMeanSquaredError(problem) # Select optimiser From e562e4df130d7914eb365e1f80330d255635ab3c Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Mon, 8 Apr 2024 20:02:05 +0100 Subject: [PATCH 2/4] Add coverage tests, increment citation version, updt changelog --- .github/release_workflow.md | 2 +- CHANGELOG.md | 1 + CITATION.cff | 2 +- .../integration/test_optimisation_options.py | 113 ++++++++++++++++++ tests/integration/test_parameterisations.py | 5 +- tests/unit/test_optimisation.py | 18 ++- 6 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 tests/integration/test_optimisation_options.py diff --git a/.github/release_workflow.md b/.github/release_workflow.md index 5308083b0..a655f9fee 100644 --- a/.github/release_workflow.md +++ b/.github/release_workflow.md @@ -9,7 +9,7 @@ To create a new release, follow these steps: 1. **Prepare the Release:** - Create a new branch for the release (i.e. `v24.XX`) from `develop`. - Increment the following; - - The version number in the `pyproject.toml` file following CalVer versioning. + - The version number in the `pyproject.toml` and `CITATION.cff` files following CalVer versioning. - The`CHANGELOG.md` version with the changes for the new version. - Open a PR to the `main` branch. Once the PR is merged, proceed to the next step. diff --git a/CHANGELOG.md b/CHANGELOG.md index 27b9e0c71..2fb9d5bfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Features +- [#251](https://github.com/pybop-team/PyBOP/pull/251) - Increment PyBaMM > v25.3, remove redundant tests within integration tests, increment citation version, fix examples with incorrect model definitions. - [#250](https://github.com/pybop-team/PyBOP/pull/250) - Adds DFN, MPM, MSMR models and moves multiple construction variables to BaseEChem. Adds exception catch on simulate & simulateS1. - [#241](https://github.com/pybop-team/PyBOP/pull/241) - Adds experimental circuit model fitting notebook with LG M50 data. - [#268](https://github.com/pybop-team/PyBOP/pull/268) - Fixes the GitHub Release artifact uploads, allowing verification of diff --git a/CITATION.cff b/CITATION.cff index 11ce822fd..73dcce742 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -11,5 +11,5 @@ authors: family-names: Courtier - given-names: David family-names: Howey -version: "23.12" # Update this when you release a new version +version: "24.3" # Update this when you release a new version repository-code: 'https://www.github.com/pybop-team/pybop' diff --git a/tests/integration/test_optimisation_options.py b/tests/integration/test_optimisation_options.py new file mode 100644 index 000000000..e026aa40d --- /dev/null +++ b/tests/integration/test_optimisation_options.py @@ -0,0 +1,113 @@ +import numpy as np +import pytest + +import pybop + + +class TestOptimisation: + """ + A class to run integration tests on the Optimisation class. + """ + + @pytest.fixture(autouse=True) + def setup(self): + self.ground_truth = np.array([0.55, 0.55]) + np.random.normal( + loc=0.0, scale=0.05, size=2 + ) + + @pytest.fixture + def model(self): + parameter_set = pybop.ParameterSet.pybamm("Chen2020") + return pybop.lithium_ion.SPM(parameter_set=parameter_set) + + @pytest.fixture + def parameters(self): + return [ + pybop.Parameter( + "Negative electrode active material volume fraction", + prior=pybop.Gaussian(0.55, 0.05), + bounds=[0.375, 0.75], + ), + pybop.Parameter( + "Positive electrode active material volume fraction", + prior=pybop.Gaussian(0.55, 0.05), + # no bounds + ), + ] + + @pytest.fixture( + params=[ + pybop.GaussianLogLikelihoodKnownSigma, + pybop.RootMeanSquaredError, + pybop.SumSquaredError, + ] + ) + def cost_class(self, request): + return request.param + + def noise(self, sigma, values): + return np.random.normal(0, sigma, values) + + @pytest.fixture + def spm_costs(self, model, parameters, cost_class): + # Form dataset + init_soc = 0.5 + solution = self.getdata(model, self.ground_truth, init_soc) + dataset = pybop.Dataset( + { + "Time [s]": solution["Time [s]"].data, + "Current function [A]": solution["Current [A]"].data, + "Voltage [V]": solution["Voltage [V]"].data + + self.noise(0.002, len(solution["Time [s]"].data)), + } + ) + + # Define the cost to optimise + problem = pybop.FittingProblem(model, parameters, dataset, init_soc=init_soc) + if cost_class in [pybop.GaussianLogLikelihoodKnownSigma]: + return cost_class(problem, sigma=[0.03, 0.03]) + else: + return cost_class(problem) + + @pytest.mark.parametrize( + "f_guessed", + [ + True, + False, + ], + ) + @pytest.mark.integration + def test_optimisation_f_guessed(self, f_guessed, spm_costs): + # Test each optimiser + parameterisation = pybop.Optimisation( + cost=spm_costs, optimiser=pybop.XNES, sigma0=0.05 + ) + parameterisation.set_max_unchanged_iterations(iterations=35, threshold=1e-5) + parameterisation.set_max_iterations(125) + parameterisation.set_f_guessed_tracking(f_guessed) + + initial_cost = parameterisation.cost(spm_costs.x0) + x, final_cost = parameterisation.run() + + # Assertions + assert initial_cost > final_cost + np.testing.assert_allclose(x, self.ground_truth, atol=2.5e-2) + + def getdata(self, model, x, init_soc): + model.parameter_set.update( + { + "Negative electrode active material volume fraction": x[0], + "Positive electrode active material volume fraction": x[1], + } + ) + experiment = pybop.Experiment( + [ + ( + "Discharge at 0.5C for 3 minutes (1 second period)", + "Charge at 0.5C for 3 minutes (1 second period)", + ), + ] + * 2 + ) + sim = model.predict(init_soc=init_soc, experiment=experiment) + return sim diff --git a/tests/integration/test_parameterisations.py b/tests/integration/test_parameterisations.py index ae5748235..a7e010e05 100644 --- a/tests/integration/test_parameterisations.py +++ b/tests/integration/test_parameterisations.py @@ -66,10 +66,7 @@ def spm_costs(self, model, parameters, cost_class, init_soc): ) # Define the cost to optimise - signal = ["Voltage [V]"] - problem = pybop.FittingProblem( - model, parameters, dataset, signal=signal, init_soc=init_soc - ) + problem = pybop.FittingProblem(model, parameters, dataset, init_soc=init_soc) if cost_class in [pybop.GaussianLogLikelihoodKnownSigma]: return cost_class(problem, sigma=[0.03, 0.03]) else: diff --git a/tests/unit/test_optimisation.py b/tests/unit/test_optimisation.py index 864479d8d..e847de203 100644 --- a/tests/unit/test_optimisation.py +++ b/tests/unit/test_optimisation.py @@ -94,16 +94,20 @@ def test_optimiser_classes(self, two_param_cost, optimiser_class, expected_name) # Test without bounds cost.bounds = None - if optimiser_class in [pybop.SciPyDifferentialEvolution]: + if optimiser_class in [pybop.SciPyMinimize]: + opt = pybop.Optimisation(cost=cost, optimiser=optimiser_class) + assert opt.optimiser.bounds is None + elif optimiser_class in [pybop.SciPyDifferentialEvolution]: with pytest.raises(ValueError): pybop.Optimisation(cost=cost, optimiser=optimiser_class) else: opt = pybop.Optimisation(cost=cost, optimiser=optimiser_class) + assert opt.optimiser.boundaries is None - if optimiser_class in [pybop.SciPyMinimize]: - assert opt.optimiser.bounds is None - else: - assert opt.optimiser.boundaries is None + # Test setting incorrect population size + if optimiser_class in [pybop.SciPyDifferentialEvolution]: + with pytest.raises(ValueError): + opt.optimiser.set_population_size(-5) @pytest.mark.unit def test_single_parameter(self, cost): @@ -150,6 +154,10 @@ def test_halting(self, cost): x, __ = optim.run() assert optim._iterations == 2 + # Test guessed values + optim.set_f_guessed_tracking(True) + assert optim._use_f_guessed is True + # Test invalid values with pytest.raises(ValueError): optim.set_max_evaluations(-1) From 6f0419692e5c08cca215d1b7cf828dac5cda435f Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Mon, 8 Apr 2024 20:37:35 +0100 Subject: [PATCH 3/4] add coverage --- tests/unit/test_optimisation.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_optimisation.py b/tests/unit/test_optimisation.py index e847de203..54674c954 100644 --- a/tests/unit/test_optimisation.py +++ b/tests/unit/test_optimisation.py @@ -104,11 +104,14 @@ def test_optimiser_classes(self, two_param_cost, optimiser_class, expected_name) opt = pybop.Optimisation(cost=cost, optimiser=optimiser_class) assert opt.optimiser.boundaries is None - # Test setting incorrect population size + # Test setting population size if optimiser_class in [pybop.SciPyDifferentialEvolution]: with pytest.raises(ValueError): opt.optimiser.set_population_size(-5) + # Correct value + opt.optimiser.set_population_size(5) + @pytest.mark.unit def test_single_parameter(self, cost): # Test catch for optimisers that can only run with multiple parameters From e92928cdb87433dfbae162abd634b1d93f990e9f Mon Sep 17 00:00:00 2001 From: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Date: Thu, 18 Apr 2024 11:09:09 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> --- CHANGELOG.md | 2 +- pyproject.toml | 2 +- scripts/ci/build_matrix.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce0a341f4..64e150de9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Features -- [#251](https://github.com/pybop-team/PyBOP/pull/251) - Increment PyBaMM > v25.3, remove redundant tests within integration tests, increment citation version, fix examples with incorrect model definitions. +- [#251](https://github.com/pybop-team/PyBOP/pull/251) - Increment PyBaMM > v23.5, remove redundant tests within integration tests, increment citation version, fix examples with incorrect model definitions. - [#285](https://github.com/pybop-team/PyBOP/pull/285) - Drop support for Python 3.8. - [#275](https://github.com/pybop-team/PyBOP/pull/275) - Adds Maximum a Posteriori (MAP) cost function with corresponding tests. - [#273](https://github.com/pybop-team/PyBOP/pull/273) - Adds notebooks to nox examples session and updates CI workflows for change. diff --git a/pyproject.toml b/pyproject.toml index 60de8f688..2d93faa7c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ classifiers = [ ] requires-python = ">=3.9, <3.13" dependencies = [ - "pybamm>23.5", + "pybamm>=23.9", "numpy>=1.16", "scipy>=1.3", "pints>=0.5", diff --git a/scripts/ci/build_matrix.sh b/scripts/ci/build_matrix.sh index 0bc20a174..9dccd5085 100755 --- a/scripts/ci/build_matrix.sh +++ b/scripts/ci/build_matrix.sh @@ -11,7 +11,7 @@ python_version=("3.9" "3.10" "3.11" "3.12") os=("ubuntu-latest" "windows-latest" "macos-latest") -# This command fetches the last three PyBaMM versions excluding release candidates from PyPI +# This command fetches the last two PyBaMM versions excluding release candidates from PyPI pybamm_version=($(curl -s https://pypi.org/pypi/pybamm/json | jq -r '.releases | keys[]' | grep -v rc | tail -n 2 | paste -sd " " -)) # open dict