-
-
Notifications
You must be signed in to change notification settings - Fork 570
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
Make basic models compatible with experiments #3995
Changes from 13 commits
c387732
6f93f00
6a3d93c
5978ec1
1b814b6
64ade63
c6ad4c1
3faa2a8
a3c5e5d
75f2a6d
0aeec78
44c51d3
c438575
20dc591
df61b36
a14400c
f214f3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,8 +7,9 @@ | |
class BaseModel(pybamm.BaseSubModel): | ||
"""Model to represent the behaviour of the external circuit.""" | ||
|
||
def __init__(self, param, options): | ||
def __init__(self, param, options, add_discharge_capacity=True): | ||
super().__init__(param, options=options) | ||
self.add_discharge_capacity = add_discharge_capacity | ||
|
||
def get_fundamental_variables(self): | ||
Q_Ah = pybamm.Variable("Discharge capacity [A.h]") | ||
|
@@ -41,23 +42,25 @@ def get_fundamental_variables(self): | |
return variables | ||
|
||
def set_initial_conditions(self, variables): | ||
Q_Ah = variables["Discharge capacity [A.h]"] | ||
Qt_Ah = variables["Throughput capacity [A.h]"] | ||
self.initial_conditions[Q_Ah] = pybamm.Scalar(0) | ||
self.initial_conditions[Qt_Ah] = pybamm.Scalar(0) | ||
if self.add_discharge_capacity: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We add these by default when using this as a submodel, but pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about just having a separate submodel for discharge capacity and related variables? |
||
Q_Ah = variables["Discharge capacity [A.h]"] | ||
Qt_Ah = variables["Throughput capacity [A.h]"] | ||
self.initial_conditions[Q_Ah] = pybamm.Scalar(0) | ||
self.initial_conditions[Qt_Ah] = pybamm.Scalar(0) | ||
if self.options["calculate discharge energy"] == "true": | ||
Q_Wh = variables["Discharge energy [W.h]"] | ||
Qt_Wh = variables["Throughput energy [W.h]"] | ||
self.initial_conditions[Q_Wh] = pybamm.Scalar(0) | ||
self.initial_conditions[Qt_Wh] = pybamm.Scalar(0) | ||
|
||
def set_rhs(self, variables): | ||
# ODEs for discharge capacity and throughput capacity | ||
Q_Ah = variables["Discharge capacity [A.h]"] | ||
Qt_Ah = variables["Throughput capacity [A.h]"] | ||
I = variables["Current [A]"] | ||
self.rhs[Q_Ah] = I / 3600 # Returns to zero after a complete cycle | ||
self.rhs[Qt_Ah] = abs(I) / 3600 # Increases with each cycle | ||
if self.add_discharge_capacity: | ||
# ODEs for discharge capacity and throughput capacity | ||
Q_Ah = variables["Discharge capacity [A.h]"] | ||
Qt_Ah = variables["Throughput capacity [A.h]"] | ||
I = variables["Current [A]"] | ||
self.rhs[Q_Ah] = I / 3600 # Returns to zero after a complete cycle | ||
self.rhs[Qt_Ah] = abs(I) / 3600 # Increases with each cycle | ||
if self.options["calculate discharge energy"] == "true": | ||
Q_Wh = variables["Discharge energy [W.h]"] | ||
Qt_Wh = variables["Throughput energy [W.h]"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't do this if we don't allow calculating summary variables on the fly. I'm not sure how useful setting a voltage cut-off at the experiment level in this way is anyway? Surely we would set cut-offs at the step level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was added in response to this request #1826. I completely forgot we had this feature but it's quite useful for GITT! The lines commented here are just for logging, the line that does the check is in the
Simulation
class, we should keep it but evaluate the voltage inside theSimulation
class instead of using the summary variable. It would only get called ifvoltage_stop is not None
, so no performance hit in general.