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

Implemented non_linear_time, an array of time as an input to step function #3627

Merged
merged 34 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
046b3a5
Implemented non_linear_time, an array of time
prady0t Dec 17, 2023
5f1b8c1
style: pre-commit fixes
pre-commit-ci[bot] Dec 17, 2023
87e42e7
Modified step to take dt as required parameter
prady0t Dec 21, 2023
2f60de6
Merge remote-tracking branch 'origin/fixing-step-function' into fixin…
prady0t Dec 21, 2023
f60bdcd
Merge branch 'develop' into fixing-step-function
prady0t Dec 21, 2023
deea700
Merge branch 'develop' into fixing-step-function
prady0t Dec 30, 2023
a545a8c
style: pre-commit fixes
pre-commit-ci[bot] Dec 30, 2023
98df4d8
Fixed failing test cases
prady0t Jan 1, 2024
22469cc
style: pre-commit fixes
pre-commit-ci[bot] Jan 1, 2024
60b5112
Delete python
prady0t Jan 1, 2024
8fe3a53
Fixed failing test cases 2
prady0t Jan 2, 2024
2bc1417
Resolved conflicts
prady0t Jan 2, 2024
091938e
style: pre-commit fixes
pre-commit-ci[bot] Jan 2, 2024
e537336
Added tests
prady0t Jan 3, 2024
9c1ee7b
Merge remote-tracking branch 'refs/remotes/origin/fixing-step-functio…
prady0t Jan 3, 2024
56bd35a
style: pre-commit fixes
pre-commit-ci[bot] Jan 3, 2024
d4c6e26
Merge branch 'develop' into fixing-step-function
arjxn-py Feb 3, 2024
319037e
Update pybamm/simulation.py
rtimms Feb 15, 2024
b18dffc
Renamed non_linear_time to t_eval and other suggested changes
prady0t Feb 19, 2024
362f7a8
style: pre-commit fixes
pre-commit-ci[bot] Feb 19, 2024
4c3cd7b
Minor docstring fix
prady0t Feb 19, 2024
71b0d07
Merge remote-tracking branch 'origin/fixing-step-function' into fixin…
prady0t Feb 19, 2024
7ea5a01
Merge branch 'develop' into fixing-step-function
prady0t Feb 19, 2024
39d7035
Added deprecation warning
prady0t Feb 19, 2024
e0bafa1
style: pre-commit fixes
pre-commit-ci[bot] Feb 19, 2024
9be39fd
Added changelog.md entry
prady0t Feb 20, 2024
a9e5d29
Merge remote-tracking branch 'origin/fixing-step-function' into fixin…
prady0t Feb 20, 2024
9d2c230
Update CHANGELOG.md
prady0t Feb 20, 2024
932749e
Merge branch 'develop' into fixing-step-function
agriyakhetarpal Feb 21, 2024
33f8a06
Merge branch 'develop' into fixing-step-function
agriyakhetarpal Feb 22, 2024
d755aa6
Merge branch 'develop' into fixing-step-function
kratman Mar 12, 2024
8539999
Merge branch 'develop' into fixing-step-function
prady0t Mar 16, 2024
8afcea3
Merge branch 'develop' into fixing-step-function
kratman Mar 18, 2024
a1ac193
Merge branch 'develop' into fixing-step-function
kratman Mar 18, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Renamed "electrode diffusivity" to "particle diffusivity" as a non-breaking change with a deprecation warning ([#3624](https://github.com/pybamm-team/PyBaMM/pull/3624))
- Add support for BPX version 0.4.0 which allows for blended electrodes and user-defined parameters in BPX([#3414](https://github.com/pybamm-team/PyBaMM/pull/3414))
- Added the ability to specify a custom solver tolerance in `get_initial_stoichiometries` and related functions ([#3714](https://github.com/pybamm-team/PyBaMM/pull/3714))
- Modified `step` function to take an array of time `t_eval` as an argument and deprecated use of `npts`. ([#3627](https://github.com/pybamm-team/PyBaMM/pull/3627))
agriyakhetarpal marked this conversation as resolved.
Show resolved Hide resolved

## Bug Fixes

Expand Down
4 changes: 3 additions & 1 deletion examples/scripts/SPMe_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@

# step model
dt = 500
# t_eval is an array of time in the interval 0 to dt, dt being size of the step.
t_eval = np.array([0, 50, 100, 200, 500])
agriyakhetarpal marked this conversation as resolved.
Show resolved Hide resolved
time = 0
end_time = solution.t[-1]
step_solver = pybamm.CasadiSolver()
step_solution = None
while time < end_time:
step_solution = step_solver.step(step_solution, model, dt=dt, npts=10)
step_solution = step_solver.step(step_solution, model, dt=dt, t_eval=t_eval)
time += dt

# plot
Expand Down
26 changes: 19 additions & 7 deletions pybamm/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ def solve(
current_solution,
model,
dt,
npts=npts,
t_eval=np.linspace(0, dt, npts),
save=False,
**kwargs,
)
Expand Down Expand Up @@ -967,15 +967,21 @@ def run_padding_rest(self, kwargs, rest_time, step_solution):
step_solution,
model,
rest_time,
npts=npts,
prady0t marked this conversation as resolved.
Show resolved Hide resolved
t_eval=np.linspace(0, rest_time, npts),
save=False,
**kwargs,
)

return step_solution_with_rest

def step(
self, dt, solver=None, npts=2, save=True, starting_solution=None, **kwargs
self,
dt,
solver=None,
t_eval=None,
save=True,
starting_solution=None,
**kwargs,
):
"""
A method to step the model forward one timestep. This method will
Expand All @@ -987,9 +993,10 @@ def step(
The timestep over which to step the solution
solver : :class:`pybamm.BaseSolver`
The solver to use to solve the model.
npts : int, optional
The number of points at which the solution will be returned during
the step dt. Default is 2 (returns the solution at t0 and t0 + dt).
t_eval : list or numpy.ndarray, optional
An array of times at which to return the solution during the step
(Note: t_eval is the time measured from the start of the step, so should start at 0 and end at dt).
By default, the solution is returned at t0 and t0 + dt.
save : bool
Turn on to store the solution of all previous timesteps
starting_solution : :class:`pybamm.Solution`
Expand All @@ -1009,7 +1016,12 @@ def step(
starting_solution = self._solution

self._solution = solver.step(
starting_solution, self.built_model, dt, npts=npts, save=save, **kwargs
starting_solution,
self.built_model,
dt,
t_eval=t_eval,
save=save,
**kwargs,
)

return self.solution
Expand Down
45 changes: 30 additions & 15 deletions pybamm/solvers/base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,8 @@ def step(
old_solution,
model,
dt,
npts=2,
t_eval=None,
npts=None,
inputs=None,
save=True,
):
Expand All @@ -1120,9 +1121,11 @@ def step(
initial_conditions
dt : numeric type
The timestep (in seconds) over which to step the solution
npts : int, optional
The number of points at which the solution will be returned during
the step dt. default is 2 (returns the solution at t0 and t0 + dt).
t_eval : list or numpy.ndarray, optional
An array of times at which to return the solution during the step
(Note: t_eval is the time measured from the start of the step, so should start at 0 and end at dt).
By default, the solution is returned at t0 and t0 + dt.
npts : deprecated
inputs : dict, optional
Any input parameters to pass to the model when solving
save : bool
Expand Down Expand Up @@ -1161,10 +1164,28 @@ def step(
f"Step time must be at least {pybamm.TimerTime(step_start_offset)}"
)

# Raise deprecation warning for npts and convert it to t_eval
if npts is not None:
warnings.warn(
"The 'npts' parameter is deprecated, use 't_eval' instead.",
DeprecationWarning,
stacklevel=2,
)
t_eval = np.linspace(0, dt, npts)

if t_eval is not None:
# Checking if t_eval lies within range
if t_eval[0] != 0 or t_eval[-1] != dt:
raise pybamm.SolverError(
"Elements inside array t_eval must lie in the closed interval 0 to dt"
)

else:
t_eval = np.array([0, dt])

t_start = old_solution.t[-1]
t_eval = t_start + t_eval
t_end = t_start + dt
# Calculate t_eval
t_eval = np.linspace(t_start, t_end, npts)
prady0t marked this conversation as resolved.
Show resolved Hide resolved

if t_start == 0:
t_start_shifted = t_start
Expand Down Expand Up @@ -1248,15 +1269,9 @@ def step(
# Report times
pybamm.logger.verbose(f"Finish stepping {model.name} ({termination})")
pybamm.logger.verbose(
(
"Set-up time: {}, Step time: {} (of which integration time: {}), "
"Total time: {}"
).format(
solution.set_up_time,
solution.solve_time,
solution.integration_time,
solution.total_time,
)
f"Set-up time: {solution.set_up_time}, Step time: {solution.solve_time}"
f"(of which integration time: {solution.integration_time}), "
f"Total time: {solution.total_time}"
)

# Return solution
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/test_solvers/test_base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,22 @@ def test_nonmonotonic_teval(self):
):
solver.step(None, model, dt)

# Checking if array t_eval lies within range
dt = 2
t_eval = np.array([0, 1])
with self.assertRaisesRegex(
pybamm.SolverError,
"Elements inside array t_eval must lie in the closed interval 0 to dt",
):
solver.step(None, model, dt, t_eval=t_eval)

t_eval = np.array([1, dt])
with self.assertRaisesRegex(
pybamm.SolverError,
"Elements inside array t_eval must lie in the closed interval 0 to dt",
):
solver.step(None, model, dt, t_eval=t_eval)

def test_solution_time_length_fail(self):
model = pybamm.BaseModel()
v = pybamm.Scalar(1)
Expand Down