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.

Sign up for GitHub

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

Jump to bottom

#3690 fix issue with skipped steps #3708

Merged
merged 3 commits into from
Jan 15, 2024
Merged

#3690 fix issue with skipped steps #3708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89e6155
#3690 fix issue with skipped steps
rtimms Jan 10, 2024
020f07d
#3690 changelog
rtimms Jan 10, 2024
1baae8c
#3690 add test
rtimms Jan 10, 2024
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# [Unreleased](https://github.com/pybamm-team/PyBaMM/)

## Bug Fixes

- Fixed a bug where if the first step(s) in a cycle are skipped then the cycle solution started from the model's initial conditions instead of from the last state of the previous cycle ([#3708](https://github.com/pybamm-team/PyBaMM/pull/3708))
Comment on lines +3 to +5
Copy link
Member

Choose a reason for hiding this comment

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

if this is going into 24.1 this needs to move

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what’s the process? should i just manually make a new section for the next release candidate?

Copy link
Member

Choose a reason for hiding this comment

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

You can leave it as it is. The workflow will automatically create a v24.1rc1 section with the new changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool. where/when should this be merged?

# [v24.1rc0](https://github.com/pybamm-team/PyBaMM/tree/v24.1rc0) - 2024-01-31

## Features
Expand Down
15 changes: 14 additions & 1 deletion pybamm/simulation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,20 @@ def solve(

steps.append(step_solution)

cycle_solution = cycle_solution + step_solution
# If there haven't been any successful steps yet in this cycle, then
# carry the solution over from the previous cycle (but
# `step_solution` should still be an EmptySolution so that in the
# list of returned step solutions we can see which steps were
# skipped)
if (
cycle_solution is None
and isinstance(step_solution, pybamm.EmptySolution)
and not isinstance(current_solution, pybamm.EmptySolution)
):
cycle_solution = current_solution.last_state
else:
cycle_solution = cycle_solution + step_solution

current_solution = cycle_solution

callbacks.on_step_end(logs)
Expand Down
19 changes: 19 additions & 0 deletions tests/unit/test_experiments/test_simulation_with_experiment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,25 @@ def test_run_experiment_skip_steps(self):
decimal=5,
)

def test_skipped_step_continuous(self):
model = pybamm.lithium_ion.SPM({"SEI": "solvent-diffusion limited"})
experiment = pybamm.Experiment(
[
("Rest for 24 hours (1 hour period)",),
(
"Charge at C/3 until 4.1 V",
"Hold at 4.1V until C/20",
"Discharge at C/3 until 2.5 V",
),
]
)
sim = pybamm.Simulation(model, experiment=experiment)
sim.solve(initial_soc=1)
np.testing.assert_array_almost_equal(
sim.solution.cycles[0].last_state.y.full(),
sim.solution.cycles[1].steps[-1].first_state.y.full(),
)

def test_all_empty_solution_errors(self):
model = pybamm.lithium_ion.SPM()
parameter_values = pybamm.ParameterValues("Chen2020")
Expand Down
Loading