From fc4b229027dc3fb60438ea6ed17027fa19b692fe Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Sat, 20 Apr 2024 10:23:46 +0100 Subject: [PATCH 1/4] Add to default echem solver, updt. tests --- pybop/models/base_model.py | 2 +- pybop/models/lithium_ion/base_echem.py | 6 +++++- tests/integration/test_model_experiment_changes.py | 8 ++++---- tests/unit/test_models.py | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py index fd2d3e4e3..a89b755aa 100644 --- a/pybop/models/base_model.py +++ b/pybop/models/base_model.py @@ -439,7 +439,7 @@ def simulateS1(self, inputs, t_eval): return y, dy except Exception as e: print(f"Error: {e}") - return [np.inf], [np.inf] + return {signal: [np.inf] for signal in self.signal}, [np.inf] else: return {signal: [np.inf] for signal in self.signal}, [np.inf] diff --git a/pybop/models/lithium_ion/base_echem.py b/pybop/models/lithium_ion/base_echem.py index 595f52935..54febac2e 100644 --- a/pybop/models/lithium_ion/base_echem.py +++ b/pybop/models/lithium_ion/base_echem.py @@ -36,7 +36,11 @@ def __init__( self.spatial_methods = ( spatial_methods or self.pybamm_model.default_spatial_methods ) - self.solver = solver or self.pybamm_model.default_solver + if solver is None: + self.solver = self.pybamm_model.default_solver + self.solver.mode = "fast with events" + else: + self.solver = solver self.solver.max_step_decrease_count = 1 # Internal attributes for the built model are initialized but not set diff --git a/tests/integration/test_model_experiment_changes.py b/tests/integration/test_model_experiment_changes.py index 8d648a84e..caf591952 100644 --- a/tests/integration/test_model_experiment_changes.py +++ b/tests/integration/test_model_experiment_changes.py @@ -54,8 +54,8 @@ def test_changing_experiment(self, parameter): ) # The datasets are not corrupted so the costs should be zero - np.testing.assert_almost_equal(cost_1, 0) - np.testing.assert_almost_equal(cost_2, 0) + np.testing.assert_almost_equal(cost_1, 0, decimal=5) + np.testing.assert_almost_equal(cost_2, 0, decimal=5) @pytest.mark.integration def test_changing_model(self, parameter): @@ -81,8 +81,8 @@ def test_changing_model(self, parameter): ) # The datasets are not corrupted so the costs should be zero - np.testing.assert_almost_equal(cost_1, 0) - np.testing.assert_almost_equal(cost_2, 0) + np.testing.assert_almost_equal(cost_1, 0, decimal=5) + np.testing.assert_almost_equal(cost_2, 0, decimal=5) def final_cost(self, solution, model, parameters, init_soc): # Compute the cost corresponding to a particular solution diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 06783dbc9..75c882388 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -267,7 +267,7 @@ def test_non_converged_solution(self): problem = pybop.FittingProblem(model, parameters=parameters, dataset=dataset) res = problem.evaluate([-0.2, -0.2]) - res_grad = problem.evaluateS1([-0.2, -0.2]) + _, res_grad = problem.evaluateS1([-0.2, -0.2]) assert np.isinf(res).any() assert np.isinf(res_grad).any() From 5c0db7966bba4a56f040c6f46a9d6bc4b59b598b Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Sat, 20 Apr 2024 10:35:06 +0100 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64e150de9..2248d0717 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Features +- [#301](https://github.com/pybop-team/PyBOP/pull/301) - Updates default echem solver to "fast with events" mode. - [#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. From 2239c096b3fbeabf11ae3c5c2777ac5b527c9ffb Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Sat, 20 Apr 2024 11:33:06 +0100 Subject: [PATCH 3/4] add non default solver tests --- pybop/models/lithium_ion/base_echem.py | 2 +- tests/unit/test_models.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pybop/models/lithium_ion/base_echem.py b/pybop/models/lithium_ion/base_echem.py index 54febac2e..f681bd890 100644 --- a/pybop/models/lithium_ion/base_echem.py +++ b/pybop/models/lithium_ion/base_echem.py @@ -39,9 +39,9 @@ def __init__( if solver is None: self.solver = self.pybamm_model.default_solver self.solver.mode = "fast with events" + self.solver.max_step_decrease_count = 1 else: self.solver = solver - self.solver.max_step_decrease_count = 1 # Internal attributes for the built model are initialized but not set self._model_with_set_params = None diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 75c882388..88bca8147 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -37,6 +37,18 @@ def test_simulate_without_build_model(self, model): ): model.simulateS1(None, None) + @pytest.mark.unit + def test_non_default_solver(self): + solver = pybamm.CasadiSolver( + mode="fast", + atol=1e-6, + rtol=1e-6, + ) + model = pybop.lithium_ion.SPM(solver=solver) + assert model.solver.mode == "fast" + assert model.solver.atol == 1e-6 + assert model.solver.rtol == 1e-6 + @pytest.mark.unit def test_predict_without_pybamm(self, model): model._unprocessed_model = None From 02bd80a1d38534d6e8e8180c99b3f53ecee1600a Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Fri, 26 Apr 2024 01:51:14 +0100 Subject: [PATCH 4/4] update tests, fix broken model.simulate exception type return --- pybop/models/base_model.py | 2 +- tests/integration/test_model_experiment_changes.py | 8 ++++---- tests/unit/test_models.py | 3 ++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py index a89b755aa..a26a5f31f 100644 --- a/pybop/models/base_model.py +++ b/pybop/models/base_model.py @@ -359,7 +359,7 @@ def simulate(self, inputs, t_eval) -> np.ndarray[np.float64]: ) except Exception as e: print(f"Error: {e}") - return [np.inf] + return {signal: [np.inf] for signal in self.signal} else: return {signal: [np.inf] for signal in self.signal} diff --git a/tests/integration/test_model_experiment_changes.py b/tests/integration/test_model_experiment_changes.py index caf591952..b8fb16880 100644 --- a/tests/integration/test_model_experiment_changes.py +++ b/tests/integration/test_model_experiment_changes.py @@ -54,8 +54,8 @@ def test_changing_experiment(self, parameter): ) # The datasets are not corrupted so the costs should be zero - np.testing.assert_almost_equal(cost_1, 0, decimal=5) - np.testing.assert_almost_equal(cost_2, 0, decimal=5) + np.testing.assert_allclose(cost_1, 0, atol=1e-5) + np.testing.assert_allclose(cost_2, 0, atol=1e-5) @pytest.mark.integration def test_changing_model(self, parameter): @@ -81,8 +81,8 @@ def test_changing_model(self, parameter): ) # The datasets are not corrupted so the costs should be zero - np.testing.assert_almost_equal(cost_1, 0, decimal=5) - np.testing.assert_almost_equal(cost_2, 0, decimal=5) + np.testing.assert_allclose(cost_1, 0, atol=1e-5) + np.testing.assert_allclose(cost_2, 0, atol=1e-5) def final_cost(self, solution, model, parameters, init_soc): # Compute the cost corresponding to a particular solution diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 88bca8147..ba545a81d 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -281,5 +281,6 @@ def test_non_converged_solution(self): res = problem.evaluate([-0.2, -0.2]) _, res_grad = problem.evaluateS1([-0.2, -0.2]) - assert np.isinf(res).any() + for key in problem.signal: + assert np.isinf(res.get(key, [])).any() assert np.isinf(res_grad).any()