External variables
valentinsulzer opened this issue · 7 comments
I am trying to understand external variables and if we really need them or if the same results can be achieved using the appropriate inputs and models. It would be nice if we could remove them, since they introduce some technical debt into both PyBaMM and liionpack (e.g. currently #196 is failing because of external variables, and in pybamm it's causing issues with dimensional models).
In liionpack I see the main use case is to use a model with options={"thermal": "lumped", "external submodels": ["thermal"]}, then providing "Volume-averaged cell temperature" as an external variable, changing over different steps. This could just as easily be achieved by setting "Ambient temperature [K]" as an input changing over different steps. If the goal is to also track the change in temperature from the cell model, the lumped model (not external) with changing ambient temperature should also achieve this. It's not clear to me why we should update the "volume-averaged cell temperature" instead of the "ambient temperature".
Am I missing something?
Also, it's now possible to provide input parameters that depend on x, which is another use case that previously could only be done with external variables
As discussed with Tom this morning, we're going to go ahead with testing removing external variables. The main concern is that updating temperature with "ambient temperature" is confusing, and instead we should add another submodel for "constant temperature" where "cell temperature" is an extra parameter.
+1 for removing and using inputs
one of the issues before was that the isothermal model didn’t compute heat source terms, so you had to choose a lumped thermal model with an external variable. we since added the option to compute the heat source terms for the isothermal model. adding a constant temperature model sounds good.
Yeah I have no issue with removing it. The 2+1D stuff became more integrated so no longer really needed for that. Tom is the only one that made good use of it so I think he should have the main say.
@TomTranter any progress with this?
None yet, last week got a bit crazy. Will look at it tomorrow
Moved issue to PyBaMM