Skip to content

Commit

Permalink
#3530 fix unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
valentinsulzer committed Mar 1, 2024
1 parent 214e302 commit 923c2a5
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 28 deletions.
16 changes: 4 additions & 12 deletions pybamm/experiment/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ def __init__(
self.cycles = cycles
self.cycle_lengths = [len(cycle) for cycle in cycles]

self.steps_unprocessed = self._set_next_start_time(
[cond for cycle in cycles for cond in cycle]
)
steps_unprocessed = [cond for cycle in cycles for cond in cycle]

# Convert strings to pybamm.step.BaseStep objects
# We only do this once per unique step, to avoid unnecessary conversions
Expand All @@ -72,10 +70,11 @@ def __init__(
self.temperature = _convert_temperature_to_kelvin(temperature)

processed_steps = self.process_steps(
self.steps_unprocessed, self.period, self.temperature
steps_unprocessed, self.period, self.temperature
)

self.steps = [processed_steps[repr(step)] for step in self.steps_unprocessed]
self.steps = [processed_steps[repr(step)] for step in steps_unprocessed]
self.steps = self._set_next_start_time(self.steps)

# Save the processed unique steps and the processed operating conditions
# for every step
Expand Down Expand Up @@ -212,13 +211,6 @@ def _set_next_start_time(steps):
# Loop over the steps in reverse order, setting the end time of each step to the
# start time of the next step
for step in reversed(steps):
if isinstance(step, str):
step = pybamm.step.string(step)
if not isinstance(step, pybamm.step.BaseStep):
raise TypeError(
"Operating conditions should be strings or _Step objects"
)

step.next_start_time = next_start_time
step.end_time = end_time

Expand Down
6 changes: 2 additions & 4 deletions pybamm/experiment/step/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,7 @@ def get_parameter_values(self, variables):
return {"Power function [W]": self.value}

def get_submodel(self, model):
return pybamm.external_circuit.PowerFunctionControl(
model.param, model.options, control=self.control
)
return pybamm.external_circuit.PowerFunctionControl(model.param, model.options)


def power(value, **kwargs):
Expand Down Expand Up @@ -245,7 +243,7 @@ def get_parameter_values(self, variables):

def get_submodel(self, model):
return pybamm.external_circuit.ResistanceFunctionControl(
model.param, model.options, control=self.control
model.param, model.options
)


Expand Down
14 changes: 4 additions & 10 deletions pybamm/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,20 +637,14 @@ def solve(
# Use 1-indexing for printing cycle number as it is more
# human-intuitive
step = self.experiment.steps[idx]

# Hacky patch to allow correct processing of end_time and next_starting time
# For efficiency purposes, step treats identical steps as the same object
# regardless of the initial time. Should be refactored as part of #3176
step_unproc = self.experiment.steps_unprocessed[idx]

start_time = current_solution.t[-1]

# If step has an end time, dt must take that into account
if getattr(step_unproc, "end_time", None):
if step.end_time is not None:
dt = min(
step.duration,
(
step_unproc.end_time
step.end_time
- (
initial_start_time
+ timedelta(seconds=float(start_time))
Expand Down Expand Up @@ -703,9 +697,9 @@ def solve(
step_termination = step_solution.termination

# Add a padding rest step if necessary
if getattr(step_unproc, "next_start_time", None) is not None:
if step.next_start_time is not None:
rest_time = (
step_unproc.next_start_time
step.next_start_time
- (
initial_start_time
+ timedelta(seconds=float(step_solution.t[-1]))
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_experiments/test_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ def test_str_repr(self):

def test_bad_strings(self):
with self.assertRaisesRegex(
TypeError, "Operating conditions should be strings or _Step objects"
TypeError, "Operating conditions must be a Step object or string."
):
pybamm.Experiment([1, 2, 3])
with self.assertRaisesRegex(
TypeError, "Operating conditions should be strings or _Step objects"
TypeError, "Operating conditions must be a Step object or string."
):
pybamm.Experiment([(1, 2, 3)])

Expand Down

0 comments on commit 923c2a5

Please sign in to comment.