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

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

Open
KimKitsurag1 opened this issue Oct 16, 2023 · 2 comments
Open

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

KimKitsurag1 opened this issue Oct 16, 2023 · 2 comments
Labels
bug Something isn't working priority: high To be resolved as soon as possible

Comments

@KimKitsurag1
Copy link

KimKitsurag1 commented Oct 16, 2023

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

@KimKitsurag1 KimKitsurag1 added the bug Something isn't working label Oct 16, 2023
@brosaplanella brosaplanella changed the title [Bug]: [Bug]: Thevenin model fails to find "SoC" variable Nov 7, 2023
@brosaplanella brosaplanella added the priority: high To be resolved as soon as possible label Nov 7, 2023
@brosaplanella
Copy link
Member

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
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: high To be resolved as soon as possible
Projects
None yet
Development

No branches or pull requests

3 participants