[Bug]: create_from_bpx(): effective values of diffusivity activation energies are overwritten by "electrolyte conductivity activation energy"

[ejfdickinson]

PyBaMM Version

23.4

Python Version

3.9.16

Describe the bug

When temperature-dependent functions are created in create_from_bpx(), the apparent activation energy for the following callable parameter definitions in the generated parameter_values instance is replaced by the value of "Electrolyte conductivity activation energy [J.mol-1]".

This appears to be a late-binding bug due to the repeat use of non-local variable E_a in the declaration these functions, which is repeatedly redefined, the electrolyte conductivity activation energy being the last such redefinition.

Affected functions:

• "Negative electrode diffusivity [m2.s-1]"
• "Positive electrode diffusivity [m2.s-1]"
• "Electrolyte diffusivity [m2.s-1]"

Note: identified in 23.4 but no relevant code changes in latest version

Steps to Reproduce

Using nmc_pouch_cell_BPX.json from the BPX example download:

import pybamm
from pybamm import constants, exp, log

# From BPX example download
params = pybamm.ParameterValues.create_from_bpx("nmc_pouch_cell_BPX.json")

Ea_cond_el   = params["Electrolyte conductivity activation energy [J.mol-1]"]
Ea_diff_neg  = params["Negative electrode diffusivity activation energy [J.mol-1]"]

# Evaluate negative electrode diffusivity at Tref
T_ref        = params["Reference temperature [K]"]
Ds_neg_ref   = params["Negative electrode diffusivity [m2.s-1]"](0.5, T_ref)

# Evaluate negative electrode diffusivity at Ttest =/= Tref
T_test       = 290
Ds_neg_test  = params["Negative electrode diffusivity [m2.s-1]"](0.5, T_test)

# Evaluate expected negative electrode diffusivity at Ttest
D_neg_test_expected = Ds_neg_ref * exp(-Ea_diff_neg / constants.R * (1/T_test - 1/T_ref))

# Evaluate apparent activation energy
Ea_diff_neg_apparent = -constants.R / (1/T_test - 1/T_ref) * log(Ds_neg_test/Ds_neg_ref)

print("Negative electrode diffusivity (test temperature, expected):", D_neg_test_expected)
print("Negative electrode diffusivity (test temperature, actual):", Ds_neg_test)

print("Negative electrode diffusivity activation energy (stated):", Ea_diff_neg)
print("Negative electrode diffusivity activation energy (apparent):", Ea_diff_neg_apparent)

print("Electrolyte conductivity activation energy (stated):", Ea_cond_el)

Relevant log output

Negative electrode diffusivity (test temperature, expected): 1.941507320004235e-14
Negative electrode diffusivity (test temperature, actual): 2.2472548468421424e-14
Negative electrode diffusivity activation energy (stated): 30000.0
Negative electrode diffusivity activation energy (apparent): 17099.999999999996
Electrolyte conductivity activation energy (stated): 17100.0

[ejfdickinson]

Simplest fix, if cause is what it looks like - use distinct variable names for all activation energies in bpx.py.

[darryl-ad]

Fixed as a pre-cursor to https://github.com/pybamm-team/PyBaMM/issues/3225

[darryl-ad]

This PR now also fixes https://github.com/pybamm-team/PyBaMM/issues/3225