#1429 codacy and coverage

pybamm/expression_tree/binary_operators.py

@@ -1112,6 +1112,25 @@ def simplified_division(left, right):
     if left.id == right.id:
         return pybamm.ones_like(left)
 
+    # anything multiplied by a matrix one returns itself if
+    # - the shapes are the same
+    # - both left and right evaluate on edges, or both evaluate on nodes, in all
+    # dimensions
+    # (and possibly more generally, but not implemented here)
+    try:
+        if left.shape_for_testing == right.shape_for_testing and all(
+            left.evaluates_on_edges(dim) == right.evaluates_on_edges(dim)
+            for dim in ["primary", "secondary", "tertiary"]
+        ):
+            if pybamm.is_matrix_one(right):
+                return left
+            # also check for negative one
+            if pybamm.is_matrix_minus_one(right):
+                return -left
+
+    except NotImplementedError:
+        pass
+
     # Return constant if both sides are constant
     if left.is_constant() and right.is_constant():
         return pybamm.simplify_if_constant(pybamm.Division(left, right))

pybamm/expression_tree/broadcasts.py

@@ -66,12 +66,6 @@ def broadcasts_to_nodes(self):
         else:
             return False
 
-    def reduce_one_dimension(self):
-        """
-        Reduce the broadcast by one dimension. See specific broadcast classes
-        """
-        raise NotImplementedError
-
 
 class PrimaryBroadcast(Broadcast):
     """A node in the expression tree representing a primary broadcasting operator.
@@ -156,7 +150,7 @@ def _evaluate_for_shape(self):
         return np.outer(child_eval, vec).reshape(-1, 1)
 
     def reduce_one_dimension(self):
-        """ See :meth:`pybamm.Broadcast.reduce_one_dimension()` """
+        """ Reduce the broadcast by one dimension. """
         return self.orphans[0]
 
 
@@ -261,6 +255,10 @@ def _evaluate_for_shape(self):
         vec = pybamm.evaluate_for_shape_using_domain(self.domain)
         return np.outer(vec, child_eval).reshape(-1, 1)
 
+    def reduce_one_dimension(self):
+        """ Reduce the broadcast by one dimension. """
+        raise NotImplementedError
+
 
 class SecondaryBroadcastToEdges(SecondaryBroadcast):
     """A secondary broadcast onto the edges of a domain."""
@@ -320,7 +318,7 @@ def _evaluate_for_shape(self):
         return child_eval * vec
 
     def reduce_one_dimension(self):
-        """ See :meth:`pybamm.Broadcast.reduce_one_dimension()` """
+        """ Reduce the broadcast by one dimension. """
         if self.auxiliary_domains == {}:
             return self.orphans[0]
         elif "tertiary" not in self.auxiliary_domains:
@@ -350,7 +348,7 @@ def _evaluates_on_edges(self, dimension):
         return True
 
     def reduce_one_dimension(self):
-        """ See :meth:`pybamm.Broadcast.reduce_one_dimension()` """
+        """ Reduce the broadcast by one dimension. """
         if self.auxiliary_domains == {}:
             return self.orphans[0]
         elif "tertiary" not in self.auxiliary_domains:
@@ -361,7 +359,7 @@ def reduce_one_dimension(self):
             return FullBroadcastToEdges(
                 self.orphans[0],
                 self.auxiliary_domains["secondary"],
-                self.auxiliary_domains["tertiary"],
+                {"secondary": self.auxiliary_domains["tertiary"]},
             )
 
 

pybamm/expression_tree/unary_operators.py

@@ -1166,14 +1166,11 @@ def x_average(symbol):
         and symbol.domain == ["negative electrode", "separator", "positive electrode"]
     ):
         a, b, c = [orp.orphans[0] for orp in symbol.orphans]
-        if a.id == b.id == c.id:
-            out = a
-        else:
-            geo = pybamm.geometric_parameters
-            l_n = geo.l_n
-            l_s = geo.l_s
-            l_p = geo.l_p
-            out = (l_n * a + l_s * b + l_p * c) / (l_n + l_s + l_p)
+        geo = pybamm.geometric_parameters
+        l_n = geo.l_n
+        l_s = geo.l_s
+        l_p = geo.l_p
+        out = (l_n * a + l_s * b + l_p * c) / (l_n + l_s + l_p)
         # To respect domains we may need to broadcast the child back out
         child = symbol.children[0]
         # If symbol being returned doesn't have empty domain, return it

tests/integration/test_solvers/test_external_variables.py

@@ -33,7 +33,7 @@ def test_on_dfn(self):
         )
         sim.solve(t_eval=np.linspace(0, 3600, 100), inputs=inputs)
 
-    def test_external_variables_SPMe(self):
+    def test_external_variables_SPMe_thermal(self):
         model_options = {"thermal": "lumped", "external submodels": ["thermal"]}
         model = pybamm.lithium_ion.SPMe(model_options)
         sim = pybamm.Simulation(model)
@@ -50,6 +50,14 @@ def test_external_variables_SPMe(self):
         sim.built_model.generate("test.c", ["Volume-averaged cell temperature"])
         os.remove("test.c")
 
+    def test_external_variables_SPMe_concentration(self):
+        model_options = {"external submodels": ["electrolyte diffusion"]}
+        model = pybamm.lithium_ion.SPMe(model_options)
+        self.assertEqual(
+            model.external_variables[0].id,
+            model.variables["Electrolyte concentration"].id,
+        )
+
 
 if __name__ == "__main__":
     import sys

tests/unit/test_expression_tree/test_binary_operators.py

@@ -403,9 +403,9 @@ def test_binary_simplifications(self):
         a = pybamm.Scalar(0, domain="domain")
         b = pybamm.Scalar(1)
         c = pybamm.Parameter("c")
-        e = pybamm.Scalar(2)
         v = pybamm.Vector(np.zeros((10, 1)))
         v1 = pybamm.Vector(np.ones((10, 1)))
+        f = pybamm.StateVector(slice(0, 10))
 
         var = pybamm.Variable("var", domain="domain")
         broad0 = pybamm.PrimaryBroadcast(0, "domain")
@@ -496,7 +496,7 @@ def test_binary_simplifications(self):
         self.assertIsInstance((c * a), pybamm.Scalar)
         self.assertEqual((c * a).evaluate(), 0)
         self.assertIsInstance((b * c), pybamm.Parameter)
-        self.assertIsInstance((e * c), pybamm.Multiplication)
+        self.assertIsInstance((2 * c), pybamm.Multiplication)
         # multiplication with -1
         self.assertEqual((c * -1).id, (-c).id)
         self.assertEqual((-1 * c).id, (-c).id)
@@ -513,10 +513,11 @@ def test_binary_simplifications(self):
         self.assertIsInstance((v * b), pybamm.Array)
         np.testing.assert_array_equal((v * b).evaluate(), np.zeros((10, 1)))
         # multiplication with matrix one
-        self.assertIsInstance((e * v1), pybamm.Array)
-        np.testing.assert_array_equal((e * v1).evaluate(), 2 * np.ones((10, 1)))
-        self.assertIsInstance((v1 * e), pybamm.Array)
-        np.testing.assert_array_equal((v1 * e).evaluate(), 2 * np.ones((10, 1)))
+        self.assertEqual((f * v1), f)
+        self.assertEqual((v1 * f), f)
+        # multiplication with matrix minus one
+        self.assertEqual((f * (-v1)).id, (-f).id)
+        self.assertEqual(((-v1) * f).id, (-f).id)
         # multiplication with broadcast
         self.assertEqual((var * broad2).id, (var * 2).id)
         self.assertEqual((broad2 * var).id, (2 * var).id)
@@ -537,8 +538,8 @@ def test_binary_simplifications(self):
         self.assertEqual((c / broad2).id, pybamm.PrimaryBroadcast(c / 2, "domain").id)
         self.assertEqual((broad2 / c).id, pybamm.PrimaryBroadcast(2 / c, "domain").id)
         # division with matrix one
-        self.assertIsInstance((e / v1), pybamm.Array)
-        np.testing.assert_array_equal((e / v1).evaluate(), 2 * np.ones((10, 1)))
+        self.assertEqual((f / v1), f)
+        self.assertEqual((f / -v1).id, (-f).id)
         # division by zero
         with self.assertRaises(ZeroDivisionError):
             b / a

tests/unit/test_expression_tree/test_broadcasts.py

@@ -13,6 +13,8 @@ def test_primary_broadcast(self):
         self.assertEqual(broad_a.name, "broadcast")
         self.assertEqual(broad_a.children[0].name, a.name)
         self.assertEqual(broad_a.domain, ["negative electrode"])
+        self.assertTrue(broad_a.broadcasts_to_nodes)
+        self.assertEqual(broad_a.reduce_one_dimension(), a)
 
         a = pybamm.Symbol(
             "a",
@@ -54,6 +56,10 @@ def test_secondary_broadcast(self):
             broad_a.auxiliary_domains,
             {"secondary": ["negative electrode"], "tertiary": ["current collector"]},
         )
+        self.assertTrue(broad_a.broadcasts_to_nodes)
+
+        with self.assertRaises(NotImplementedError):
+            broad_a.reduce_one_dimension()
 
         a = pybamm.Symbol("a")
         with self.assertRaisesRegex(TypeError, "empty domain"):
@@ -80,6 +86,24 @@ def test_full_broadcast(self):
         broad_a = pybamm.FullBroadcast(a, ["negative electrode"], "current collector")
         self.assertEqual(broad_a.domain, ["negative electrode"])
         self.assertEqual(broad_a.auxiliary_domains["secondary"], ["current collector"])
+        self.assertTrue(broad_a.broadcasts_to_nodes)
+        self.assertEqual(
+            broad_a.reduce_one_dimension().id,
+            pybamm.PrimaryBroadcast(a, "current collector").id,
+        )
+
+        broad_a = pybamm.FullBroadcast(a, ["negative electrode"], {})
+        self.assertEqual(broad_a.reduce_one_dimension(), a)
+
+        broad_a = pybamm.FullBroadcast(
+            a,
+            "negative particle",
+            {"secondary": "negative electrode", "tertiary": "current collector"},
+        )
+        self.assertEqual(
+            broad_a.reduce_one_dimension().id,
+            pybamm.FullBroadcast(a, "negative electrode", "current collector").id,
+        )
 
     def test_full_broadcast_number(self):
         broad_a = pybamm.FullBroadcast(1, ["negative electrode"], None)
@@ -117,12 +141,17 @@ def test_ones_like(self):
 
     def test_broadcast_to_edges(self):
         a = pybamm.Symbol("a")
+
+        # primary
         broad_a = pybamm.PrimaryBroadcastToEdges(a, ["negative electrode"])
         self.assertEqual(broad_a.name, "broadcast to edges")
         self.assertEqual(broad_a.children[0].name, a.name)
         self.assertEqual(broad_a.domain, ["negative electrode"])
         self.assertTrue(broad_a.evaluates_on_edges("primary"))
+        self.assertFalse(broad_a.broadcasts_to_nodes)
+        self.assertEqual(broad_a.reduce_one_dimension(), a)
 
+        # secondary
         a = pybamm.Symbol(
             "a",
             domain=["negative particle"],
@@ -135,14 +164,35 @@ def test_broadcast_to_edges(self):
             {"secondary": ["negative electrode"], "tertiary": ["current collector"]},
         )
         self.assertTrue(broad_a.evaluates_on_edges("primary"))
+        self.assertFalse(broad_a.broadcasts_to_nodes)
 
+        # full
         a = pybamm.Symbol("a")
         broad_a = pybamm.FullBroadcastToEdges(
             a, ["negative electrode"], "current collector"
         )
         self.assertEqual(broad_a.domain, ["negative electrode"])
         self.assertEqual(broad_a.auxiliary_domains["secondary"], ["current collector"])
         self.assertTrue(broad_a.evaluates_on_edges("primary"))
+        self.assertFalse(broad_a.broadcasts_to_nodes)
+        self.assertEqual(
+            broad_a.reduce_one_dimension().id,
+            pybamm.PrimaryBroadcastToEdges(a, "current collector").id,
+        )
+        broad_a = pybamm.FullBroadcastToEdges(a, ["negative electrode"], {})
+        self.assertEqual(broad_a.reduce_one_dimension(), a)
+
+        broad_a = pybamm.FullBroadcastToEdges(
+            a,
+            "negative particle",
+            {"secondary": "negative electrode", "tertiary": "current collector"},
+        )
+        self.assertEqual(
+            broad_a.reduce_one_dimension().id,
+            pybamm.FullBroadcastToEdges(
+                a, "negative electrode", "current collector"
+            ).id,
+        )
 
 
 if __name__ == "__main__":

tests/unit/test_expression_tree/test_operations/test_copy.py

@@ -15,6 +15,7 @@ def test_symbol_new_copy(self):
         x_n = pybamm.standard_spatial_vars.x_n
         v_s = pybamm.Variable("v", "separator")
         vec = pybamm.Vector([1, 2, 3, 4, 5])
+        mat = pybamm.Matrix([[1, 2], [3, 4]])
         mesh = get_mesh_for_testing()
 
         for symbol in [
@@ -56,6 +57,7 @@ def test_symbol_new_copy(self):
             ),
             pybamm.minimum(a, b),
             pybamm.maximum(a, b),
+            pybamm.SparseStack(mat, mat),
         ]:
             self.assertEqual(symbol.id, symbol.new_copy().id)
 

tests/unit/test_expression_tree/test_symbol.py

@@ -95,7 +95,8 @@ def test_symbol_methods(self):
         self.assertIsInstance(-a, pybamm.Negate)
         self.assertIsInstance(abs(a), pybamm.AbsoluteValue)
         # special cases
-        self.assertEqual(-(-a), a)
+        neg_a = -a
+        self.assertEqual(-neg_a, a)
         abs_a = abs(a)
         self.assertEqual(abs(abs_a), abs_a)
 

tests/unit/test_expression_tree/test_unary_operators.py

@@ -471,6 +471,14 @@ def test_boundary_value(self):
             pybamm.boundary_value(var, "negative tab")
             pybamm.boundary_value(var, "positive tab")
 
+        # boundary value of symbol that evaluates on edges raises error
+        symbol_on_edges = pybamm.PrimaryBroadcastToEdges(1, "domain")
+        with self.assertRaisesRegex(
+            ValueError,
+            "Can't take the boundary value of a symbol that evaluates on edges",
+        ):
+            pybamm.boundary_value(symbol_on_edges, "right")
+
     def test_x_average(self):
         a = pybamm.Scalar(4)
         average_a = pybamm.x_average(a)