From 4d1920943630c58943fd9083c4ab6755c69e4351 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 9 Aug 2022 15:27:09 +0100 Subject: [PATCH 01/28] remove function diffusivities (they were constant anyway) --- .../graphite_LGM50_diffusivity_Chen2020.py | 33 ------------------- .../nmc_LGM50_diffusivity_Chen2020.py | 33 ------------------- 2 files changed, 66 deletions(-) delete mode 100644 pybamm/input/parameters/lithium_ion/negative_electrodes/graphite_Chen2020/graphite_LGM50_diffusivity_Chen2020.py delete mode 100644 pybamm/input/parameters/lithium_ion/positive_electrodes/nmc_Chen2020/nmc_LGM50_diffusivity_Chen2020.py diff --git a/pybamm/input/parameters/lithium_ion/negative_electrodes/graphite_Chen2020/graphite_LGM50_diffusivity_Chen2020.py b/pybamm/input/parameters/lithium_ion/negative_electrodes/graphite_Chen2020/graphite_LGM50_diffusivity_Chen2020.py deleted file mode 100644 index 0108c24e99..0000000000 --- a/pybamm/input/parameters/lithium_ion/negative_electrodes/graphite_Chen2020/graphite_LGM50_diffusivity_Chen2020.py +++ /dev/null @@ -1,33 +0,0 @@ -from pybamm import exp, constants - - -def graphite_LGM50_diffusivity_Chen2020(sto, T): - """ - LG M50 Graphite diffusivity as a function of stochiometry, in this case the - diffusivity is taken to be a constant. The value is taken from [1]. - - References - ---------- - .. [1] Chang-Hui Chen, Ferran Brosa Planella, Kieran O’Regan, Dominika Gastol, W. - Dhammika Widanage, and Emma Kendrick. "Development of Experimental Techniques for - Parameterization of Multi-scale Lithium-ion Battery Models." Journal of the - Electrochemical Society 167 (2020): 080534. - - Parameters - ---------- - sto: :class:`pybamm.Symbol` - Electrode stochiometry - T: :class:`pybamm.Symbol` - Dimensional temperature - - Returns - ------- - :class:`pybamm.Symbol` - Solid diffusivity - """ - - D_ref = 3.3e-14 - E_D_s = 0 # to be implemented - arrhenius = exp(E_D_s / constants.R * (1 / 298.15 - 1 / T)) - - return D_ref * arrhenius diff --git a/pybamm/input/parameters/lithium_ion/positive_electrodes/nmc_Chen2020/nmc_LGM50_diffusivity_Chen2020.py b/pybamm/input/parameters/lithium_ion/positive_electrodes/nmc_Chen2020/nmc_LGM50_diffusivity_Chen2020.py deleted file mode 100644 index d5b52e55a0..0000000000 --- a/pybamm/input/parameters/lithium_ion/positive_electrodes/nmc_Chen2020/nmc_LGM50_diffusivity_Chen2020.py +++ /dev/null @@ -1,33 +0,0 @@ -from pybamm import exp, constants - - -def nmc_LGM50_diffusivity_Chen2020(sto, T): - """ - NMC diffusivity as a function of stoichiometry, in this case the - diffusivity is taken to be a constant. The value is taken from [1]. - - References - ---------- - .. [1] Chang-Hui Chen, Ferran Brosa Planella, Kieran O’Regan, Dominika Gastol, W. - Dhammika Widanage, and Emma Kendrick. "Development of Experimental Techniques for - Parameterization of Multi-scale Lithium-ion Battery Models." Journal of the - Electrochemical Society 167 (2020): 080534. - - Parameters - ---------- - sto: :class:`pybamm.Symbol` - Electrode stochiometry - T: :class:`pybamm.Symbol` - Dimensional temperature - - Returns - ------- - :class:`pybamm.Symbol` - Solid diffusivity - """ - - D_ref = 4e-15 - E_D_s = 0 # to be implemented - arrhenius = exp(E_D_s / constants.R * (1 / 298.15 - 1 / T)) - - return D_ref * arrhenius From 06022b9d3e5ba5f8fdbb8421edf80b04f25138ed Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 9 Aug 2022 15:52:45 +0100 Subject: [PATCH 02/28] improve docstring --- pybamm/models/event.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pybamm/models/event.py b/pybamm/models/event.py index b5885609e2..b43566ddb6 100644 --- a/pybamm/models/event.py +++ b/pybamm/models/event.py @@ -12,6 +12,9 @@ class EventType(Enum): should return the time that the discontinuity occurs. The solver will integrate up to the discontinuity and then restart just after the discontinuity. + INTERPOLANT_EXTRAPOLATION indicates that a pybamm.Interpolant object has been evaluated outside of the range. + + SWITCH indicates an event switch that is used in CasADI "fast with events" model. """ TERMINATION = 0 @@ -29,12 +32,11 @@ class Event: ---------- name: str - A string giving the name of the event - event_type: :class:`pybamm.EventType` - An enum defining the type of event + A string giving the name of the event. expression: :class:`pybamm.Symbol` - An expression that defines when the event occurs - + An expression that defines when the event occurs. + event_type: :class:`pybamm.EventType` (optional) + An enum defining the type of event. By default it is set to TERMINATION. """ From 3818f743fc8bc045c246ec7a891fcca8b7a64630 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 9 Aug 2022 15:53:27 +0100 Subject: [PATCH 03/28] add test events --- tests/unit/test_models/test_event.py | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 tests/unit/test_models/test_event.py diff --git a/tests/unit/test_models/test_event.py b/tests/unit/test_models/test_event.py new file mode 100644 index 0000000000..356bf7131a --- /dev/null +++ b/tests/unit/test_models/test_event.py @@ -0,0 +1,59 @@ +# +# Tests Event class +# +import pybamm +import numpy as np +import unittest + + +class TestEvent(unittest.TestCase): + def test_event(self): + expression = pybamm.Scalar(1) + event = pybamm.Event("my event", expression) + + self.assertEqual(event.name, "my event") + self.assertEqual(event.__str__(), "my event") + self.assertEqual(event.expression, expression) + self.assertEqual(event.event_type, pybamm.EventType.TERMINATION) + + def test_expression_evaluate(self): + # Test t + expression = pybamm.t + event = pybamm.Event("my event", expression) + self.assertEqual(event.evaluate(t=1), 1) + + # Test y + sv = pybamm.StateVector(slice(0, 10)) + expression = sv + eval_array = np.linspace(0, 2, 19) + test_array = np.linspace(0, 1, 10)[:, np.newaxis] + event = pybamm.Event("my event", expression) + np.testing.assert_array_equal(event.evaluate(y=eval_array), test_array) + + # Test y_dot + expression = sv.diff(pybamm.t) + event = pybamm.Event("my event", expression) + np.testing.assert_array_equal(event.evaluate(y_dot=eval_array), test_array) + + def test_event_types(self): + event_types = [ + pybamm.EventType.TERMINATION, + pybamm.EventType.DISCONTINUITY, + pybamm.EventType.INTERPOLANT_EXTRAPOLATION, + pybamm.EventType.SWITCH, + ] + + for event_type in event_types: + event = pybamm.Event("my event", pybamm.Scalar(1), event_type) + self.assertEqual(event.event_type, event_type) + + + +if __name__ == "__main__": + print("Add -v for more debug output") + import sys + + if "-v" in sys.argv: + debug = True + pybamm.settings.debug_mode = True + unittest.main() From bd80d2cb0187963c3a30cf763ede387c52f278c8 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 9 Aug 2022 16:01:36 +0100 Subject: [PATCH 04/28] remove Kim2011 ocp function --- .../nca_Kim2011/nca_ocp_Kim2011_function.py | 37 ------------------- 1 file changed, 37 deletions(-) delete mode 100644 pybamm/input/parameters/lithium_ion/positive_electrodes/nca_Kim2011/nca_ocp_Kim2011_function.py diff --git a/pybamm/input/parameters/lithium_ion/positive_electrodes/nca_Kim2011/nca_ocp_Kim2011_function.py b/pybamm/input/parameters/lithium_ion/positive_electrodes/nca_Kim2011/nca_ocp_Kim2011_function.py deleted file mode 100644 index 3edf208341..0000000000 --- a/pybamm/input/parameters/lithium_ion/positive_electrodes/nca_Kim2011/nca_ocp_Kim2011_function.py +++ /dev/null @@ -1,37 +0,0 @@ -from pybamm import exp - - -def nca_ocp_Kim2011_function(sto): - """ - NCA open-circuit potential (OCP) [1]. Fit in paper seems wrong to using - nca_ocp_Kim2011_data.csv instead. - References - ---------- - .. [1] Kim, G. H., Smith, K., Lee, K. J., Santhanagopalan, S., & Pesaran, A. - (2011). Multi-domain modeling of lithium-ion batteries encompassing - multi-physics in varied length scales. Journal of The Electrochemical - Society, 158(8), A955-A969. - - Parameters - ---------- - sto : :class:`pybamm.Symbol` - Stochiometry of material (li-fraction) - - """ - - u_eq = ( - 1.68 * sto ** 10 - - 2.222 * sto ** 9 - + 15.056 * sto ** 8 - - 23.488 * sto ** 7 - + 81.246 * sto ** 6 - - 344.566 * sto ** 5 - + 621.3475 * sto ** 4 - - 544.774 * sto ** 3 - + 264.427 * sto ** 2 - - 66.3691 * sto - + 11.8058 - - 0.61386 * exp(5.8201 * sto ** 136.4) - ) - - return u_eq From 6b7241c2465b99178b4f89a92aaedd9e24d58957 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 9 Aug 2022 16:22:57 +0100 Subject: [PATCH 05/28] fix warning test t_eval drive cycle --- tests/unit/test_simulation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py index b0c6f5840e..61ee1b1aca 100644 --- a/tests/unit/test_simulation.py +++ b/tests/unit/test_simulation.py @@ -416,7 +416,7 @@ def test_drive_cycle_interpolant(self): # check warning raised if the largest gap in t_eval is bigger than the # smallest gap in the data with self.assertWarns(pybamm.SolverWarning): - sim.solve(t_eval=np.linspace(0, 1, 100)) + sim.solve(t_eval=np.linspace(0, 10, 3)) # check warning raised if t_eval doesnt contain time_data , but has a finer # resolution (can still solve, but good for users to know they dont have From f4d92511c8c64898f1ebff3054050ed0df995e17 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 09:23:50 +0100 Subject: [PATCH 06/28] add tests binary operators --- .../test_binary_operators.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/test_expression_tree/test_binary_operators.py b/tests/unit/test_expression_tree/test_binary_operators.py index 2625df5da4..b268fb3491 100644 --- a/tests/unit/test_expression_tree/test_binary_operators.py +++ b/tests/unit/test_expression_tree/test_binary_operators.py @@ -22,6 +22,8 @@ def test_binary_operator(self): bin2 = pybamm.BinaryOperator("binary test", c, d) with self.assertRaises(NotImplementedError): bin2.evaluate() + with self.assertRaises(NotImplementedError): + bin2._binary_jac(a, b) def test_binary_operator_domains(self): # same domain @@ -265,6 +267,19 @@ def test_inner(self): # check doesn't evaluate on edges anymore self.assertEqual(model.variables["inner"].evaluates_on_edges("primary"), False) + # check _binary_jac + a = pybamm.Symbol("a") + b = pybamm.Symbol("b") + + inner = pybamm.inner(2, i) + self.assertEqual(inner._binary_jac(a, b), 2 * b) + + inner = pybamm.inner(i, 2) + self.assertEqual(inner._binary_jac(a, b), 2 * a) + + inner = pybamm.inner(i, i) + self.assertEqual(inner._binary_jac(a, b), i * a + i * b) + def test_source(self): u = pybamm.Variable("u", domain="current collector") v = pybamm.Variable("v", domain="current collector") @@ -303,6 +318,7 @@ def test_equality(self): self.assertEqual(equal.evaluate(y=np.array([1])), 1) self.assertEqual(equal.evaluate(y=np.array([2])), 0) self.assertEqual(str(equal), "1.0 == y[0:1]") + self.assertEqual(equal.diff(b), 0) def test_sigmoid(self): a = pybamm.Scalar(1) From ffc17f22bb65f3ab5717ed7ea86f109c03811909 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 09:24:10 +0100 Subject: [PATCH 07/28] add tests simulation --- tests/unit/test_simulation.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py index 61ee1b1aca..0e60d03089 100644 --- a/tests/unit/test_simulation.py +++ b/tests/unit/test_simulation.py @@ -45,6 +45,15 @@ def test_basic_ops(self): self.assertTrue(val.has_symbol_of_classes(pybamm.Parameter)) self.assertFalse(val.has_symbol_of_classes(pybamm.Matrix)) + self.assertEqual(sim.submesh_types, model.default_submesh_types) + self.assertEqual(sim.var_pts, model.default_var_pts) + self.assertIsNone(sim.mesh) + for key in sim.spatial_methods.keys(): + self.assertEqual( + sim.spatial_methods[key].__class__, + model.default_spatial_methods[key].__class__ + ) + sim.build() self.assertFalse(sim._mesh is None) self.assertFalse(sim._disc is None) From c9f21073841a6e5674d5f48b252b38c31037a5aa Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 09:28:39 +0100 Subject: [PATCH 08/28] flake8 --- tests/unit/test_models/test_event.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_models/test_event.py b/tests/unit/test_models/test_event.py index 356bf7131a..939f5acd0f 100644 --- a/tests/unit/test_models/test_event.py +++ b/tests/unit/test_models/test_event.py @@ -48,7 +48,6 @@ def test_event_types(self): self.assertEqual(event.event_type, event_type) - if __name__ == "__main__": print("Add -v for more debug output") import sys From bb0f545f85b52904f1f4c43a1918c0145129983d Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 09:32:23 +0100 Subject: [PATCH 09/28] flake8 --- pybamm/models/event.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pybamm/models/event.py b/pybamm/models/event.py index b43566ddb6..e93262641d 100644 --- a/pybamm/models/event.py +++ b/pybamm/models/event.py @@ -12,7 +12,8 @@ class EventType(Enum): should return the time that the discontinuity occurs. The solver will integrate up to the discontinuity and then restart just after the discontinuity. - INTERPOLANT_EXTRAPOLATION indicates that a pybamm.Interpolant object has been evaluated outside of the range. + INTERPOLANT_EXTRAPOLATION indicates that a pybamm.Interpolant object has been + evaluated outside of the range. SWITCH indicates an event switch that is used in CasADI "fast with events" model. """ From 008e7d937391a49a2ddba112fa7470c2a66668e7 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 12:07:03 +0100 Subject: [PATCH 10/28] add test to cover jac_algebraic case --- .../test_discretisation.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/unit/test_discretisations/test_discretisation.py b/tests/unit/test_discretisations/test_discretisation.py index de4e061974..3f5aa7176b 100644 --- a/tests/unit/test_discretisations/test_discretisation.py +++ b/tests/unit/test_discretisations/test_discretisation.py @@ -850,6 +850,58 @@ def test_process_model_dae(self): with self.assertRaises(pybamm.ModelError): disc.process_model(model) + def test_process_model_algebraic(self): + # TODO: implement this based on test_process_model_dae + # one rhs equation and one algebraic + whole_cell = ["negative electrode", "separator", "positive electrode"] + c = pybamm.Variable("c", domain=whole_cell) + N = pybamm.grad(c) + Q = pybamm.Scalar(1) + model = pybamm.BaseModel() + model.algebraic = {c: pybamm.div(N) - Q} + model.initial_conditions = {c: pybamm.Scalar(0)} + + model.boundary_conditions = { + c: {"left": (0, "Dirichlet"), "right": (0, "Dirichlet")} + } + model.variables = {"c": c, "N": N} + + # create discretisation + disc = get_discretisation_for_testing() + mesh = disc.mesh + + disc.process_model(model) + combined_submesh = mesh.combine_submeshes(*whole_cell) + + y0 = model.concatenated_initial_conditions.evaluate() + np.testing.assert_array_equal( + y0, + np.zeros_like(combined_submesh.nodes)[:, np.newaxis], + ) + + # grad and div are identity operators here + np.testing.assert_array_equal( + model.concatenated_rhs.evaluate(None, y0), np.ones([0, 1]) + ) + + np.testing.assert_array_equal( + model.concatenated_algebraic.evaluate(None, y0), + -np.ones_like(combined_submesh.nodes[:, np.newaxis]), + ) + + # mass matrix is identity upper left, zeros elsewhere + mass = np.zeros( + (np.size(combined_submesh.nodes), np.size(combined_submesh.nodes)) + ) + np.testing.assert_array_equal( + mass, model.mass_matrix.entries.toarray() + ) + + # jacobian + y = pybamm.StateVector(slice(0, np.size(y0))) + jacobian = model.concatenated_algebraic.jac(y).evaluate(0, y0) + np.testing.assert_array_equal(np.eye(combined_submesh.npts), jacobian.toarray()) + def test_process_model_concatenation(self): # concatenation of variables as the key cn = pybamm.Variable("c", domain=["negative electrode"]) From 23b4628893145812c9697a62f474aea87f9a26d0 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 15:50:12 +0100 Subject: [PATCH 11/28] add some no cover lines --- pybamm/expression_tree/operations/evaluate_python.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pybamm/expression_tree/operations/evaluate_python.py b/pybamm/expression_tree/operations/evaluate_python.py index 3323be5e6b..5fda112574 100644 --- a/pybamm/expression_tree/operations/evaluate_python.py +++ b/pybamm/expression_tree/operations/evaluate_python.py @@ -38,7 +38,7 @@ class JaxCooMatrix: """ def __init__(self, row, col, data, shape): - if not pybamm.have_jax(): + if not pybamm.have_jax(): # pragma: no cover raise ModuleNotFoundError( "Jax or jaxlib is not installed, please see https://pybamm.readthedocs.io/en/latest/install/GNU-linux.html#optional-jaxsolver" # noqa: E501 ) @@ -413,7 +413,7 @@ def to_python(symbol, debug=False, output_jax=False): line_format = "{} = {}" - if debug: + if debug: # pragma: no cover variable_lines = [ "print('{}'); ".format( line_format.format(id_to_python_variable(symbol_id, False), symbol_line) @@ -540,7 +540,7 @@ class EvaluatorJax: """ def __init__(self, symbol): - if not pybamm.have_jax(): + if not pybamm.have_jax(): # pragma: no cover raise ModuleNotFoundError( "Jax or jaxlib is not installed, please see https://pybamm.readthedocs.io/en/latest/install/GNU-linux.html#optional-jaxsolver" # noqa: E501 ) From cd7640c589cc9519047051e545a1b7b38435e56e Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 16:09:32 +0100 Subject: [PATCH 12/28] add tests and fix raise error --- pybamm/expression_tree/concatenations.py | 2 +- .../test_expression_tree/test_concatenations.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/pybamm/expression_tree/concatenations.py b/pybamm/expression_tree/concatenations.py index 66414b9b4c..b3abef26f6 100644 --- a/pybamm/expression_tree/concatenations.py +++ b/pybamm/expression_tree/concatenations.py @@ -117,7 +117,7 @@ def _concatenation_new_copy(self, children): def _concatenation_jac(self, children_jacs): """Calculate the jacobian of a concatenation.""" - return NotImplementedError + raise NotImplementedError def _evaluate_for_shape(self): """See :meth:`pybamm.Symbol.evaluate_for_shape`""" diff --git a/tests/unit/test_expression_tree/test_concatenations.py b/tests/unit/test_expression_tree/test_concatenations.py index 6d9c95e1f2..df05511307 100644 --- a/tests/unit/test_expression_tree/test_concatenations.py +++ b/tests/unit/test_expression_tree/test_concatenations.py @@ -45,6 +45,13 @@ def test_base_concatenation(self): b = pybamm.Variable("b", domain="test b") with self.assertRaisesRegex(TypeError, "ConcatenationVariable"): pybamm.Concatenation(a, b) + + # base concatenation jacobian + a = pybamm.Symbol("a", domain="test a") + b = pybamm.Symbol("b", domain="test b") + conc3 = pybamm.Concatenation(a, b) + with self.assertRaises(NotImplementedError): + conc3._concatenation_jac(None) def test_concatenation_domains(self): a = pybamm.Symbol("a", domain=["negative electrode"]) @@ -135,6 +142,9 @@ def test_numpy_concatenation_vectors(self): conc = pybamm.NumpyConcatenation(a, b, c) y = np.linspace(0, 1, 23)[:, np.newaxis] np.testing.assert_array_equal(conc.evaluate(None, y), y) + # empty concatenation + conc = pybamm.NumpyConcatenation() + self.assertEqual(conc._concatenation_jac(None), 0) def test_numpy_concatenation_vector_scalar(self): # with entries @@ -176,6 +186,13 @@ def test_domain_concatenation_domains(self): ], ) + conc.secondary_dimensions_npts = 2 + with self.assertRaisesRegex( + ValueError, "Concatenation and children must have" + ): + conc.create_slices(None) + + def test_concatenation_orphans(self): a = pybamm.Variable("a", domain=["negative electrode"]) b = pybamm.Variable("b", domain=["separator"]) From a0107c808a35927e542a1559172ae1b48aa040ee Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 10 Aug 2022 16:10:43 +0100 Subject: [PATCH 13/28] flake8 --- tests/unit/test_expression_tree/test_concatenations.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_expression_tree/test_concatenations.py b/tests/unit/test_expression_tree/test_concatenations.py index df05511307..2a1b834d6b 100644 --- a/tests/unit/test_expression_tree/test_concatenations.py +++ b/tests/unit/test_expression_tree/test_concatenations.py @@ -45,7 +45,7 @@ def test_base_concatenation(self): b = pybamm.Variable("b", domain="test b") with self.assertRaisesRegex(TypeError, "ConcatenationVariable"): pybamm.Concatenation(a, b) - + # base concatenation jacobian a = pybamm.Symbol("a", domain="test a") b = pybamm.Symbol("b", domain="test b") @@ -187,12 +187,9 @@ def test_domain_concatenation_domains(self): ) conc.secondary_dimensions_npts = 2 - with self.assertRaisesRegex( - ValueError, "Concatenation and children must have" - ): + with self.assertRaisesRegex(ValueError, "Concatenation and children must have"): conc.create_slices(None) - def test_concatenation_orphans(self): a = pybamm.Variable("a", domain=["negative electrode"]) b = pybamm.Variable("b", domain=["separator"]) From 948af41708f9d79d5bee089bc7617126adeb9565 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 12:28:54 +0100 Subject: [PATCH 14/28] add test copy scalar --- tests/unit/test_expression_tree/test_scalar.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/test_expression_tree/test_scalar.py b/tests/unit/test_expression_tree/test_scalar.py index 52dd1faa63..8b624aec49 100644 --- a/tests/unit/test_expression_tree/test_scalar.py +++ b/tests/unit/test_expression_tree/test_scalar.py @@ -38,6 +38,11 @@ def test_to_equation(self): b.print_name = "test" self.assertEqual(str(b.to_equation()), "test") + def test_copy(self): + a = pybamm.Scalar(5) + b = a.create_copy() + self.assertEqual(a, b) + if __name__ == "__main__": print("Add -v for more debug output") From ec7a29b1f65e1ee70a16fe59ba456c62daa7131e Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 12:32:14 +0100 Subject: [PATCH 15/28] test error evaluate y_dot=None --- tests/unit/test_expression_tree/test_state_vector.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/test_expression_tree/test_state_vector.py b/tests/unit/test_expression_tree/test_state_vector.py index 85a022de2c..e586ad8019 100644 --- a/tests/unit/test_expression_tree/test_state_vector.py +++ b/tests/unit/test_expression_tree/test_state_vector.py @@ -78,6 +78,13 @@ def test_evaluate(self): ): sv.evaluate(y_dot=y_dot2) + # Try evaluating with y_dot=None + with self.assertRaisesRegex( + TypeError, + "StateVectorDot cannot evaluate input 'y_dot=None'", + ): + sv.evaluate(y_dot=None) + def test_name(self): sv = pybamm.StateVectorDot(slice(0, 10)) self.assertEqual(sv.name, "y_dot[0:10]") From e5a923e72e8e817a07e431e5baa552b9bedb1d77 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 13:08:06 +0100 Subject: [PATCH 16/28] increase coverage symbol.py --- pybamm/expression_tree/symbol.py | 4 ++-- tests/unit/test_expression_tree/test_symbol.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/pybamm/expression_tree/symbol.py b/pybamm/expression_tree/symbol.py index f0c7f166e3..2d26c74ca5 100644 --- a/pybamm/expression_tree/symbol.py +++ b/pybamm/expression_tree/symbol.py @@ -811,7 +811,7 @@ def evaluate_ignoring_errors(self, t=0): return None elif error.args[0] == "StateVectorDot cannot evaluate input 'y_dot=None'": return None - else: + else: # pragma: no cover raise error except ValueError as e: # return None if specific ValueError is raised @@ -891,7 +891,7 @@ def create_copy(self): """ raise NotImplementedError( """method self.new_copy() not implemented - for symbol {!s} of type {}""".format( + for symbol {!s} of type {}""".format( self, type(self) ) ) diff --git a/tests/unit/test_expression_tree/test_symbol.py b/tests/unit/test_expression_tree/test_symbol.py index 9ed32588a5..8f4039c837 100644 --- a/tests/unit/test_expression_tree/test_symbol.py +++ b/tests/unit/test_expression_tree/test_symbol.py @@ -165,6 +165,13 @@ def test_symbol_methods(self): ): a + "two" + def test_symbol_create_copy(self): + a = pybamm.Symbol("a") + with self.assertRaisesRegex( + NotImplementedError, "method self.new_copy()" + ): + a.create_copy() + def test_sigmoid(self): # Test that smooth heaviside is used when the setting is changed a = pybamm.Symbol("a") @@ -244,6 +251,15 @@ def test_evaluate_ignoring_errors(self): self.assertEqual(pybamm.t.evaluate_ignoring_errors(t=0), 0) self.assertIsNone(pybamm.Parameter("a").evaluate_ignoring_errors()) self.assertIsNone(pybamm.StateVector(slice(0, 1)).evaluate_ignoring_errors()) + self.assertIsNone(pybamm.StateVectorDot(slice(0, 1)).evaluate_ignoring_errors()) + + M1 = pybamm.Matrix(np.zeros((2, 3))) + M2 = pybamm.Matrix(np.ones((4, 5))) + with self.assertRaisesRegex( + pybamm.ShapeError, "Cannot find shape" + ): + pybamm.inner(M1, M2).evaluate_ignoring_errors() + np.testing.assert_array_equal( pybamm.InputParameter("a").evaluate_ignoring_errors(), np.nan ) From 593764d659e822561c9dbe7bc92c4d3dc8f4d840 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 13:09:38 +0100 Subject: [PATCH 17/28] flake8 --- tests/unit/test_expression_tree/test_symbol.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_expression_tree/test_symbol.py b/tests/unit/test_expression_tree/test_symbol.py index 8f4039c837..5166a75312 100644 --- a/tests/unit/test_expression_tree/test_symbol.py +++ b/tests/unit/test_expression_tree/test_symbol.py @@ -167,9 +167,7 @@ def test_symbol_methods(self): def test_symbol_create_copy(self): a = pybamm.Symbol("a") - with self.assertRaisesRegex( - NotImplementedError, "method self.new_copy()" - ): + with self.assertRaisesRegex(NotImplementedError, "method self.new_copy()"): a.create_copy() def test_sigmoid(self): @@ -253,12 +251,10 @@ def test_evaluate_ignoring_errors(self): self.assertIsNone(pybamm.StateVector(slice(0, 1)).evaluate_ignoring_errors()) self.assertIsNone(pybamm.StateVectorDot(slice(0, 1)).evaluate_ignoring_errors()) - M1 = pybamm.Matrix(np.zeros((2, 3))) - M2 = pybamm.Matrix(np.ones((4, 5))) - with self.assertRaisesRegex( - pybamm.ShapeError, "Cannot find shape" - ): - pybamm.inner(M1, M2).evaluate_ignoring_errors() + with self.assertRaisesRegex(pybamm.ShapeError, "Cannot find shape"): + pybamm.inner( + pybamm.Matrix(np.zeros((2, 3))), pybamm.Matrix(np.ones((4, 5))) + ).evaluate_ignoring_errors() np.testing.assert_array_equal( pybamm.InputParameter("a").evaluate_ignoring_errors(), np.nan From 55320f667795f6cf5e95695b4a1a22f3d2644631 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 13:37:31 +0100 Subject: [PATCH 18/28] comment out test --- tests/unit/test_expression_tree/test_symbol.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_expression_tree/test_symbol.py b/tests/unit/test_expression_tree/test_symbol.py index 5166a75312..7209d72dc2 100644 --- a/tests/unit/test_expression_tree/test_symbol.py +++ b/tests/unit/test_expression_tree/test_symbol.py @@ -251,10 +251,10 @@ def test_evaluate_ignoring_errors(self): self.assertIsNone(pybamm.StateVector(slice(0, 1)).evaluate_ignoring_errors()) self.assertIsNone(pybamm.StateVectorDot(slice(0, 1)).evaluate_ignoring_errors()) - with self.assertRaisesRegex(pybamm.ShapeError, "Cannot find shape"): - pybamm.inner( - pybamm.Matrix(np.zeros((2, 3))), pybamm.Matrix(np.ones((4, 5))) - ).evaluate_ignoring_errors() + # with self.assertRaisesRegex(pybamm.ShapeError, "Cannot find shape"): + # pybamm.inner( + # pybamm.Matrix(np.zeros((2, 3))), pybamm.Matrix(np.ones((4, 5))) + # ).evaluate_ignoring_errors() np.testing.assert_array_equal( pybamm.InputParameter("a").evaluate_ignoring_errors(), np.nan From b1d7fbcea81c60bb2a3742f8370a03f08adfce19 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 14:51:26 +0100 Subject: [PATCH 19/28] change general error statement in symbol.py --- pybamm/expression_tree/symbol.py | 7 ++++--- tests/unit/test_expression_tree/test_symbol.py | 5 ----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pybamm/expression_tree/symbol.py b/pybamm/expression_tree/symbol.py index 2d26c74ca5..7a3bb4cc92 100644 --- a/pybamm/expression_tree/symbol.py +++ b/pybamm/expression_tree/symbol.py @@ -813,12 +813,13 @@ def evaluate_ignoring_errors(self, t=0): return None else: # pragma: no cover raise error - except ValueError as e: + except ValueError as error: # return None if specific ValueError is raised # (there is a e.g. Time in the tree) - if e.args[0] == "t must be provided": + if error.args[0] == "t must be provided": return None - raise pybamm.ShapeError("Cannot find shape (original error: {})".format(e)) + else: # pragma: no cover + return error return result def evaluates_to_number(self): diff --git a/tests/unit/test_expression_tree/test_symbol.py b/tests/unit/test_expression_tree/test_symbol.py index 7209d72dc2..08dba77b69 100644 --- a/tests/unit/test_expression_tree/test_symbol.py +++ b/tests/unit/test_expression_tree/test_symbol.py @@ -251,11 +251,6 @@ def test_evaluate_ignoring_errors(self): self.assertIsNone(pybamm.StateVector(slice(0, 1)).evaluate_ignoring_errors()) self.assertIsNone(pybamm.StateVectorDot(slice(0, 1)).evaluate_ignoring_errors()) - # with self.assertRaisesRegex(pybamm.ShapeError, "Cannot find shape"): - # pybamm.inner( - # pybamm.Matrix(np.zeros((2, 3))), pybamm.Matrix(np.ones((4, 5))) - # ).evaluate_ignoring_errors() - np.testing.assert_array_equal( pybamm.InputParameter("a").evaluate_ignoring_errors(), np.nan ) From 27886889d4f9548cfc9676a8422c2e2c512e897c Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 15:21:03 +0100 Subject: [PATCH 20/28] revert to previous and add no cover --- pybamm/expression_tree/symbol.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pybamm/expression_tree/symbol.py b/pybamm/expression_tree/symbol.py index 7a3bb4cc92..47f324a97e 100644 --- a/pybamm/expression_tree/symbol.py +++ b/pybamm/expression_tree/symbol.py @@ -818,8 +818,9 @@ def evaluate_ignoring_errors(self, t=0): # (there is a e.g. Time in the tree) if error.args[0] == "t must be provided": return None - else: # pragma: no cover - return error + raise pybamm.ShapeError( + f"Cannot find shape (original error: {error})" + ) # pragma: no cover return result def evaluates_to_number(self): From cc1c868a787245413fa6948a9723725437aa508b Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 23 Aug 2022 15:22:30 +0100 Subject: [PATCH 21/28] flake8 --- pybamm/expression_tree/symbol.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pybamm/expression_tree/symbol.py b/pybamm/expression_tree/symbol.py index 47f324a97e..b12cbc535c 100644 --- a/pybamm/expression_tree/symbol.py +++ b/pybamm/expression_tree/symbol.py @@ -820,7 +820,7 @@ def evaluate_ignoring_errors(self, t=0): return None raise pybamm.ShapeError( f"Cannot find shape (original error: {error})" - ) # pragma: no cover + ) # pragma: no cover return result def evaluates_to_number(self): From 6142676601f643d6afb204e842c205a03ac3ae42 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 24 Aug 2022 10:31:31 +0100 Subject: [PATCH 22/28] try if linsolver works --- pybamm/solvers/scikits_ode_solver.py | 2 +- .../unit/test_solvers/test_scikits_solvers.py | 41 ++++++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/pybamm/solvers/scikits_ode_solver.py b/pybamm/solvers/scikits_ode_solver.py index 186b8f04eb..5187fe9ea7 100644 --- a/pybamm/solvers/scikits_ode_solver.py +++ b/pybamm/solvers/scikits_ode_solver.py @@ -51,7 +51,7 @@ def __init__( extrap_tol=0, extra_options=None, ): - if scikits_odes_spec is None: + if scikits_odes_spec is None: # pragma: no cover raise ImportError("scikits.odes is not installed") super().__init__(method, rtol, atol, extrap_tol=extrap_tol) diff --git a/tests/unit/test_solvers/test_scikits_solvers.py b/tests/unit/test_solvers/test_scikits_solvers.py index 7d3fef0c4d..c8ff13cd53 100644 --- a/tests/unit/test_solvers/test_scikits_solvers.py +++ b/tests/unit/test_solvers/test_scikits_solvers.py @@ -172,21 +172,34 @@ def test_model_solver_ode_jacobian_python(self): ) N = combined_submesh.npts - # Solve - solver = pybamm.ScikitsOdeSolver(rtol=1e-9, atol=1e-9) - t_eval = np.linspace(0, 1, 100) - solution = solver.solve(model, t_eval) - np.testing.assert_array_equal(solution.t, t_eval) + # Solve testing various linear solvers + linsolvers = [ + "dense", + "lapackdense", + "spgmr", + "spbcgs", + "sptfqmr", + ] - T, Y = solution.t, solution.y - np.testing.assert_array_almost_equal( - model.variables["var1"].evaluate(T, Y), - np.ones((N, T.size)) * np.exp(T[np.newaxis, :]), - ) - np.testing.assert_array_almost_equal( - model.variables["var2"].evaluate(T, Y), - np.ones((N, T.size)) * (T[np.newaxis, :] - np.exp(T[np.newaxis, :])), - ) + for linsolver in linsolvers: + solver = pybamm.ScikitsOdeSolver( + rtol=1e-9, + atol=1e-9, + # extra_options={linsolver: linsolver} + ) + t_eval = np.linspace(0, 1, 100) + solution = solver.solve(model, t_eval) + np.testing.assert_array_equal(solution.t, t_eval) + + T, Y = solution.t, solution.y + np.testing.assert_array_almost_equal( + model.variables["var1"].evaluate(T, Y), + np.ones((N, T.size)) * np.exp(T[np.newaxis, :]), + ) + np.testing.assert_array_almost_equal( + model.variables["var2"].evaluate(T, Y), + np.ones((N, T.size)) * (T[np.newaxis, :] - np.exp(T[np.newaxis, :])), + ) def test_model_solver_dae_python(self): model = pybamm.BaseModel() From 01414e1fb003b9b9d503e3f035f5938b48b20200 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 24 Aug 2022 10:51:09 +0100 Subject: [PATCH 23/28] add test BoundaryGradient --- .../unit/test_spatial_methods/test_scikit_finite_element.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/test_spatial_methods/test_scikit_finite_element.py b/tests/unit/test_spatial_methods/test_scikit_finite_element.py index a45d2dfeff..815471698e 100644 --- a/tests/unit/test_spatial_methods/test_scikit_finite_element.py +++ b/tests/unit/test_spatial_methods/test_scikit_finite_element.py @@ -420,6 +420,11 @@ def test_neg_pos(self): extrap_pos_disc.evaluate(None, constant_y), 1 ) + # test BoundaryGradient not implemented + extrap_neg = pybamm.BoundaryGradient(var, "negative tab") + with self.assertRaises(NotImplementedError): + disc.process_symbol(extrap_neg) + def test_boundary_integral(self): mesh = get_2p1d_mesh_for_testing(include_particles=False) spatial_methods = { From 73216c9c1831a12e420bfe5c8cc5672df709190d Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 24 Aug 2022 13:27:00 +0100 Subject: [PATCH 24/28] add test base spatial method --- tests/unit/test_spatial_methods/test_base_spatial_method.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/test_spatial_methods/test_base_spatial_method.py b/tests/unit/test_spatial_methods/test_base_spatial_method.py index bfc6ceddec..b5543c33b0 100644 --- a/tests/unit/test_spatial_methods/test_base_spatial_method.py +++ b/tests/unit/test_spatial_methods/test_base_spatial_method.py @@ -130,6 +130,11 @@ def test_boundary_value_checks(self): with self.assertRaisesRegex(TypeError, "Cannot process BoundaryGradient"): spatial_method.boundary_value_or_flux(symbol, child) + # test also symbol "right" + symbol = pybamm.BoundaryGradient(child, "right") + with self.assertRaisesRegex(TypeError, "Cannot process BoundaryGradient"): + spatial_method.boundary_value_or_flux(symbol, child) + mesh = get_1p1d_mesh_for_testing() spatial_method = pybamm.SpatialMethod() spatial_method.build(mesh) From a3701d56a0b5abf4cdd56a46a7526a4db17ce8f2 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 24 Aug 2022 13:29:49 +0100 Subject: [PATCH 25/28] add linsolver tests --- tests/unit/test_solvers/test_scikits_solvers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_solvers/test_scikits_solvers.py b/tests/unit/test_solvers/test_scikits_solvers.py index c8ff13cd53..9c08c4a22c 100644 --- a/tests/unit/test_solvers/test_scikits_solvers.py +++ b/tests/unit/test_solvers/test_scikits_solvers.py @@ -185,7 +185,7 @@ def test_model_solver_ode_jacobian_python(self): solver = pybamm.ScikitsOdeSolver( rtol=1e-9, atol=1e-9, - # extra_options={linsolver: linsolver} + extra_options={linsolver: linsolver} ) t_eval = np.linspace(0, 1, 100) solution = solver.solve(model, t_eval) From 3c37f46e6e7dd02c2c7f9341f294b3e76ac89337 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 24 Aug 2022 13:32:46 +0100 Subject: [PATCH 26/28] flake8 --- tests/unit/test_spatial_methods/test_base_spatial_method.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_spatial_methods/test_base_spatial_method.py b/tests/unit/test_spatial_methods/test_base_spatial_method.py index b5543c33b0..259173092e 100644 --- a/tests/unit/test_spatial_methods/test_base_spatial_method.py +++ b/tests/unit/test_spatial_methods/test_base_spatial_method.py @@ -7,7 +7,7 @@ from tests import ( get_mesh_for_testing, get_1p1d_mesh_for_testing, - get_size_distribution_mesh_for_testing + get_size_distribution_mesh_for_testing, ) @@ -133,7 +133,7 @@ def test_boundary_value_checks(self): # test also symbol "right" symbol = pybamm.BoundaryGradient(child, "right") with self.assertRaisesRegex(TypeError, "Cannot process BoundaryGradient"): - spatial_method.boundary_value_or_flux(symbol, child) + spatial_method.boundary_value_or_flux(symbol, child) mesh = get_1p1d_mesh_for_testing() spatial_method = pybamm.SpatialMethod() From c4f50b639a426254bed42820c1bfdad4c17b8246 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 24 Aug 2022 15:29:12 +0100 Subject: [PATCH 27/28] fix typo with options --- tests/unit/test_solvers/test_scikits_solvers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_solvers/test_scikits_solvers.py b/tests/unit/test_solvers/test_scikits_solvers.py index 9c08c4a22c..bfc993ccc7 100644 --- a/tests/unit/test_solvers/test_scikits_solvers.py +++ b/tests/unit/test_solvers/test_scikits_solvers.py @@ -185,7 +185,7 @@ def test_model_solver_ode_jacobian_python(self): solver = pybamm.ScikitsOdeSolver( rtol=1e-9, atol=1e-9, - extra_options={linsolver: linsolver} + extra_options={"linsolver": linsolver} ) t_eval = np.linspace(0, 1, 100) solution = solver.solve(model, t_eval) From 577f32577a14d08027608a4c0633f7f7229f3a9b Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Wed, 14 Sep 2022 13:57:05 +0100 Subject: [PATCH 28/28] Valentin's comments --- .../test_binary_operators.py | 13 ------------- .../test_operations/test_jac.py | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/unit/test_expression_tree/test_binary_operators.py b/tests/unit/test_expression_tree/test_binary_operators.py index f8a4ec6081..175cbf8986 100644 --- a/tests/unit/test_expression_tree/test_binary_operators.py +++ b/tests/unit/test_expression_tree/test_binary_operators.py @@ -267,19 +267,6 @@ def test_inner(self): # check doesn't evaluate on edges anymore self.assertEqual(model.variables["inner"].evaluates_on_edges("primary"), False) - # check _binary_jac - a = pybamm.Symbol("a") - b = pybamm.Symbol("b") - - inner = pybamm.inner(2, i) - self.assertEqual(inner._binary_jac(a, b), 2 * b) - - inner = pybamm.inner(i, 2) - self.assertEqual(inner._binary_jac(a, b), 2 * a) - - inner = pybamm.inner(i, i) - self.assertEqual(inner._binary_jac(a, b), i * a + i * b) - def test_source(self): u = pybamm.Variable("u", domain="current collector") v = pybamm.Variable("v", domain="current collector") diff --git a/tests/unit/test_expression_tree/test_operations/test_jac.py b/tests/unit/test_expression_tree/test_operations/test_jac.py index 98cb9b3578..692fd00878 100644 --- a/tests/unit/test_expression_tree/test_operations/test_jac.py +++ b/tests/unit/test_expression_tree/test_operations/test_jac.py @@ -279,6 +279,22 @@ def test_jac_of_unary_operator(self): with self.assertRaises(NotImplementedError): b.jac(y) + def test_jac_of_binary_operator(self): + a = pybamm.Symbol("a") + b = pybamm.Symbol("b") + + phi_s = pybamm.standard_variables.phi_s_n + i = pybamm.grad(phi_s) + + inner = pybamm.inner(2, i) + self.assertEqual(inner._binary_jac(a, b), 2 * b) + + inner = pybamm.inner(i, 2) + self.assertEqual(inner._binary_jac(a, b), 2 * a) + + inner = pybamm.inner(i, i) + self.assertEqual(inner._binary_jac(a, b), i * a + i * b) + def test_jac_of_independent_variable(self): a = pybamm.IndependentVariable("Variable") y = pybamm.StateVector(slice(0, 1))