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

Add functions of time in experiment step #4222

Merged
merged 8 commits into from
Jun 28, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Features

- Added functionality to pass in arbitrary functions of time as the argument for a (`pybamm.step`). ([#4222](https://github.com/pybamm-team/PyBaMM/pull/4222))
Copy link
Member

Choose a reason for hiding this comment

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

This should be made into a new section as it won't be included in this release

Copy link
Member Author

Choose a reason for hiding this comment

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

This is under Unreleased right now. Do you want me to move everything else under Unreleased into a new section?

Copy link
Member

Choose a reason for hiding this comment

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

Yes - though let's check with release team @Saransh-cpp @agriyakhetarpal @kratman to see the best way to deal with this

Copy link
Member

Choose a reason for hiding this comment

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

Since this PR is going into the develop branch and is a feature addition, the merged commit here won't be cherry-picked into further release candidates – it will go straight into v24.9. I think it should be fine to leave as is. Ideally, cherry-picked bug fixes will carry their own CHANGELOG entries into the next release candidate or final release.

Since the release notes are still copied over manually, we'll have a chance there to check for unneeded entries

- Added new parameters `"f{pref]Initial inner SEI on cracks thickness [m]"` and `"f{pref]Initial outer SEI on cracks thickness [m]"`, instead of hardcoding these to `L_inner_0 / 10000` and `L_outer_0 / 10000`. ([#4168](https://github.com/pybamm-team/PyBaMM/pull/4168))
- Added `pybamm.DataLoader` class to fetch data files from [pybamm-data](https://github.com/pybamm-team/pybamm-data/releases/tag/v1.0.0) and store it under local cache. ([#4098](https://github.com/pybamm-team/PyBaMM/pull/4098))
- Added `time` as an option for `Experiment.termination`. Now allows solving up to a user-specified time while also allowing different cycles and steps in an experiment to be handled normally. ([#4073](https://github.com/pybamm-team/PyBaMM/pull/4073))
Expand Down
22 changes: 21 additions & 1 deletion pybamm/experiment/step/base_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
----------
value : float
The value of the step, corresponding to the type of step. Can be a number, a
2-tuple (for cccv_ode), or a 2-column array (for drive cycles)
2-tuple (for cccv_ode), a 2-column array (for drive cycles), or a 1-argument function of t
duration : float, optional
The duration of the step in seconds.
termination : str or list, optional
Expand Down Expand Up @@ -73,6 +73,7 @@
):
# Check if drive cycle
self.is_drive_cycle = isinstance(value, np.ndarray)
is_python_function = callable(value)
if self.is_drive_cycle:
if value.ndim != 2 or value.shape[1] != 2:
raise ValueError(
Expand All @@ -83,6 +84,21 @@
t = value[:, 0]
if t[0] != 0:
raise ValueError("Drive cycle must start at t=0")
elif is_python_function:
t0 = 0
# Check if the function is only a function of t
try:
value_t0 = value(t0)
except TypeError:
raise TypeError(

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

View check run for this annotation

Codecov / codecov/patch

pybamm/experiment/step/base_step.py#L92-L93

Added lines #L92 - L93 were not covered by tests
"Input function must have only 1 positional argument for time"
) from None

# Check if the value at t0 is feasible
if not (np.isfinite(value_t0) and np.isscalar(value_t0)):
raise ValueError(

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

View check run for this annotation

Codecov / codecov/patch

pybamm/experiment/step/base_step.py#L99

Added line #L99 was not covered by tests
f"Input function must return a real number output at t = {t0}"
)

# Set duration
if duration is None:
Expand Down Expand Up @@ -135,6 +151,10 @@
name="Drive Cycle",
)
self.period = np.diff(t).min()
elif is_python_function:
t = pybamm.t - pybamm.InputParameter("start time")
self.value = value(t)
self.period = _convert_time_to_seconds(period)
else:
self.value = value
self.period = _convert_time_to_seconds(period)
Expand Down
3 changes: 3 additions & 0 deletions pybamm/experiment/step/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,9 @@ def value_based_charge_or_discharge(step_value):
elif isinstance(step_value, pybamm.Symbol):
inpt = {"start time": 0}
init_curr = step_value.evaluate(t=0, inputs=inpt).flatten()[0]
elif callable(step_value):
inpt = {"start time": 0}
init_curr = step_value(pybamm.t).evaluate(t=0, inputs=inpt).flatten()[0]
else:
init_curr = step_value
sign = np.sign(init_curr)
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/test_simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,33 @@ def test_step_with_inputs(self):
sim.solution.all_inputs[1]["Current function [A]"], 2
)

def test_time_varying_input_function(self):
tf = 20.0

def oscillating(t):
return 3.6 + 0.1 * np.sin(2 * np.pi * t / tf)

model = pybamm.lithium_ion.SPM()

operating_modes = {
"Current [A]": pybamm.step.current,
"C-rate": pybamm.step.c_rate,
"Voltage [V]": pybamm.step.voltage,
"Power [W]": pybamm.step.power,
}
for name in operating_modes:
step = operating_modes[name](oscillating, duration=tf / 2)
experiment = pybamm.Experiment([step, step], period=f"{tf / 100} seconds")

solver = pybamm.CasadiSolver(rtol=1e-8, atol=1e-8)
sim = pybamm.Simulation(model, experiment=experiment, solver=solver)
sim.solve()
for sol in sim.solution.sub_solutions:
t0 = sol.t[0]
np.testing.assert_array_almost_equal(
sol[name].entries, np.array(oscillating(sol.t - t0))
)

def test_save_load(self):
with TemporaryDirectory() as dir_name:
test_name = os.path.join(dir_name, "tests.pickle")
Expand Down
Loading