Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some standard battery models from papers #1356

Closed
valentinsulzer opened this issue Feb 3, 2021 · 24 comments
Closed

Add some standard battery models from papers #1356

valentinsulzer opened this issue Feb 3, 2021 · 24 comments
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours

Comments

@valentinsulzer
Copy link
Member

We are now at the stage where it might be useful to add some standard "full" battery models from the literature with exactly the degradation models (model options) and parameter set used in that model.
Something like

class OKane2020(DFN):
   def __init__(self):
        options = {"plating": "reversible"}
        super().__init__(self, options=options, name="O'Kane 2020 model")
  
   @property
   def default_parameter_values(self):
       return pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Chen2020_plating)

Then the model can be used as pybamm.lithium_ion.OKane2020().

To start with:

  • Yang 2017
  • O'Kane 2020
  • Ai 2020
@valentinsulzer valentinsulzer added the difficulty: easy A good issue for someone new. Can be done in a few hours label Feb 3, 2021
@valentinsulzer
Copy link
Member Author

Doing this for the @weilongai and @DrSOKane papers should be fairly easy as a first issue. Can you comment here exactly which options and parameters are needed for this?
For the Yang2017 paper, it is slightly more involved as it needs a new parameter set.

@weilongai
Copy link
Contributor

Hi @tinosulzer , this sounds to be a good idea, and I would suggest the following class. Are you going to make the DFN by default?

class Ai2020(DFN):
   def __init__(self):
        options = {   "particle": "Fickian diffusion",   "particle cracking": "both" }
        super().__init__(self, options=options, name="Ai 2020 model")

   @property
   def default_parameter_values(self):
       return pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Ai2020)

@valentinsulzer
Copy link
Member Author

Thanks

Are you going to make the DFN by default

I guess whichever model the paper uses as the underlying model

@DrSOKane
Copy link
Contributor

DrSOKane commented Feb 3, 2021

I actually used the Ecker et al. parameter set in my 2020 paper, not Chen et al. I used Chen et al. in the example because I was unable to implement the model in my paper, for two reasons:

  1. The nonlinear diffusion function currently in PyBaMM for Ecker et al. is an approximation used by Ivan Korotkin to fit discharge curves at various C-rates, and is not the real diffusivity data. PyBaMM did not converge when an analytic fit to the real diffusivity data was used. (Even COMSOL struggled!)
  2. The charging protocol in my 2020 paper cuts off at 80% SoC, determined by Coulomb counting. I don't believe this has yet been implemented in the Experiment class.

Implementing full nonlinear diffusion in PyBaMM is #1 on my list of long-term goals but I urgently need to get another paper out before I can work on this.

@valentinsulzer
Copy link
Member Author

I urgently need to get another paper out before I can work on this.

don't we all 😅

@brosaplanella
Copy link
Member

I am working on implementing the nonlinear diffusion coefficients for the LG M50, if that helps :)

@KennethNwanoro
Copy link
Contributor

Hi everyone,

I am Kenneth Nwanoro. I joined the Lancaster energy storage group couple of months ago.
Recently, I have started using Pybamm for battery modelling. I just wanted to say that I will implement the Yang 2017 model. This may take a little time as I will need to find all the parameters values used in the paper (not all the parameters values were given in the paper). Many thanks.

@valentinsulzer
Copy link
Member Author

Thanks @KennethNwanoro . When you have the Yang2017 model ready could you add it please (with the wrong parameters for now), to avoid duplication of work as I am aware of some other projects that will also want to use that model. I believe the following options give you the model or very close:

{"sei": "ec reaction", "sei film resistance": "distributed", "sei porosity change": "true", "lithium plating": "irreversible"}

We might just need to add an option for porosity change due to plating

@KennethNwanoro
Copy link
Contributor

Thanks @tinosulzer. I have already started creating the Yang2017 model as you can see below. I will now include the "sei film resistance" option as suggested.

class Yang2017(DFN):
def init(self):
options = options = {"sei": "ec reaction limited", "sei porosity change": "true", "lithium plating": "irreversible"}
super().init(self, options=options, name="Yang 2017")

@Property
def default_parameter_values(self):
return pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Yang2017)

@valentinsulzer
Copy link
Member Author

thanks, would be awesome if you could also help us double check the equations are indeed the same

btw, you can write code in github by enclosing it in a block with ```python3 and ```

@KennethNwanoro
Copy link
Contributor

