pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
1.04k stars 520 forks source link

[Bug]: Thevenin model fails to find "SoC" variable #3451

Open KimKitsurag1 opened 10 months ago

KimKitsurag1 commented 10 months ago

PyBaMM Version

23.5

Python Version

3.11.5150

Describe the bug

Got an issue trying to run a solution for following model, simulation, and parameters.

options = {'number of rc elements':1, "calculate discharge energy":'true', 'operating mode':'current'}

model = pybamm.equivalent_circuit.Thevenin(name = 'LFP-model', options = options)
R_p = (4.29841228e-02)/30
C_p = 5.39174860e+01 / R_p
params = {'R1 [Ohm]' : R_p ,
         'R0 [Ohm]' : 0.0003,
         'Cell capacity [A.h]' : 231.0,
         'Lower voltage cut-off [V]' : 2.5,
         'Upper voltage cut-off [V]' : 3.35,
         'Initial SoC' : 1.0,
         'Current function [A]' : 30.0,
         'C1 [F]' : C_p, 
         'Open-circuit voltage [V]' : 3.3,
         'Entropic change [V/K]' : 0,
         'Cell-jig heat transfer coefficient [W/K]' : 0,
         'Cell thermal mass [J/K]' : 4.1*1050,
         'Jig-air heat transfer coefficient [W/K]' : 5,
         'Ambient temperature [K]' : 298,
         "Jig thermal mass [J/K]": 500,
         "Element-1 initial overpotential [V]": 0,
         'Initial temperature [K]' : 298}
params = pybamm.ParameterValues(params)
sim = pybamm.Simulation(model, parameter_values = params)
sim.solve([0, 3600])

Then got:

ModelError: 
                    No key set for variable 'SoC'. Make sure it is included in either
                    model.rhs or model.algebraic in an unmodified form
                    (e.g. not Broadcasted)

But SoC was presented in model.rhs as Variable(-0x22f01313d791d1c7, SoC, children=[], domains={}): Multiplication(0x511128df7b7d7ad5, *, children=['0.0002777777777777778', '-Current function [A] / Cell capacity [A.h]'], domains={}) What am I doing wrong?

Steps to Reproduce

  1. initialise Thevenin model
  2. initialise simulation with suitable params
  3. run a solution

Relevant log output

No response

brosaplanella commented 9 months ago

That's very strange. The model runs just fine with the default parameters but not with the updated ones. We will look into it.

pipliggins commented 7 months ago

I've been having a look into this issue. I can recreate the error with the most recent PyBaMM release, and narrowed down the cause to the updated C1, R0, R1 and Open-circuit voltage parameters - comment out any one of these in the updated parameters and the example runs fine (actually a different error pertaining to the maximum voltage crops up, but increasing that value solves the issue). If you only update one of the problematic parameters and leave the rest as defaults, the simulation still runs without error.

Having looked at the model.rhs at the point of discretisation, I think the issue is that in the default parameters, all 4 are functions containing the SoC variable, which is created in the get_fundamental_variables method of the OCVElement class. When all 4 parameters are overwritten as in the above example, the SoC variable no longer appears in the rhs equation values, even though it's present in the model.rhs keys prior to solving. SoC is then removed from the rhs entirely as an independent variable by remove_independent_variables_from_rhs(). The final error comes because the default model events contain the min/maximum SoC which depend on the SoC variable. As the variable has been removed from the model RHS, the error is raised.

In the case where only one parameter is updated, the SoC variable is still present as part of the other three from initialisation.

@brosaplanella I'm not a battery modeller, so I'm not entirely sure which part is the bug here; is the model valid without an SoC variable? From having a brief look at the Thevenin reference paper it seems like OCV, and possibly the other parameters, should always depend on SoC? In which case the way the parameters are updated should be edited. Or is the model valid without an SoC variable, and the model events need to update to remove those that depend on missing variables?