#2418 working on scales

pybamm/discretisations/discretisation.py

@@ -539,15 +539,12 @@ def process_boundary_conditions(self, model):
             if any("tab" in side for side in list(bcs.keys())):
                 bcs = self.check_tab_conditions(key, bcs)
 
-            # Calculate scale if the key has a scale
-            scale = getattr(key, "scale", 1)
-
             # Process boundary conditions
             for side, bc in bcs.items():
                 eqn, typ = bc
                 pybamm.logger.debug("Discretise {} ({} bc)".format(key, side))
                 processed_eqn = self.process_symbol(eqn)
-                processed_bcs[key][side] = (processed_eqn / scale, typ)
+                processed_bcs[key][side] = (processed_eqn, typ)
 
         return processed_bcs
 
@@ -946,7 +943,7 @@ def _process_symbol(self, symbol):
                     *self.y_slices[symbol.get_variable()],
                     domains=symbol.domains,
                 )
-                # * symbol.scale
+                * symbol.scale
             )
 
         elif isinstance(symbol, pybamm.Variable):
@@ -993,9 +990,9 @@ def _process_symbol(self, symbol):
                     )
                 # Multiply the output by the symbol's scale so that the state vector
                 # is of order 1
-                return pybamm.StateVector(
-                    *y_slices, domains=symbol.domains
-                )  # * symbol.scale
+                return (
+                    pybamm.StateVector(*y_slices, domains=symbol.domains) * symbol.scale
+                )
 
         elif isinstance(symbol, pybamm.SpatialVariable):
             return spatial_method.spatial_variable(symbol)

pybamm/expression_tree/concatenations.py

@@ -42,6 +42,7 @@ def __init__(self, *children, name=None, check_domain=True, concat_fun=None):
         else:
             domains = {"primary": []}
         self.concatenation_function = concat_fun
+
         super().__init__(name, children, domains=domains)
 
     def __str__(self):
@@ -91,6 +92,13 @@ def get_children_domains(self, children):
 
         return domains
 
+    def set_scale(self):
+        if len(self.children) > 0:
+            if all(child.scale == self.children[0].scale for child in self.children):
+                self._scale = self.children[0].scale
+            else:
+                raise ValueError("Cannot concatenate symbols with different scales")
+
     def _concatenation_evaluate(self, children_eval):
         """See :meth:`Concatenation._concatenation_evaluate()`."""
         if len(children_eval) == 0:

pybamm/expression_tree/symbol.py

@@ -215,6 +215,9 @@ def __init__(
         # Set domains (and hence id)
         self.domains = self.read_domain_or_domains(domain, auxiliary_domains, domains)
 
+        # Set scale
+        self.set_scale()
+
         self._saved_evaluates_on_edges = {}
         self._print_name = None
 
@@ -404,6 +407,22 @@ def set_id(self):
             + tuple([(k, tuple(v)) for k, v in self.domains.items() if v != []])
         )
 
+    @property
+    def scale(self):
+        return self._scale
+
+    def set_scale(self):
+        scale = None
+        for child in self.children:
+            if child.scale != 1:
+                if scale is None:
+                    scale = child.scale
+                elif scale != child.scale:
+                    raise ValueError("Cannot scale children with different scales")
+        if scale is None:
+            scale = 1
+        self._scale = scale
+
     def __eq__(self, other):
         try:
             return self._id == other._id

pybamm/expression_tree/variable.py

@@ -71,7 +71,11 @@ def __init__(
         self.print_name = print_name
         if isinstance(scale, numbers.Number):
             scale = pybamm.Scalar(scale)
-        self.scale = scale
+        self._scale = scale
+
+    def set_scale(self):
+        # scale is set in init
+        pass
 
     def create_copy(self):
         """See :meth:`pybamm.Symbol.new_copy()`."""

pybamm/models/full_battery_models/lithium_ion/basic_dfn_dimensional.py

@@ -48,17 +48,17 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
         c_e_n = pybamm.Variable(
             "Negative electrolyte concentration [mol.m-3]",
             domain="negative electrode",
-            scale=1 + 0 * param.c_e_typ,
+            scale=param.c_e_typ,
         )
         c_e_s = pybamm.Variable(
             "Separator electrolyte concentration [mol.m-3]",
             domain="separator",
-            scale=1 + 0 * param.c_e_typ,
+            scale=param.c_e_typ,
         )
         c_e_p = pybamm.Variable(
             "Positive electrolyte concentration [mol.m-3]",
             domain="positive electrode",
-            scale=1 + 0 * param.c_e_typ,
+            scale=param.c_e_typ,
         )
         # Concatenations combine several variables into a single variable, to simplify
         # implementing equations that hold over several domains
@@ -90,19 +90,14 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
             "Negative particle concentration",
             domain="negative particle",
             auxiliary_domains={"secondary": "negative electrode"},
-            scale=1 + 0 * param.n.prim.c_max,
+            scale=param.n.prim.c_max,
         )
         c_s_p = pybamm.Variable(
             "Positive particle concentration",
             domain="positive particle",
             auxiliary_domains={"secondary": "positive electrode"},
             scale=param.p.prim.c_max,
         )
-        c_s_p_nondim = c_s_p
-        c_s_p_dim = c_s_p * param.p.prim.c_max
-        c_scale = param.p.prim.c_max
-        # c_s_p_dim = c_s_p
-        # c_s_p_nondim = c_s_p / param.p.prim.c_max
 
         # Constant temperature
         T = param.T_init_dim