Thanks @tinosulzer. I don't know if this is the right place to post this. I have compared the degradation (SEI and irreversible Li plating) model equations from Yang2017 and the irreversible Li-plating model currently in PyBaMM. There are some slight differences:

  1. The irreversible lithium-plating model currently in PyBaMM uses reaction rate constant parameter and is also dependent on electrolyte concentration, while Yang2017 is based on a constant current exchange density. This will likely give different trends in results if the current
    Li-plating model is used for the Yang 2017 because of variation in electrolyte concentration. Because of this, a suitable
    reversible Li-plating model for Yang 2017 is likely to be added. Or is there a way to change this to constant exchange current density parameter without having to create another irreversible lithium plating model class or modifying the
    present irreversible Li plating model class to include the option of constant current density for the cathodic Tafel equation used for the irreversible Li plating? If not, I will have to add the constant exchange current density option for the irreversible lithium plating.

  2. The SEI reactions (currently in PyBaMM and Yang2017) are based on the EC reaction limited and look similar. I have tried recasting the EC reaction limited SEI model in PyBaMM to dimensional form and it looks similar that in Yang2017. However, is it possible to also post or give reference to the dimensional form of the EC limited SEI reaction model currently in PyBaMM for proper comparison?.

  3. In the Yang2017, the total film thickness is required in updating the electrolyte porosity. This includes film thickness growth due to irreversible lithium plating as well as SEI reactions. In the porosity class, currently only the SEI reaction contributes to the total film thickness and porosity change. This can be found for example in the "full_reaction_driven_porosity" class.

Many thanks.

@valentinsulzer
Copy link
Member Author

valentinsulzer commented Feb 5, 2021

Hi @KennethNwanoro ,

  1. For the "main" reaction, exchange-current density is given as a FunctionParameter (which usually has the standard square root dependencies). For consistency, I suggest we do the same with the exchange current densities for forward and backwards plating reactions
  2. The EC reaction model in pybamm is based on Yang 2017 so I do expect them to be similar. Generally, we should put together a better reference for the SEI models, see Literature references for SEI models #1260
  3. Yes, the "full_reaction_driven_porosity" class should be updated to also simulate change in porosity due to lithium plating. For example, we could add a "lithium plating porosity change" option that is like "sei porosity change", and then have options to add terms to deps_dt depending on which options are set

@KennethNwanoro
Copy link
Contributor

Hi@tinosulzer,
I see, using FunctionParameter with conditional statements could be used for the plating exchange current densities.

