Merge branch 'pybop-team:develop' into #238_multiple_datasets

.all-contributorsrc

@@ -61,7 +61,8 @@
       "avatar_url": "https://avatars.githubusercontent.com/u/28443643?v=4",
       "profile": "https://www.brosaplanella.xyz",
       "contributions": [
-        "review"
+        "review",
+        "code"
       ]
     },
     {

.pre-commit-config.yaml

@@ -4,7 +4,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: "v0.3.7"
+    rev: "v0.4.1"
     hooks:
       - id: ruff
         args: [--fix, --show-fixes]

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.

README.md

@@ -120,7 +120,8 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
       <td align="center" valign="top" width="14.28%"><a href="https://github.com/NicolaCourtier"><img src="https://avatars.githubusercontent.com/u/45851982?v=4?s=100" width="100px;" alt="NicolaCourtier"/><br /><sub><b>NicolaCourtier</b></sub></a><br /><a href="https://github.com/pybop-team/PyBOP/commits?author=NicolaCourtier" title="Code">💻</a> <a href="https://github.com/pybop-team/PyBOP/pulls?q=is%3Apr+reviewed-by%3ANicolaCourtier" title="Reviewed Pull Requests">👀</a> <a href="#example-NicolaCourtier" title="Examples">💡</a> <a href="https://github.com/pybop-team/PyBOP/commits?author=NicolaCourtier" title="Tests">⚠️</a></td>
       <td align="center" valign="top" width="14.28%"><a href="http://howey.eng.ox.ac.uk"><img src="https://avatars.githubusercontent.com/u/2247552?v=4?s=100" width="100px;" alt="David Howey"/><br /><sub><b>David Howey</b></sub></a><br /><a href="#ideas-davidhowey" title="Ideas, Planning, & Feedback">🤔</a> <a href="#mentoring-davidhowey" title="Mentoring">🧑‍🏫</a></td>
       <td align="center" valign="top" width="14.28%"><a href="http://www.rse.ox.ac.uk"><img src="https://avatars.githubusercontent.com/u/1148404?v=4?s=100" width="100px;" alt="Martin Robinson"/><br /><sub><b>Martin Robinson</b></sub></a><br /><a href="#ideas-martinjrobins" title="Ideas, Planning, & Feedback">🤔</a> <a href="#mentoring-martinjrobins" title="Mentoring">🧑‍🏫</a> <a href="https://github.com/pybop-team/PyBOP/pulls?q=is%3Apr+reviewed-by%3Amartinjrobins" title="Reviewed Pull Requests">👀</a> <a href="https://github.com/pybop-team/PyBOP/commits?author=martinjrobins" title="Code">💻</a> <a href="https://github.com/pybop-team/PyBOP/commits?author=martinjrobins" title="Tests">⚠️</a></td>
-      <td align="center" valign="top" width="14.28%"><a href="https://www.brosaplanella.xyz"><img src="https://avatars.githubusercontent.com/u/28443643?v=4?s=100" width="100px;" alt="Ferran Brosa Planella"/><br /><sub><b>Ferran Brosa Planella</b></sub></a><br /><a href="https://github.com/pybop-team/PyBOP/pulls?q=is%3Apr+reviewed-by%3Abrosaplanella" title="Reviewed Pull Requests">👀</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://www.brosaplanella.xyz"><img src="https://avatars.githubusercontent.com/u/28443643?v=4?s=100" width="100px;" alt="Ferran Brosa Planella"/><br /><sub><b>Ferran Brosa Planella</b></sub></a><br /><a href="https://github.com/pybop-team/PyBOP/pulls?q=is%3Apr+reviewed-by%3Abrosaplanella" title="Reviewed Pull Requests">👀</a> <a href="https://github.com/pybop-team/PyBOP/commits?author=brosaplanella" title="Code">💻</a></td>
+      <a href="https://github.com/pybop-team/PyBOP/commits?author=brosaplanella" title="Code">💻</a></td>
       <td align="center" valign="top" width="14.28%"><a href="https://github.com/agriyakhetarpal"><img src="https://avatars.githubusercontent.com/u/74401230?v=4?s=100" width="100px;" alt="Agriya Khetarpal"/><br /><sub><b>Agriya Khetarpal</b></sub></a><br /><a href="https://github.com/pybop-team/PyBOP/commits?author=agriyakhetarpal" title="Code">💻</a> <a href="#infra-agriyakhetarpal" title="Infrastructure (Hosting, Build-Tools, etc)">🚇</a></td>
     </tr>
     <tr>

pybop/models/base_model.py

@@ -361,7 +361,7 @@ def simulate(self, inputs, t_eval) -> dict[str, 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}
 

pybop/models/lithium_ion/base_echem.py

@@ -36,8 +36,12 @@ def __init__(
         self.spatial_methods = (
             spatial_methods or self.pybamm_model.default_spatial_methods
         )
-        self.solver = solver or self.pybamm_model.default_solver
-        self.solver.max_step_decrease_count = 1
+        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
 
         # Internal attributes for the built model are initialized but not set
         self._model_with_set_params = None

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_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)
-        np.testing.assert_almost_equal(cost_2, 0)
+        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

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
@@ -269,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()