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

#3611 use actual cell volume for average total heating #3707

Merged
merged 5 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# [Unreleased](https://github.com/pybamm-team/PyBaMM/)

## Bug fixes

- Fixed a bug where the lumped thermal model conflates cell volume with electrode volume ([#3707](https://github.com/pybamm-team/PyBaMM/pull/3707))

## Breaking changes
- The parameters `GeometricParameters.A_cooling` and `GeometricParameters.V_cell` are now automatically computed from the electrode heights, widths and thicknesses if the "cell geometry" option is "pouch" and from the parameters "Cell cooling surface area [m2]" and "Cell volume [m3]", respectively, otherwise. When using the lumped thermal model we recommend using the "arbitrary" cell geometry and specifying the parameters "Cell cooling surface area [m2]", "Cell volume [m3]" and "Total heat transfer coefficient [W.m-2.K-1]" directly. ([#3707](https://github.com/pybamm-team/PyBaMM/pull/3707))
# [v24.1rc0](https://github.com/pybamm-team/PyBaMM/tree/v24.1rc0) - 2024-01-31

## Features
Expand Down
19 changes: 11 additions & 8 deletions docs/source/examples/notebooks/models/jelly-roll-model.ipynb

Large diffs are not rendered by default.

31 changes: 10 additions & 21 deletions docs/source/examples/notebooks/models/pouch-cell-model.ipynb

Large diffs are not rendered by default.

950 changes: 492 additions & 458 deletions docs/source/examples/notebooks/models/thermal-models.ipynb

Large diffs are not rendered by default.

34 changes: 27 additions & 7 deletions examples/scripts/thermal_lithium_ion.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@
pybamm.set_logging_level("INFO")

# load models
# for the full model we use the "x-full" thermal submodel, which means that we solve
# the thermal model in the x-direction for a single-layer pouch cell
# for the lumped model we use the "arbitrary" cell geometry, which means that we can
# specify the surface area for cooling and total heat transfer coefficient
full_thermal_model = pybamm.lithium_ion.SPMe(
{"thermal": "x-full"}, name="full thermal model"
)
lumped_thermal_model = pybamm.lithium_ion.SPMe(
{"thermal": "lumped"}, name="lumped thermal model"
{"cell geometry": "arbitrary", "thermal": "lumped"}, name="lumped thermal model"
)
models = [full_thermal_model, lumped_thermal_model]

Expand All @@ -31,27 +35,43 @@
}
)
# for the lumped model we set the "Total heat transfer coefficient [W.m-2.K-1]"
# parameter as well as the "Cell cooling surface area [m2]" parameter. Since the "full"
# model only accounts for cooling from the large surfaces of the pouch, we set the
# "Surface area for cooling" parameter to the area of the large surfaces of the pouch,
# and the total heat transfer coefficient to 5 W.m-2.K-1
# parameter as well as the "Cell cooling surface area [m2]" and "Cell volume [m3]
# parameters. Since the "full" model only accounts for cooling from the large surfaces
# of the pouch, we set the "Surface area for cooling [m2]" parameter to the area of the
# large surfaces of the pouch, and the total heat transfer coefficient to 5 W.m-2.K-1
A = parameter_values["Electrode width [m]"] * parameter_values["Electrode height [m]"]
contributing_layers = [
"Negative current collector",
"Negative electrode",
"Separator",
"Positive electrode",
"Positive current collector",
]
total_thickness = sum(
[parameter_values[layer + " thickness [m]"] for layer in contributing_layers]
)
electrode_volume = (
total_thickness
* parameter_values["Electrode height [m]"]
* parameter_values["Electrode width [m]"]
)
lumped_params = parameter_values.copy()
lumped_params.update(
{
"Total heat transfer coefficient [W.m-2.K-1]": 5,
"Cell cooling surface area [m2]": 2 * A,
"Cell volume [m3]": electrode_volume,
}
)
params = [full_params, lumped_params]

# loop over the models and solve
params = [full_params, lumped_params]
sols = []
for model, param in zip(models, params):
sim = pybamm.Simulation(model, parameter_values=param)
sim.solve([0, 3600])
sols.append(sim.solution)


# plot
output_variables = [
"Voltage [V]",
Expand Down
19 changes: 0 additions & 19 deletions pybamm/models/full_battery_models/base_battery_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import pybamm
from functools import cached_property
import warnings

from pybamm.expression_tree.operations.serialise import Serialise

Expand Down Expand Up @@ -683,24 +682,6 @@ def __init__(self, extra_options):
f"Possible values are {self.possible_options[option]}"
)

# Issue a warning to let users know that the 'lumped' thermal option (or
# equivalently 'x-lumped' with 0D current collectors) now uses the total heat
# transfer coefficient, surface area for cooling, and cell volume parameters,
# regardless of the 'cell geometry option' chosen.
thermal_option = options["thermal"]
dimensionality_option = options["dimensionality"]
if thermal_option == "lumped" or (
thermal_option == "x-lumped" and dimensionality_option == 0
):
message = (
f"The '{thermal_option}' thermal option with "
f"'dimensionality' {dimensionality_option} now uses the parameters "
"'Cell cooling surface area [m2]', 'Cell volume [m3]' and "
"'Total heat transfer coefficient [W.m-2.K-1]' to compute the cell "
"cooling term, regardless of the value of the the 'cell geometry' "
"option. Please update your parameters accordingly."
)
warnings.warn(message, pybamm.OptionWarning, stacklevel=2)
super().__init__(options.items())

@property
Expand Down
36 changes: 31 additions & 5 deletions pybamm/models/submodels/thermal/base_thermal.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,32 +179,58 @@ def _get_standard_coupled_variables(self, variables):
Q = Q_ohm + Q_rxn + Q_rev

# Compute the X-average over the entire cell, including current collectors
# Note: this can still be a function of y and z for higher-dimensional pouch
# cell models
Q_ohm_av = self._x_average(Q_ohm, Q_ohm_s_cn, Q_ohm_s_cp)
Q_rxn_av = self._x_average(Q_rxn, 0, 0)
Q_rev_av = self._x_average(Q_rev, 0, 0)
Q_av = self._x_average(Q, Q_ohm_s_cn, Q_ohm_s_cp)

# Compute volume-averaged heat source terms
Q_ohm_vol_av = self._yz_average(Q_ohm_av)
Q_rxn_vol_av = self._yz_average(Q_rxn_av)
Q_rev_vol_av = self._yz_average(Q_rev_av)
Q_vol_av = self._yz_average(Q_av)
# Compute the integrated heat source per unit simulated electrode-pair area
# in W.m-2
# Note: this can still be a function of y and z for higher-dimensional pouch
# cell models
Q_ohm_Wm2 = Q_ohm_av * param.L
Q_rxn_Wm2 = Q_rxn_av * param.L
Q_rev_Wm2 = Q_rev_av * param.L
Q_Wm2 = Q_av * param.L
# Now average over the electrode height and width
Q_ohm_Wm2_av = self._yz_average(Q_ohm_Wm2)
Q_rxn_Wm2_av = self._yz_average(Q_rxn_Wm2)
Q_rev_Wm2_av = self._yz_average(Q_rev_Wm2)
Q_Wm2_av = self._yz_average(Q_Wm2)

# Compute volume-averaged heat source terms over the *entire cell volume*, not
# the product of electrode height * electrode width * electrode stack thickness
A = param.L_y * param.L_z # *modelled* electrode area
rtimms marked this conversation as resolved.
Show resolved Hide resolved
V = param.V_cell # *actual* cell volume
Q_ohm_vol_av = Q_ohm_Wm2_av * A / V
Q_rxn_vol_av = Q_rxn_Wm2_av * A / V
Q_rev_vol_av = Q_rev_Wm2_av * A / V
Q_vol_av = Q_Wm2_av * A / V

variables.update(
{
"Ohmic heating [W.m-3]": Q_ohm,
"X-averaged Ohmic heating [W.m-3]": Q_ohm_av,
"Volume-averaged Ohmic heating [W.m-3]": Q_ohm_vol_av,
"Integrated Ohmic heating per unit electrode-pair area "
"[W.m-2]": Q_ohm_Wm2,
"Irreversible electrochemical heating [W.m-3]": Q_rxn,
"X-averaged irreversible electrochemical heating [W.m-3]": Q_rxn_av,
"Volume-averaged irreversible electrochemical heating "
+ "[W.m-3]": Q_rxn_vol_av,
"Integrated irreversible electrochemical heating per unit "
+ "electrode-pair area [W.m-2]": Q_rxn_Wm2,
"Reversible heating [W.m-3]": Q_rev,
"X-averaged reversible heating [W.m-3]": Q_rev_av,
"Volume-averaged reversible heating [W.m-3]": Q_rev_vol_av,
"Integrated reversible heating per unit electrode-pair area "
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not totally convinced by the name. I find clearer "Total reversible heating per unit electrode-pair area [W.m-2]", but at the same time I can see how a user might find this more confusing, so happy to leave it like that.

In my mind, "total" already means the integrated value and the "per ... area" and units make it clear enough how it has been integrated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I avoided "total" as we already use that to mean "sum of irreversible and reversible", but happy to take suggestions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest that you just delete the word "Integrated"; I agree with @brosaplanella that "per __" intrinsically makes clear how the quantity is scaled. You have long established the approach of using the same name with different units to account for normalisation, and I think that is also fine.

If you are always using "Total" to mean "sum of irreversible and irreversible" then the name "Total reversible heating" is malformed, though.

In the realm of breaking changes, I would call the [W.m-3] quantities a "volumetric heat source" and the [W] quantities a "heat rate".

Also (pedantically), "ohmic" should be lowercase (c.f. voltaic, faradaic). Not worth changing if breaking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, yes, those “Total…” shouldn’t be there, not sure why I put them there. Probably not worth changing the capitalisation of Ohmic.

To avoid any other breaking changes I’ll keep the same names and infer things from units.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember now, the “Total” came from copy the definition of the existing “Total heating [W]” variable. I’ll make these changes. Otherwise, does this look correct?

"[W.m-2]": Q_rev_Wm2,
"Total heating [W.m-3]": Q,
"X-averaged total heating [W.m-3]": Q_av,
"Volume-averaged total heating [W.m-3]": Q_vol_av,
"Integrated total heating per unit electrode-pair area [W.m-2]": Q_Wm2,
"Negative current collector Ohmic heating [W.m-3]": Q_ohm_s_cn,
"Positive current collector Ohmic heating [W.m-3]": Q_ohm_s_cp,
}
Expand Down
15 changes: 13 additions & 2 deletions pybamm/parameters/geometric_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,19 @@ def _set_parameters(self):
self.r_inner = pybamm.Parameter("Inner cell radius [m]")
self.r_outer = pybamm.Parameter("Outer cell radius [m]")
self.A_cc = self.L_y * self.L_z # Current collector cross sectional area
self.A_cooling = pybamm.Parameter("Cell cooling surface area [m2]")
self.V_cell = pybamm.Parameter("Cell volume [m3]")

# Cell surface area and volume (for thermal models only)
cell_geometry = self.options.get("cell geometry", None)
if cell_geometry == "pouch":
# assuming a single-layer pouch cell for now, see
# https://github.com/pybamm-team/PyBaMM/issues/1777
self.A_cooling = 2 * (
self.L_y * self.L_z + self.L_z * self.L + self.L_y * self.L
)
self.V_cell = self.L_y * self.L_z * self.L
else:
self.A_cooling = pybamm.Parameter("Cell cooling surface area [m2]")
self.V_cell = pybamm.Parameter("Cell volume [m3]")


class DomainGeometricParameters(BaseParameters):
Expand Down