[Bug]: Electrolyte concentration not conserved for non-constant transference number

[DrSOKane]

PyBaMM Version

23.1

Python Version

3.8.10

Describe the bug

The change to full_diffusion.py in PR#2552 fixed lithium conservation for algebraic surface form. However, by moving the migration flux outside the divergence operator, the same fix now violates conservation for the case where transference number is a FunctionParameter, e.g. in the ORegan2022 parameter set.

There must be a general fix that solves this issue for both cases but to solve this issue I would need to learn how the surface form option works and why putting that term inside the divergence operator breaks it.

Steps to Reproduce

Run any model with the ORegan2022 parameter set and you'll find that "Total lithium [mol]" changes over time

Relevant log output

No response

[brosaplanella]

Posting here the images reported by @MarcAntoine88 in #2681

[MarcAntoine88]

Thank you @brosaplanella , the issue is exactly what @DrSOKane explained. What is our action plan for this one ? I'll try to see how to fix it locally

[DrSOKane]

I decided to rewrite the entire set_rhs function, with an additional term to correct for the gradient of the transference number:

def set_rhs(self, variables):

    param = self.param

    eps_c_e = variables["Porosity times concentration"]
    c_e = variables["Electrolyte concentration"]
    T = variables["Cell temperature"]
    t_plus = param.t_plus(c_e, T)
    div_Vbox = variables["Transverse volume-averaged acceleration"]

    N_e_diffusion = variables["Electrolyte diffusion flux"]
    N_e_convection = variables["Electrolyte convection flux"]
    flux_terms = -pybamm.div(N_e_diffusion + N_e_convection) / param.C_e

    sum_s_j = variables["Sum of electrolyte reaction source terms"]
    sum_a_j = variables["Sum of volumetric interfacial current densities"]
    sum_s_j.print_name = "a"
    source_terms = (sum_s_j - t_plus * sum_a_j) / self.param.gamma_e

    # extra term required for when t_plus varies in x
    i_e = variables["Electrolyte current density"]
    migration_term = -pybamm.grad(t_plus) * i_e

    self.rhs = {
        eps_c_e: flux_terms + source_terms + migration_term - c_e * div_Vbox
    }

However, I get the following error:

ShapeError: Cannot find shape (original error: operands could not be broadcast together with shapes (59,1) (61,1) )

One possible fix would be to create a new FunctionParameter for grad(t_plus), which would remove the need to use pybamm.grad. But that would be messy, not to mention extra work for anyone creating parameter sets...

[brosaplanella]

Can't we rewrite the interface conditions in terms of the flux?

That should take care of conservation even with variable t^+, but not sure if the differential form doesn't allow for it.

[valentinsulzer]

@brosaplanella 's suggestion is how it used to be done but it didn't quite conserve mass for some reason that I can't remember. But it was definitely better than what I changed it to so maybe it should just be reverted.

Regarding @DrSOKane 's shape error, it's because it needs boundary conditions for t(c_e). Any object in a grad must have boundary conditions. Off-by-two shape errors are usually due to that (yes, we need a better error message for this case). In this case I think the boundary conditions should be homogeneous Neumann, by the chain rule

[DrSOKane]

Is it possible to apply boundary conditions to a FunctionParameter?

[valentinsulzer]

Yes, for boundary conditions it can be any object, it just needs to be exactly the same as the argument of grad().

[DrSOKane]

Are there any examples of adding boundary conditions to a FunctionParameter? I know how to do it for variables (add a set_boundary_conditions function to the submodel) but not for a parameter.

[ss711]

Hi all I am having the same issue when I add my transference number function. I need to plot the electrolyte concentration gradient over time for a paper revision however, the concentration gradient continues to shift up infinitely due to this issue described above I believe. Do you know of a temporary fix I can use? I am using a half cell DFN model, but when I use the full cell I don't seem to have the same issue which is odd. Many thanks for your help.

[MarcAntoine88]

Hi all, I try to deepen the equations and I have 1 question that come to me:

• Where does the (- t_plus * sum_a_j) come from in the source term ?

[MarcAntoine88]

Hi all, I started to derive the equations of the set_rhs from the mass balance of the anion/cation and can not make it match with the ones in the non dimensionless full diffusion submodel, does anyone see why ? I get (1-tplus) instead of tplus and for me the convection is already taken into account in the div operator. Thanks a lot

[valentinsulzer]

@MarcAntoine88 the PyBaMM implementation right now rhs = div(N_d+c) + (1-t_+) * aj / F - c_e * div_Vbox, not the one you wrote. The problem is that t_+ should be inside the div in the case where t_+ depends on c_e

Should be fixed by #2758

By the way, models are no longer dimensionless, make sure you check out the latest version of develop

[MarcAntoine88]

Thanks for the feedback @tinosulzer, I got confused by sum_s_a_j and sum_a_j, with no side reactions we have sum_s_a_j = sum_a_j and (1-t_+) * aj.

I agree for the t_+

The fact that models are no longer dimensionless is a really good thing :)