What is your suggestion regarding the electrolyte concentration term c_e_n in the present Li-plating in PyBaMM which is not required in Yang2017 model:

            -(1 / C_plating)
            * c_e_n
            * (pybamm.exp(-0.5 * (phi_s_n - phi_e_n + phi_ref + eta_sei)))
        )```

Do you think using  ```FunctionParameter``` for the electrolyte concentration term here will be a good idea so that conditional statement can be used to keep the value to unity if Yang2017 instance is true?

@valentinsulzer
Copy link
Member Author

Yes, the c_e term should go into the exchange-current density function.
You actually don't need a conditional statement. See the following example for what happens where a "function parameter" is given a scalar as an input

import pybamm

c = pybamm.Variable("c")
f = pybamm.FunctionParameter("f", {"c": c})

# Processing with a scalar behaves like a normal parameter
param1 = pybamm.ParameterValues({"f": 2})
param1.process_symbol(f) # returns a Scalar with value 2

# Processing with a function behaves like a function
param2 = pybamm.ParameterValues({"f": lambda x: x ** 2})
param2.process_symbol(f) # returns c ** 2

@KennethNwanoro
Copy link
Contributor

KennethNwanoro commented Feb 10, 2021

Hi @tinosulzer,

Could you please check this function out and see if it is what you have in mind. The function will return both exchange current density scale and c_e_n values suitable for both Yang2017 (with constant lithium plating exchange current density that is independent of concentrations) and concentration dependent exchange current density (e.g. OKane2020).

    def C_plating(self, c_e_n):
       """ Exchnage current density  scale and 
       # also returns correct electrolyte concentration value for Lithium plating model type"""
       inputs = {"electrolyte concentration":c_e_n, }
       
       fun_Li_pating= pybamm.FunctionParameter("electrolyte concentration", inputs)
       
       if self.options["lithium plating exchange current density value"] != "constant":
           c_plating = self.j_scale_n / self.j0_plating_dimensional
           plating_c_e_n_param = pybamm.ParameterValues({"electrolyte concentration": lambda x: x})
           plating_c_e_n = plating_c_e_n_param.process_symbol(fun_Li_pating) # returns true value of c_e_n
           
       else:
           c_plating = (self.j_scale_n / self.m_plating_dimensional) * pybamm.exp(
               -self.F * self.U_n_ref) / (2 * self.R * self.T_ref)
           plating_c_e_n_param = pybamm.ParameterValues({"electrolyte concentration": 1})
           plating_c_e_n = plating_c_e_n_param.process_symbol(fun_Li_pating) # returns electrolyte concentration a Scalar with value 1
         
       return (c_plating , plating_c_e_n)
         
       return (c_plating , plating_c_e_n)

@valentinsulzer
Copy link
Member Author

Kind of but not quite - just need to put the code in different places. I'll open a branch with the main changes required and you can look at the diff on github

@KennethNwanoro
Copy link
Contributor

Great, please let me know when this is done.
Many thanks.

@valentinsulzer
Copy link
Member Author

Here you go: https://github.com/pybamm-team/PyBaMM/compare/yang-model

Now for the Yang model you just need to update the plating_exchange_current_density and stripping_exchange_current_density functions, i.e. replace with a constant

@KennethNwanoro
Copy link
Contributor

Great, got that now.

@KennethNwanoro
Copy link
Contributor

I have couple of questions regarding the porosity change submodel:

  1. The expression for the porosity-time derivative includes this parameter beta_surf_n which is described as an electrolyte property (please see lithium_ion_parameters.py file). It is given a value of 0. My question is, why include this term in the porosity-time derivative expression when it is actually 0 or is it updated elsewhere? This question is important because, for Lithium plating induced porosity change, I would like to know if the term beta_surf_n is required?

  2. The full reaction driven porosity class needs to be updated to account for lithium plating induced porosity change and also coupled SEI and lithium plating induced porosity change options. I preferred doing slight modifications in the current full reaction driven porosity class to include the two additional options which are

    1. SEI induced porosity change is false while it is true for lithium plating porosity change (i.e., user is only interested in lithium plating)
    2. When both SEI and lithium plating porosity change are required e.g. Yang2017.
      The code below (also currently available in my own branch) is the modification of the get_coupled_variables function:
def get_coupled_variables(self, variables):
      
        j_n = variables["Negative electrode interfacial current density"]
        j_p = variables["Positive electrode interfacial current density"]
     
        
        if self.options["sei porosity change"] == "true" and self.options["lithium plating porosity change"] == "false":
            
             j_sei_n = variables["Negative electrode sei interfacial current density"]
             beta_sei_n = self.param.beta_sei_n
             deps_n_dt = -self.param.beta_surf_n * j_n + beta_sei_n * j_sei_n
             
        elif self.options["lithium plating porosity change"] == "true" and self.options["sei porosity change"] == "false": 
            
            j_plating = variables[
            "Negative electrode lithium plating interfacial current density"
        ]
            beta_plating = self.param.beta_plating 
            
            deps_n_dt = -self.param.beta_surf_n * j_n + beta_plating * j_plating
            
        elif self.options["lithium plating porosity change"] == "true" and self.options["sei porosity change"] == "true":
           
            j_plating = variables[
            "Negative electrode lithium plating interfacial current density"
        ]
            
            j_sei_n = variables["Negative electrode sei interfacial current density"]
            
            beta_plating = self.param.beta_plating 
            beta_sei_n = self.param.beta_sei_n
            
            deps_n_dt = -self.param.beta_surf_n * j_n + beta_sei_n * j_sei_n + beta_plating * j_plating
        
        
        deps_s_dt = pybamm.FullBroadcast(
            0, "separator", auxiliary_domains={"secondary": "current collector"}
        )

Or would you rather prefer separate class for each of the two additional options listed above?
For example, full_lithium_plating_driven_porosity_change and full_lithium_plating_with_SEI_driven_porosity_change .
If this is the case, then I will have to created another two additional classes similar to leading_reaction_driven_porosity to account for the two additional options for the leading order models.
I will now wait until I hear your preference.

@valentinsulzer
Copy link
Member Author

why include this term in the porosity-time derivative expression when it is actually 0

The beta_surf_n parameter accounts for changes in porosity due to the "main" reaction. This is zero for lithium-ion, but we have it there so that we can reuse the same submodel for lead-acid (and any other chemistry where there are porosity changes due to the main reaction).

I preferred doing slight modifications in the current full reaction driven porosity class to include the two additional options

Yes, I also prefer this way to adding separate classes. You can make the code a little bit cleaner as follows

def get_coupled_variables(self, variables):
      
        j_n = variables["Negative electrode interfacial current density"]
        j_p = variables["Positive electrode interfacial current density"]
       deps_n_dt = -self.param.beta_surf_n * j_n
        
        if self.options["sei porosity change"] == "true":
            
             j_sei_n = variables["Negative electrode sei interfacial current density"]
             beta_sei_n = self.param.beta_sei_n
             deps_n_dt += beta_sei_n * j_sei_n
             
        elif self.options["lithium plating porosity change"] == "true": 
            
            j_plating = variables[
            "Negative electrode lithium plating interfacial current density"
        ]
            beta_plating = self.param.beta_plating 
            
            deps_n_dt += beta_plating * j_plating
        
        deps_s_dt = pybamm.FullBroadcast(
            0, "separator", auxiliary_domains={"secondary": "current collector"}
        )
        deps_p_dt = -self.param.beta_surf_p * j_p

@KennethNwanoro
Copy link
Contributor

The Yang2017 model is ready. The initial test results looks reasonable. I was wondering if there is any kind of standard benchmark tests for the SEI and lithium plating models for comparison before creating pull request?

@valentinsulzer
Copy link
Member Author

We don't have benchmark tests, only the StandardOutputTests, feel free to open a PR and we'll let you know if any tests are missing

@valentinsulzer
Copy link
Member Author

Closing as too broad an issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours
Projects
None yet
Development

No branches or pull requests

5 participants