Skip to content

Added support for time series temperature in an experiment step #4855

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

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open
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 src/pybamm/experiment/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ def __init__(

steps_unprocessed = [cond for cycle in cycles for cond in cycle]

self.temperature_interpolant = None

# Convert strings to pybamm.step.BaseStep objects
# We only do this once per unique step, to avoid unnecessary conversions
# Assign experiment period and temperature if not specified in step
Expand All @@ -75,6 +77,10 @@ def __init__(

self.steps = [processed_steps[repr(step)] for step in steps_unprocessed]
self.steps = self._set_next_start_time(self.steps)
for step in self.steps:
if getattr(step, "has_time_series_temperature", False):
self.temperature_interpolant = step.temperature
break

# Save the processed unique steps and the processed operating conditions
# for every step
Expand Down
43 changes: 35 additions & 8 deletions src/pybamm/experiment/step/base_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
f"Invalid direction: {direction}. Must be one of {potential_directions}"
)
self.input_duration = duration
self.input_duration = duration
self.input_value = value
self.skip_ok = skip_ok
# Check if drive cycle
Expand Down Expand Up @@ -158,7 +157,6 @@
):
direction = self.value_based_charge_or_discharge()
self.direction = direction

self.repr_args, self.hash_args = self.record_tags(
value,
duration,
Expand All @@ -184,8 +182,12 @@
term = _read_termination(term)
self.termination.append(term)

self.temperature = _convert_temperature_to_kelvin(temperature)

self.temperature = _convert_temperature_to_kelvin(
temperature
) # change name of the function
self.has_time_series_temperature = isinstance(
self.temperature, pybamm.Interpolant
)
if tags is None:
tags = []
elif isinstance(tags, str):
Expand Down Expand Up @@ -424,9 +426,13 @@
hash_args += f", termination={termination}"
if period:
repr_args += f", period={period}"
if temperature:
repr_args += f", temperature={temperature}"
hash_args += f", temperature={temperature}"
if temperature is not None:
if isinstance(temperature, np.ndarray):
repr_args += ", temperature=<time-series>"
hash_args += ", temperature=<time-series>"
else:
repr_args += f", temperature={temperature}"
hash_args += f", temperature={temperature}"
if tags:
repr_args += f", tags={tags}"
if start_time:
Expand Down Expand Up @@ -535,7 +541,28 @@


def _convert_temperature_to_kelvin(temperature_and_units):
"""Convert a temperature in Celsius or Kelvin to a temperature in Kelvin"""
"""
If the input is a 2D numpy array (time series), return an Interpolant.
Otherwise Convert a temperature in Celsius or Kelvin to a temperature in Kelvin
"""

# Check if the temperature input is a time-series array
if isinstance(temperature_and_units, np.ndarray):
if temperature_and_units.ndim == 2 and temperature_and_units.shape[1] == 2:
# Assume first column is time (s) and second column is temperature (K)
times = temperature_and_units[:, 0]
temps = temperature_and_units[:, 1]
# Create an interpolant function parameter that depends on time
return pybamm.Interpolant(times, temps, pybamm.t, interpolator="linear")
else:
raise ValueError(

Check warning on line 558 in src/pybamm/experiment/step/base_step.py

View check run for this annotation

Codecov / codecov/patch

src/pybamm/experiment/step/base_step.py#L558

Added line #L558 was not covered by tests
"Temperature time-series must be a 2D array with two columns (time, temperature)."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you write a test for this. Sometimes it is easiest to cover error handling code by extracting a smaller function and using that in the tests. You can make the if-else block a helper function, then covering it in tests should be easy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah sure, I'll add a test for it

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've added the tests


# If it's already an Interpolant, do nothing further.
if isinstance(temperature_and_units, pybamm.Interpolant):
return temperature_and_units

# If the temperature is a number, assume it is in Kelvin
if isinstance(temperature_and_units, (int, float)) or temperature_and_units is None:
return temperature_and_units
Expand Down
5 changes: 4 additions & 1 deletion src/pybamm/experiment/step/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,10 @@ class Voltage(BaseStepImplicit):
"""

def get_parameter_values(self, variables):
return {"Voltage function [V]": self.value}
params = {"Voltage function [V]": self.value}
if self.temperature is not None:
params["Ambient temperature [K]"] = self.temperature
return params

def get_submodel(self, model):
return pybamm.external_circuit.VoltageFunctionControl(
Expand Down
17 changes: 17 additions & 0 deletions src/pybamm/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,15 @@
if initial_soc is not None:
self.set_initial_soc(initial_soc, inputs=inputs)

if (
getattr(self, "experiment", None)
and hasattr(self.experiment, "temperature_interpolant")
and self.experiment.temperature_interpolant is not None
):
self.parameter_values.update(
{"Ambient temperature [K]": self.experiment.temperature_interpolant}
)

if self._built_model:
return
elif self._model.is_discretised:
Expand Down Expand Up @@ -444,6 +453,14 @@
See :meth:`pybamm.BaseSolver.solve`.
"""
pybamm.telemetry.capture("simulation-solved")
if (
getattr(self, "experiment", None)
and hasattr(self.experiment, "temperature_interpolant")
and self.experiment.temperature_interpolant is not None
):
self.parameter_values.update(

Check warning on line 461 in src/pybamm/simulation.py

View check run for this annotation

Codecov / codecov/patch

src/pybamm/simulation.py#L461

Added line #L461 was not covered by tests
{"Ambient temperature [K]": self.experiment.temperature_interpolant}
)

# Setup
if solver is None:
Expand Down
35 changes: 35 additions & 0 deletions tests/unit/test_experiments/test_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from datetime import datetime
import pybamm
import pytest
import numpy as np


class TestExperiment:
Expand Down Expand Up @@ -214,3 +215,37 @@ def test_set_next_start_time(self):

# TODO: once #3176 is completed, the test should pass for
# operating_conditions_steps (or equivalent) as well


def test_temperature_time_series_simulation():
time_data = np.array([0, 600, 1200, 1800])
voltage_data = np.array([4.2, 4.0, 3.8, 3.6])
temperature_data = np.array([298.15, 310.15, 305.15, 300.00])

voltage_profile = np.column_stack((time_data, voltage_data))
temperature_profile = np.column_stack((time_data, temperature_data))

experiment = pybamm.Experiment(
[pybamm.step.voltage(voltage_profile, temperature=temperature_profile)]
)

model = pybamm.lithium_ion.DFN()

param_values = pybamm.ParameterValues("Marquis2019")
param_values.update({"Ambient temperature [K]": 298.15})

sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param_values)
sim.build()

ambient_temp = sim.parameter_values["Ambient temperature [K]"]
assert hasattr(ambient_temp, "evaluate"), (
"Ambient temperature parameter is not time-dependent as expected."
)

t_eval = 600
interpolated_temp = ambient_temp.evaluate(t=t_eval)
np.testing.assert_allclose(interpolated_temp, 310.15, atol=1e-3)

t_eval2 = 1200
interpolated_temp2 = ambient_temp.evaluate(t=t_eval2)
np.testing.assert_allclose(interpolated_temp2, 305.15, atol=1e-3)