From 1f1e91de5a253b35b4cf69a75b052c39f29e9ec1 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Sun, 5 May 2024 04:30:04 +0100 Subject: [PATCH 01/11] fixes to pybop.experiment, pybop.base_model for pybamm@develop ahead of pybamm v24.5 --- examples/scripts/parameters/example_BPX.json | 7 ++----- pybop/_experiment.py | 2 -- pybop/models/base_model.py | 16 ++++++++++++---- pyproject.toml | 4 +++- tests/unit/test_experiment.py | 6 +++--- 5 files changed, 20 insertions(+), 15 deletions(-) diff --git a/examples/scripts/parameters/example_BPX.json b/examples/scripts/parameters/example_BPX.json index 43bbcf905..1e1efaadb 100644 --- a/examples/scripts/parameters/example_BPX.json +++ b/examples/scripts/parameters/example_BPX.json @@ -1,7 +1,7 @@ { "Header": { - "BPX": 0.1, - "Title": "Parameterisation example of an LFP|graphite 2 Ah cylindrical 18650 cell.", + "BPX": 0.4, + "Title": "Parameterisation example of an LFP|graphite 2 Ah cylindrical 18650 cell. File downloaded on 19/3/24 from https://github.com/FaradayInstitution/BPX/blob/main/examples/lfp_18650_cell_BPX.json", "Description": "LFP|graphite 2 Ah cylindrical 18650 cell. Parameterisation by About:Energy Limited (aboutenergy.io), December 2022, based on cell cycling data, and electrode data gathered after cell teardown. Electrolyte properties from Nyman et al. 2008 (doi:10.1016/j.electacta.2008.04.023). Negative electrode entropic coefficient data are from O'Regan et al. 2022 (doi:10.1016/j.electacta.2022.140700). Positive electrode entropic coefficient data are from Gerver and Meyers 2011 (doi:10.1149/1.3591799). Other thermal properties are estimated.", "Model": "DFN" }, @@ -70,9 +70,6 @@ "Thickness [m]": 2e-05, "Porosity": 0.47, "Transport efficiency": 0.3222 - }, - "User-defined": { - "Source:": "An example BPX json file downloaded on 19/3/24 from https://github.com/FaradayInstitution/BPX/blob/main/examples/lfp_18650_cell_BPX.json" } } } diff --git a/pybop/_experiment.py b/pybop/_experiment.py index 1c4953849..a651dffc2 100644 --- a/pybop/_experiment.py +++ b/pybop/_experiment.py @@ -49,6 +49,4 @@ def __init__( period, temperature, termination, - drive_cycles, - cccv_handling, ) diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py index a26a5f31f..4134bfd8d 100644 --- a/pybop/models/base_model.py +++ b/pybop/models/base_model.py @@ -121,9 +121,13 @@ def build( self.set_params() self._mesh = pybamm.Mesh(self.geometry, self.submesh_types, self.var_pts) - self._disc = pybamm.Discretisation(self.mesh, self.spatial_methods) + self._disc = pybamm.Discretisation( + mesh=self.mesh, + spatial_methods=self.spatial_methods, + check_model=check_model, + ) self._built_model = self._disc.process_model( - self._model_with_set_params, inplace=False, check_model=check_model + self._model_with_set_params, inplace=False ) # Clear solver and setup model @@ -229,9 +233,13 @@ def rebuild( self.set_params(rebuild=True) self._mesh = pybamm.Mesh(self.geometry, self.submesh_types, self.var_pts) - self._disc = pybamm.Discretisation(self.mesh, self.spatial_methods) + self._disc = pybamm.Discretisation( + mesh=self.mesh, + spatial_methods=self.spatial_methods, + check_model=check_model, + ) self._built_model = self._disc.process_model( - self._model_with_set_params, inplace=False, check_model=check_model + self._model_with_set_params, inplace=False ) # Clear solver and setup model diff --git a/pyproject.toml b/pyproject.toml index 14ec41be7..7e4f89ed4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,9 @@ classifiers = [ ] requires-python = ">=3.9, <3.13" dependencies = [ - "pybamm>=23.9", + # "pybamm>=23.9", + # "pybamm @ git+https://github.com/pybamm-team/PyBaMM@develop", + "pybamm @ git+https://github.com/BradyPlanden/PyBaMM@fix-electrode-diffusion-rename", "numpy>=1.16", "scipy>=1.3", "pints>=0.5", diff --git a/tests/unit/test_experiment.py b/tests/unit/test_experiment.py index 6d18ef509..71355ff84 100644 --- a/tests/unit/test_experiment.py +++ b/tests/unit/test_experiment.py @@ -18,9 +18,9 @@ def test_experiment(self): pybop_experiment = pybop.Experiment(protocol) pybamm_experiment = pybamm.Experiment(protocol) - assert [ - step.to_dict() for step in pybop_experiment.operating_conditions_steps - ] == [step.to_dict() for step in pybamm_experiment.operating_conditions_steps] + assert [step.to_dict() for step in pybop_experiment.steps] == [ + step.to_dict() for step in pybamm_experiment.steps + ] assert pybop_experiment.cycle_lengths == pybamm_experiment.cycle_lengths From 1770dc818ebd714b088a4715eaebf72c03dda7a6 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Mon, 6 May 2024 00:15:05 +0100 Subject: [PATCH 02/11] increment number of states in spm_UKF example --- examples/scripts/spm_UKF.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/scripts/spm_UKF.py b/examples/scripts/spm_UKF.py index 5299c5816..49380aba6 100644 --- a/examples/scripts/spm_UKF.py +++ b/examples/scripts/spm_UKF.py @@ -22,7 +22,7 @@ # Make a prediction with measurement noise sigma = 0.001 -t_eval = np.arange(0, 300, 2) +t_eval = np.arange(0, 900, 0.5) values = model.predict(t_eval=t_eval) corrupt_values = values["Voltage [V]"].data + np.random.normal(0, sigma, len(t_eval)) @@ -42,8 +42,8 @@ signal = ["Voltage [V]"] n_states = model.n_states n_signals = len(signal) -covariance = np.diag([0] * 19 + [sigma**2] + [0] * 19 + [sigma**2]) -process_noise = np.diag([0] * 19 + [1e-6] + [0] * 19 + [1e-6]) +covariance = np.diag([0] * 20 + [sigma**2] + [0] * 20 + [sigma**2]) +process_noise = np.diag([0] * 20 + [1e-6] + [0] * 20 + [1e-6]) measurement_noise = np.diag([sigma**2]) observer = pybop.UnscentedKalmanFilterObserver( parameters, From 786d9ca637740a4c49eb84ffabca0ea0c6d6c948 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Wed, 22 May 2024 16:55:41 +0100 Subject: [PATCH 03/11] fix: update non-converged DFN test --- pyproject.toml | 3 +-- tests/unit/test_models.py | 9 ++++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9d2b02f91..0de3b16ef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,8 +27,7 @@ classifiers = [ requires-python = ">=3.9, <3.13" dependencies = [ # "pybamm>=23.9", - # "pybamm @ git+https://github.com/pybamm-team/PyBaMM@develop", - "pybamm @ git+https://github.com/BradyPlanden/PyBaMM@fix-electrode-diffusion-rename", + "pybamm @ git+https://github.com/pybamm-team/PyBaMM@develop", "numpy>=1.16", "scipy>=1.3", "pints>=0.5", diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 3137f6f2e..671097b7e 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -282,10 +282,13 @@ def test_non_converged_solution(self): "Voltage [V]": np.zeros(100), } ) - problem = pybop.FittingProblem(model, parameters=parameters, dataset=dataset) - res = problem.evaluate([-0.2, -0.2]) - _, res_grad = problem.evaluateS1([-0.2, -0.2]) + + # Simulate the DFN with active material values of 0 + # This should not converge, and as such, the + # solution from model.simulate should be inf + res = problem.evaluate([0, 0]) + _, res_grad = problem.evaluateS1([0, 0]) for key in problem.signal: assert np.isinf(res.get(key, [])).any() From 7711a4e80658c26feec3cd6a4e45f35981382502 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Thu, 27 Jun 2024 18:47:28 +0100 Subject: [PATCH 04/11] update target pybamm source to forked develop w/ diffusivity fix --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0de3b16ef..1b0247093 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,7 +27,7 @@ classifiers = [ requires-python = ">=3.9, <3.13" dependencies = [ # "pybamm>=23.9", - "pybamm @ git+https://github.com/pybamm-team/PyBaMM@develop", + "pybamm @ git+https://github.com/bradyplanden/PyBaMM@v24.5rc0-diffusivity-test", "numpy>=1.16", "scipy>=1.3", "pints>=0.5", From a76959f41de09d8a6f39449fa7468d2a00b9afcb Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Fri, 28 Jun 2024 09:58:45 +0100 Subject: [PATCH 05/11] tests: updts to test_non_converged_solution. Changes to pybamm v24.5 have resulted in solver.solve() to return values for incorrect inputs values when not requesting sensitivities. This removes the test that assert output == np.inf in this case. --- tests/unit/test_models.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 2bb577337..a458a4d26 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -342,12 +342,11 @@ def test_non_converged_solution(self): ) problem = pybop.FittingProblem(model, parameters=parameters, dataset=dataset) - # Simulate the DFN with active material values of 0 - # This should not converge, and as such, the - # solution from model.simulate should be inf - res = problem.evaluate([0, 0]) - _, res_grad = problem.evaluateS1([0, 0]) + # Simulate the DFN with active material values of 0. + # The solution from model.simulateS1 should be inf + # and the gradient should be inf. + output_S1, res_grad = problem.evaluateS1([0, 0]) for key in problem.signal: - assert np.isinf(res.get(key, [])).any() + assert np.isinf(output_S1.get(key, [])).any() assert np.isinf(res_grad).any() From c8deb1501eb9b3dfae3bdf9fee0c304316d22fa8 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Fri, 28 Jun 2024 10:50:06 +0100 Subject: [PATCH 06/11] test: updt asserts to capture differing outputs --- tests/unit/test_models.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index a458a4d26..4a05b55a0 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -343,10 +343,11 @@ def test_non_converged_solution(self): problem = pybop.FittingProblem(model, parameters=parameters, dataset=dataset) # Simulate the DFN with active material values of 0. - # The solution from model.simulateS1 should be inf - # and the gradient should be inf. + # The solutions will not change as the solver will not converge. + output = problem.evaluate([0, 0]) output_S1, res_grad = problem.evaluateS1([0, 0]) for key in problem.signal: + assert np.allclose(output.get(key, [])[0], output.get(key, [])) assert np.isinf(output_S1.get(key, [])).any() assert np.isinf(res_grad).any() From 06dd97980ee411b2fac8b51d15185121f2e42598 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Fri, 28 Jun 2024 11:08:25 +0100 Subject: [PATCH 07/11] test: revert to allclose as output vector varies for each OS --- tests/unit/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 4a05b55a0..6fc431d56 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -349,5 +349,5 @@ def test_non_converged_solution(self): for key in problem.signal: assert np.allclose(output.get(key, [])[0], output.get(key, [])) - assert np.isinf(output_S1.get(key, [])).any() + assert np.allclose(output_S1.get(key, [])[0], output_S1.get(key, [])) assert np.isinf(res_grad).any() From c2613700ca18c51d95b57cd0c5337fe655c3e0c4 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Fri, 28 Jun 2024 18:26:54 +0100 Subject: [PATCH 08/11] tests: remove gradient == np.inf assert --- tests/unit/test_models.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 6fc431d56..07147a74a 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -345,9 +345,8 @@ def test_non_converged_solution(self): # Simulate the DFN with active material values of 0. # The solutions will not change as the solver will not converge. output = problem.evaluate([0, 0]) - output_S1, res_grad = problem.evaluateS1([0, 0]) + output_S1, _ = problem.evaluateS1([0, 0]) for key in problem.signal: assert np.allclose(output.get(key, [])[0], output.get(key, [])) assert np.allclose(output_S1.get(key, [])[0], output_S1.get(key, [])) - assert np.isinf(res_grad).any() From 3e4012850c4de4d340fd326bd8797bf95402e787 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Sun, 30 Jun 2024 13:34:09 +0100 Subject: [PATCH 09/11] tests: Add missing asserts --- tests/unit/test_cost.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_cost.py b/tests/unit/test_cost.py index 3c0d81514..49a1b8b06 100644 --- a/tests/unit/test_cost.py +++ b/tests/unit/test_cost.py @@ -149,7 +149,8 @@ def test_costs(self, cost): cost([1.1]) # Test option setting - cost.set_fail_gradient(1) + cost.set_fail_gradient(10) + assert cost._de == 10 if isinstance(cost, pybop.SumSquaredError): e, de = cost.evaluateS1([0.5]) @@ -229,4 +230,4 @@ def test_design_costs( # Compute after updating nominal capacity cost = cost_class(problem, update_capacity=True) - cost([0.4]) + assert np.isfinite(cost([0.4])) From 40133fb4ebd918259dcffc0d33ecb5e5ee58b180 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Sat, 20 Jul 2024 11:24:58 +0100 Subject: [PATCH 10/11] build: increment pybamm to 24.5rc2, ci: increment tests alongside. --- .gitignore | 3 +++ CHANGELOG.md | 1 + examples/scripts/gitt.py | 4 ++-- pybop/models/lithium_ion/weppner_huggins.py | 2 +- pyproject.toml | 3 +-- scripts/ci/build_matrix.sh | 11 +++++++---- tests/integration/test_model_experiment_changes.py | 2 +- tests/unit/test_models.py | 2 +- 8 files changed, 17 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 3c3bb708d..2bafcca63 100644 --- a/.gitignore +++ b/.gitignore @@ -314,3 +314,6 @@ $RECYCLE.BIN/ # Airspeed Velocity *.asv/ results/ + +# Pycharm +*.idea/ diff --git a/CHANGELOG.md b/CHANGELOG.md index be08914bf..9450ad113 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - [#393](https://github.com/pybop-team/PyBOP/pull/383) - Adds Minkowski and SumofPower cost classes, with an example and corresponding tests. - [#403](https://github.com/pybop-team/PyBOP/pull/403/) - Adds lychee link checking action. +- [#313](https://github.com/pybop-team/PyBOP/pull/313/) - Fixes for PyBaMM v24.5, drops support for PyBaMM v23.9, v24.1 ## Bug Fixes diff --git a/examples/scripts/gitt.py b/examples/scripts/gitt.py index 6d3b4a94b..4b8c1561b 100644 --- a/examples/scripts/gitt.py +++ b/examples/scripts/gitt.py @@ -36,10 +36,10 @@ model = pybop.lithium_ion.WeppnerHuggins(parameter_set=parameter_set) parameters = pybop.Parameter( - "Positive electrode diffusivity [m2.s-1]", + "Positive particle diffusivity [m2.s-1]", prior=pybop.Gaussian(5e-14, 1e-13), bounds=[1e-16, 1e-11], - true_value=parameter_set["Positive electrode diffusivity [m2.s-1]"], + true_value=parameter_set["Positive particle diffusivity [m2.s-1]"], ) problem = pybop.FittingProblem( diff --git a/pybop/models/lithium_ion/weppner_huggins.py b/pybop/models/lithium_ion/weppner_huggins.py index 74c42c70e..b8707cca9 100644 --- a/pybop/models/lithium_ion/weppner_huggins.py +++ b/pybop/models/lithium_ion/weppner_huggins.py @@ -65,7 +65,7 @@ def __init__(self, name="Weppner & Huggins model", **model_kwargs): # Parameters ###################### - d_s = Parameter("Positive electrode diffusivity [m2.s-1]") + d_s = Parameter("Positive particle diffusivity [m2.s-1]") c_s_max = Parameter("Maximum concentration in positive electrode [mol.m-3]") diff --git a/pyproject.toml b/pyproject.toml index cd99a096d..eacb65f7d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,8 +26,7 @@ classifiers = [ ] requires-python = ">=3.9, <3.13" dependencies = [ - # "pybamm>=23.9", - "pybamm @ git+https://github.com/bradyplanden/PyBaMM@v24.5rc0-diffusivity-test", + "pybamm>=24.5rc2", "numpy>=1.16, <2.0", "scipy>=1.3", "pints>=0.5", diff --git a/scripts/ci/build_matrix.sh b/scripts/ci/build_matrix.sh index 9dfe32249..88d5b9b38 100755 --- a/scripts/ci/build_matrix.sh +++ b/scripts/ci/build_matrix.sh @@ -11,8 +11,11 @@ python_version=("3.9" "3.10" "3.11" "3.12") os=("ubuntu-latest" "windows-latest" "macos-13" "macos-14") -# 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 " " -)) +# This command fetches the last PyBaMM version excluding release candidates from PyPI +#pybamm_version=($(curl -s https://pypi.org/pypi/pybamm/json | jq -r '.releases | keys[]' | grep -v rc | tail -n 1 | paste -sd " " -)) + +# This command fetches the last PyBaMM versions including release candidates from PyPI +pybamm_version=($(curl -s https://pypi.org/pypi/pybamm/json | jq -r '.releases | keys[]' | tail -n 1 | paste -sd " " -)) # open dict json='{ @@ -40,7 +43,7 @@ json+=' ] }' -# Filter out incompatible combinations -json=$(echo "$json" | jq -c 'del(.include[] | select(.pybamm_version == "23.9" and .python_version == "3.12"))') +# Example for filtering out incompatible combinations +#json=$(echo "$json" | jq -c 'del(.include[] | select(.pybamm_version == "23.9" and .python_version == "3.12"))') echo "$json" | jq -c . diff --git a/tests/integration/test_model_experiment_changes.py b/tests/integration/test_model_experiment_changes.py index 64d27132a..7bcc33ddb 100644 --- a/tests/integration/test_model_experiment_changes.py +++ b/tests/integration/test_model_experiment_changes.py @@ -22,7 +22,7 @@ class TestModelAndExperimentChanges: ), pybop.Parameters( pybop.Parameter( # non-geometric parameter - "Positive electrode diffusivity [m2.s-1]", + "Positive particle diffusivity [m2.s-1]", prior=pybop.Gaussian(3.43e-15, 1e-15), bounds=[1e-15, 5e-15], true_value=4e-15, diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index b2e298428..b12b3639e 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -361,7 +361,7 @@ def test_non_converged_solution(self): problem = pybop.FittingProblem(model, parameters=parameters, dataset=dataset) # Simulate the DFN with active material values of 0. - # The solutions will not change as the solver will not converge. + # The solution elements will not change as the solver will not converge. output = problem.evaluate([0, 0]) output_S1, _ = problem.evaluateS1([0, 0]) From 4c100ff618b9ee4327040e77b1fd7656e8ee9ce3 Mon Sep 17 00:00:00 2001 From: Brady Planden <brady.planden@gmail.com> Date: Sat, 20 Jul 2024 15:13:38 +0100 Subject: [PATCH 11/11] tests: increase coverage, fix flaky observer tests --- tests/unit/test_observer_unscented_kalman.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/test_observer_unscented_kalman.py b/tests/unit/test_observer_unscented_kalman.py index 0b5d3067b..c83fae315 100644 --- a/tests/unit/test_observer_unscented_kalman.py +++ b/tests/unit/test_observer_unscented_kalman.py @@ -93,6 +93,15 @@ def test_cholupdate(self): SquareRootUKF.cholupdate(R1_, u.copy(), 1.0) np.testing.assert_array_almost_equal(R1, R1_) + # Test hypot + f = 10.0 + j = 20.0 + out_1 = f * np.sqrt(1 + 1.0 * f**2 / j**2) + np.testing.assert_allclose(SquareRootUKF.hypot(f, j, 1.0), out_1) + + j = 10.0 + np.testing.assert_allclose(SquareRootUKF.hypot(f, j, 1.0), float(0)) + @pytest.mark.unit def test_unscented_kalman_filter(self, dataset, observer): t_eval = dataset["Time [s]"] @@ -116,6 +125,11 @@ def test_unscented_kalman_filter(self, dataset, observer): decimal=4, ) + # Test get covariance + cov = observer.get_current_measure() + assert cov.shape == (1, 1) + assert float(0) <= cov[0] + @pytest.mark.unit def test_observe_no_measurement(self, observer): with pytest.raises(ValueError):