@@ -153,10 +148,13 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
         Feta_RT_n = param.F * eta_n / (param.R * T)
         j_n = 2 * j0_n * pybamm.sinh(param.n.prim.ne / 2 * Feta_RT_n)
         # j_n = pybamm.PrimaryBroadcast(i_cell / (a_n * param.n.L), "negative electrode")
-        c_s_surf_p_dim = pybamm.surf(c_s_p_dim)
-        c_s_surf_p_nondim = pybamm.surf(c_s_p_nondim)
-        j0_p = param.p.prim.j0_dimensional(c_e_p, c_s_surf_p_dim, T)
-        eta_p = phi_s_p - phi_e_p - param.p.prim.U_dimensional(c_s_surf_p_nondim, T)
+        c_s_surf_p = pybamm.surf(c_s_p)
+        j0_p = param.p.prim.j0_dimensional(c_e_p, c_s_surf_p, T)
+        eta_p = (
+            phi_s_p
+            - phi_e_p
+            - param.p.prim.U_dimensional(c_s_surf_p / param.p.prim.c_max, T)
+        )
         Feta_RT_p = param.F * eta_p / (param.R * T)
         j_s = pybamm.PrimaryBroadcast(0, "separator")
         j_p = 2 * j0_p * pybamm.sinh(param.p.prim.ne / 2 * Feta_RT_p)
@@ -184,7 +182,7 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
         # The div and grad operators will be converted to the appropriate matrix
         # multiplication at the discretisation stage
         N_s_n = -param.n.prim.D_dimensional(c_s_n, T) * pybamm.grad(c_s_n)
-        N_s_p = -param.p.prim.D_dimensional(c_s_p_dim, T) * pybamm.grad(c_s_p) * c_scale
+        N_s_p = -param.p.prim.D_dimensional(c_s_p, T) * pybamm.grad(c_s_p)
         self.rhs[c_s_n] = -pybamm.div(N_s_n)
         self.rhs[c_s_p] = -pybamm.div(N_s_p)
         # Boundary conditions must be provided for equations with spatial derivatives
@@ -198,7 +196,7 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
         self.boundary_conditions[c_s_p] = {
             "left": (pybamm.Scalar(0), "Neumann"),
             "right": (
-                -j_p / (param.F * param.p.prim.D_dimensional(c_s_surf_p_dim, T)),
+                -j_p / (param.F * param.p.prim.D_dimensional(c_s_surf_p, T)),
                 "Neumann",
             ),
         }
@@ -266,8 +264,8 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
             "Negative particle surface concentration [mol.m-3]": c_s_surf_n,
             "Negative particle surface concentration": c_s_surf_n / param.n.prim.c_max,
             "Electrolyte concentration [mol.m-3]": c_e,
-            "Positive particle surface concentration [mol.m-3]": c_s_surf_p_dim,
-            "Positive particle surface concentration": c_s_surf_p_nondim,
+            "Positive particle surface concentration [mol.m-3]": c_s_surf_p,
+            "Positive particle surface concentration": c_s_surf_p / param.p.prim.c_max,
             "Current [A]": I,
             "Negative electrode potential [V]": phi_s_n,
             "Electrolyte potential [V]": phi_e,

pybamm/parameters/parameter_values.py

@@ -745,7 +745,7 @@ def _process_symbol(self, symbol):
         # Variables: update scale
         elif isinstance(symbol, pybamm.Variable):
             new_symbol = symbol.create_copy()
-            new_symbol.scale = self.process_symbol(symbol.scale)
+            new_symbol._scale = self.process_symbol(symbol.scale)
             return new_symbol
 
         else:

pybamm/solvers/algebraic_solver.py

@@ -208,7 +208,7 @@ def jac_norm(y):
                     )
                     integration_time += timer.time()
 
-                if sol.success:# and np.all(abs(sol.fun) < self.tol):
+                if sol.success and np.all(abs(sol.fun) < self.tol):
                     # update initial guess for the next iteration
                     y0_alg = sol.x
                     # update solution array

tests/unit/test_expression_tree/test_concatenations.py

@@ -97,6 +97,23 @@ def test_concatenation_auxiliary_domains(self):
         ):
             pybamm.concatenation(a, b, c)
 
+    def test_concatenations_scale(self):
+        a = pybamm.Symbol("a", domain="test a")
+        b = pybamm.Symbol("b", domain="test b")
+
+        conc = pybamm.concatenation(a, b)
+        self.assertEqual(conc.scale, 1)
+
+        a._scale = 2
+        with self.assertRaisesRegex(
+            ValueError, "Cannot concatenate symbols with different scales"
+        ):
+            pybamm.concatenation(a, b)
+
+        b._scale = 2
+        conc = pybamm.concatenation(a, b)
+        self.assertEqual(conc.scale, 2)
+
     def test_concatenation_simplify(self):
         # Primary broadcast
         var = pybamm.Variable("var", "current collector")

tests/unit/test_expression_tree/test_symbol.py

@@ -111,6 +111,22 @@ def test_symbol_auxiliary_domains(self):
         with self.assertRaisesRegex(NotImplementedError, "auxiliary_domains"):
             a.auxiliary_domains
 
+    def test_symbol_scale(self):
+        a = pybamm.Symbol("a")
+        self.assertEqual(a.scale, 1)
+
+        a._scale = 2
+        self.assertEqual(a.scale, 2)
+
+        self.assertEqual((a + 2).scale, 2)
+
+        b = pybamm.Symbol("b")
+        b._scale = 3
+        with self.assertRaisesRegex(
+            ValueError, "Cannot scale children with different scales"
+        ):
+            a + b
+
     def test_symbol_methods(self):
         a = pybamm.Symbol("a")
         b = pybamm.Symbol("b")