#2860 fix bpx data interpolant

CHANGELOG.md

@@ -13,6 +13,7 @@
 ## Bug fixes
 
 - Fix `pybamm_install_odes` and update the required SUNDIALS version ([#2958](https://github.com/pybamm-team/PyBaMM/pull/2958))
+- Fixed a bug where all data included in a BPX was incorrectly assumed to be given as a function of time.([#2957](https://github.com/pybamm-team/PyBaMM/pull/2957))
 - Remove brew install for Mac from the recommended developer installation options for SUNDIALS ([#2925](https://github.com/pybamm-team/PyBaMM/pull/2925))
 
 ## Breaking changes

pybamm/parameters/bpx.py

@@ -5,7 +5,6 @@
 import numpy as np
 from pybamm import constants
 from pybamm import exp
-import copy
 
 
 import types
@@ -134,39 +133,73 @@ def _bpx_to_param_dict(bpx: BPX) -> dict:
 
     # TODO: allow setting function parameters in a loop over domains
 
+    # ocp
+
+    # negative electrode (only need to check for data, other cases pass through)
+    U_n = pybamm_dict[negative_electrode.pre_name + "OCP [V]"]
+    if isinstance(U_n, tuple):
+
+        def _negative_electrode_ocp(sto):
+            name, (x, y) = U_n
+            return pybamm.Interpolant(x, y, sto, name=name, interpolator="linear")
+
+        pybamm_dict[negative_electrode.pre_name + "OCP [V]"] = _negative_electrode_ocp
+
+    # positive electrode (only need to check for data, other cases pass through)
+    U_p = pybamm_dict[positive_electrode.pre_name + "OCP [V]"]
+    if isinstance(U_p, tuple):
+
+        def _positive_electrode_ocp(sto):
+            name, (x, y) = U_p
+            return pybamm.Interpolant(x, y, sto, name=name, interpolator="linear")
+
+        pybamm_dict[positive_electrode.pre_name + "OCP [V]"] = _positive_electrode_ocp
+
     # entropic change
 
     # negative electrode
-    U_n = pybamm_dict[
+    dUdT_n = pybamm_dict[
         negative_electrode.pre_name + "entropic change coefficient [V.K-1]"
     ]
-    if callable(U_n):
+    if callable(dUdT_n):
 
         def _negative_electrode_entropic_change(sto, c_s_max):
-            return U_n(sto)
+            return dUdT_n(sto)
+
+    elif isinstance(dUdT_n, tuple):
+
+        def _negative_electrode_entropic_change(sto, c_s_max):
+            name, (x, y) = dUdT_n
+            return pybamm.Interpolant(x, y, sto, name=name, interpolator="linear")
 
     else:
 
         def _negative_electrode_entropic_change(sto, c_s_max):
-            return U_n
+            return dUdT_n
 
     pybamm_dict[
         negative_electrode.pre_name + "OCP entropic change [V.K-1]"
     ] = _negative_electrode_entropic_change
 
     # positive electrode
-    U_p = pybamm_dict[
+    dUdT_p = pybamm_dict[
         positive_electrode.pre_name + "entropic change coefficient [V.K-1]"
     ]
-    if callable(U_p):
+    if callable(dUdT_p):
+
+        def _positive_electrode_entropic_change(sto, c_s_max):
+            return dUdT_p(sto)
+
+    elif isinstance(dUdT_p, tuple):
 
         def _positive_electrode_entropic_change(sto, c_s_max):
-            return U_p(sto)
+            name, (x, y) = dUdT_p
+            return pybamm.Interpolant(x, y, sto, name=name, interpolator="linear")
 
     else:
 
         def _positive_electrode_entropic_change(sto, c_s_max):
-            return U_p
+            return dUdT_p
 
     pybamm_dict[
         positive_electrode.pre_name + "OCP entropic change [V.K-1]"
@@ -252,6 +285,15 @@ def _negative_electrode_diffusivity(sto, T):
             arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
             return arrhenius * D_n_ref(sto)
 
+    elif isinstance(D_n_ref, tuple):
+
+        def _negative_electrode_diffusivity(sto, T):
+            arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
+            name, (x, y) = D_n_ref
+            return arrhenius * pybamm.Interpolant(
+                x, y, sto, name=name, interpolator="linear"
+            )
+
     else:
 
         def _negative_electrode_diffusivity(sto, T):
@@ -274,6 +316,15 @@ def _positive_electrode_diffusivity(sto, T):
             arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
             return arrhenius * D_p_ref(sto)
 
+    elif isinstance(D_p_ref, tuple):
+
+        def _positive_electrode_diffusivity(sto, T):
+            arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
+            name, (x, y) = D_p_ref
+            return arrhenius * pybamm.Interpolant(
+                x, y, sto, name=name, interpolator="linear"
+            )
+
     else:
 
         def _positive_electrode_diffusivity(sto, T):
@@ -296,6 +347,15 @@ def _electrolyte_diffusivity(sto, T):
             arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
             return arrhenius * D_e_ref(sto)
 
+    elif isinstance(D_e_ref, tuple):
+
+        def _electrolyte_diffusivity(sto, T):
+            arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
+            name, (x, y) = D_e_ref
+            return arrhenius * pybamm.Interpolant(
+                x, y, sto, name=name, interpolator="linear"
+            )
+
     else:
 
         def _electrolyte_diffusivity(sto, T):
@@ -310,23 +370,28 @@ def _electrolyte_diffusivity(sto, T):
     E_a = pybamm_dict.get(
         electrolyte.pre_name + "conductivity activation energy [J.mol-1]", 0.0
     )
-    C_ref_value = pybamm_dict[electrolyte.pre_name + "conductivity [S.m-1]"]
+    C_e_ref = pybamm_dict[electrolyte.pre_name + "conductivity [S.m-1]"]
+
+    if callable(C_e_ref):
+
+        def _conductivity(c_e, T):
+            arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
+            return arrhenius * C_e_ref(c_e)
 
-    if callable(C_ref_value):
-        C_ref_fun = copy.copy(C_ref_value)
+    elif isinstance(C_e_ref, tuple):
 
         def _conductivity(c_e, T):
-            C_ref = C_ref_fun(c_e)
             arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
-            return arrhenius * C_ref
+            name, (x, y) = C_e_ref
+            return arrhenius * pybamm.Interpolant(
+                x, y, c_e, name=name, interpolator="linear"
+            )
 
     else:
-        C_ref_number = C_ref_value
 
         def _conductivity(c_e, T):
-            C_ref = C_ref_number
             arrhenius = exp(E_a / constants.R * (1 / T_ref - 1 / T))
-            return arrhenius * C_ref
+            return arrhenius * C_e_ref
 
     pybamm_dict[electrolyte.pre_name + "conductivity [S.m-1]"] = _copy_func(
         _conductivity
@@ -346,12 +411,12 @@ def _bpx_to_domain_param_dict(instance: BPX, pybamm_dict: dict, domain: Domain)
         elif isinstance(value, Function):
             value = value.to_python_function(preamble=preamble)
         elif isinstance(value, InterpolatedTable):
+            # return (name, (x, y)) to match the output of
+            # `pybamm.parameters.process_1D_data` we will create an interpolant on a
+            # case-by-case basis to get the correct argument for each parameter
             x = np.array(value.x)
             y = np.array(value.y)
-            interpolator = "linear"
-            value = pybamm.Interpolant(
-                [x], y, pybamm.t, name=name, interpolator=interpolator
-            )
+            value = (name, (x, y))
 
         pybamm_name = field.field_info.alias
         pybamm_name_lower = pybamm_name[:1].lower() + pybamm_name[1:]

tests/unit/test_parameters/test_bpx.py

@@ -165,8 +165,26 @@ def test_constant_functions(self):
 
     def test_table_data(self):
         bpx_obj = copy.copy(self.base)
+        data = {"x": [0, 1], "y": [0, 1]}
         bpx_obj["Parameterisation"]["Electrolyte"].update(
-            {"Conductivity [S.m-1]": {"x": [800, 1000, 1200], "y": [0.9, 1, 1.1]}}
+            {
+                "Conductivity [S.m-1]": data,
+                "Diffusivity [m2.s-1]": data,
+            }
+        )
+        bpx_obj["Parameterisation"]["Negative electrode"].update(
+            {
+                "Diffusivity [m2.s-1]": data,
+                "OCP [V]": data,
+                "Entropic change coefficient [V.K-1]": data,
+            }
+        )
+        bpx_obj["Parameterisation"]["Positive electrode"].update(
+            {
+                "Diffusivity [m2.s-1]": data,
+                "OCP [V]": data,
+                "Entropic change coefficient [V.K-1]": data,
+            }
         )
 
         filename = "tmp.json"
@@ -179,7 +197,26 @@ def test_table_data(self):
             json.dump(bpx_obj, tmp)
             tmp.flush()
 
-            pybamm.ParameterValues.create_from_bpx(tmp.name)
+            param = pybamm.ParameterValues.create_from_bpx(tmp.name)
+
+            # Check that the electrolyte conductivity is an Interpolant with the
+            # correct child
+            c = pybamm.Variable("c")
+            kappa = param["Electrolyte conductivity [S.m-1]"](c, 298.15)
+            self.assertIsInstance(kappa, pybamm.Interpolant)
+            self.assertEqual(kappa.children[0], c)
+            # Check other parameters give interpolants
+            D = param["Electrolyte diffusivity [m2.s-1]"](c, 298.15)
+            self.assertIsInstance(D, pybamm.Interpolant)
+            for electrode in ["Negative", "Positive"]:
+                D = param[f"{electrode} electrode diffusivity [m2.s-1]"](c, 298.15)
+                self.assertIsInstance(D, pybamm.Interpolant)
+                OCP = param[f"{electrode} electrode OCP [V]"](c)
+                self.assertIsInstance(OCP, pybamm.Interpolant)
+                dUdT = param[f"{electrode} electrode OCP entropic change [V.K-1]"](
+                    c, 10000
+                )
+                self.assertIsInstance(dUdT, pybamm.Interpolant)
 
     def test_bpx_soc_error(self):
         with self.assertRaisesRegex(ValueError, "Target SOC"):