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

Default solver from Casadi to IDAKLU #546

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
8 changes: 8 additions & 0 deletions pybop/models/base_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,14 @@ def set_up_for_eis(self, model):
model : pybamm.Model
The PyBaMM model to be used for EIS simulations.
"""
if not isinstance(self._solver, pybamm.CasadiSolver):
print(
"CasadiSolver is required for EIS predictions, the solver has been changed."
)
self._solver = pybamm.CasadiSolver(
atol=self._solver.atol, rtol=self._solver.rtol
)

V_cell = pybamm.Variable("Voltage variable [V]")
model.variables["Voltage variable [V]"] = V_cell
V = model.variables["Voltage [V]"]
Expand Down
2 changes: 1 addition & 1 deletion pybop/models/empirical/base_ecm.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def __init__(
self._spatial_methods = (
spatial_methods or self.pybamm_model.default_spatial_methods
)
self._solver = solver or self.pybamm_model.default_solver
self._solver = solver or pybamm.IDAKLUSolver()

# Internal attributes for the built model are initialized but not set
self._model_with_set_params = None
Expand Down
5 changes: 2 additions & 3 deletions pybop/models/lithium_ion/base_echem.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import warnings
from typing import Optional

import pybamm
from pybamm import lithium_ion as pybamm_lithium_ion

from pybop.models.base_model import BaseModel, Inputs
Expand Down Expand Up @@ -72,9 +73,7 @@ def __init__(
spatial_methods or self.pybamm_model.default_spatial_methods
)
if solver is None:
self._solver = self.pybamm_model.default_solver
self._solver.mode = "fast with events"
self._solver.max_step_decrease_count = 1
self._solver = pybamm.IDAKLUSolver(atol=1e-5, rtol=1e-5)
else:
self._solver = solver

Expand Down
23 changes: 15 additions & 8 deletions tests/integration/test_model_experiment_changes.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import numpy as np
import pybamm
import pytest

import pybop
Expand Down Expand Up @@ -34,14 +35,18 @@ class TestModelAndExperimentChanges:
def parameters(self, request):
return request.param

@pytest.fixture
def solver(self):
return pybamm.IDAKLUSolver(atol=1e-6, rtol=1e-6)

@pytest.mark.integration
def test_changing_experiment(self, parameters):
def test_changing_experiment(self, parameters, solver):
# Change the experiment and check that the results are different.

parameter_set = pybop.ParameterSet.pybamm("Chen2020")
parameter_set.update(parameters.as_dict("true"))
initial_state = {"Initial SoC": 0.5}
model = pybop.lithium_ion.SPM(parameter_set=parameter_set)
model = pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)

t_eval = np.arange(0, 3600, 2) # Default 1C discharge to cut-off voltage
solution_1 = model.predict(initial_state=initial_state, t_eval=t_eval)
Expand All @@ -66,19 +71,19 @@ def test_changing_experiment(self, parameters):
np.testing.assert_allclose(cost_2, 0, atol=1e-5)

@pytest.mark.integration
def test_changing_model(self, parameters):
def test_changing_model(self, parameters, solver):
# Change the model and check that the results are different.

parameter_set = pybop.ParameterSet.pybamm("Chen2020")
parameter_set.update(parameters.as_dict("true"))
initial_state = {"Initial SoC": 0.5}
experiment = pybop.Experiment(["Charge at 1C until 4.1 V (2 seconds period)"])

model = pybop.lithium_ion.SPM(parameter_set=parameter_set)
model = pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
solution_1 = model.predict(initial_state=initial_state, experiment=experiment)
cost_1 = self.final_cost(solution_1, model, parameters)

model = pybop.lithium_ion.SPMe(parameter_set=parameter_set)
model = pybop.lithium_ion.SPMe(parameter_set=parameter_set, solver=solver)
solution_2 = model.predict(initial_state=initial_state, experiment=experiment)
cost_2 = self.final_cost(solution_2, model, parameters)

Expand Down Expand Up @@ -109,7 +114,7 @@ def final_cost(self, solution, model, parameters):
return results.final_cost

@pytest.mark.integration
def test_multi_fitting_problem(self):
def test_multi_fitting_problem(self, solver):
parameter_set = pybop.ParameterSet.pybamm("Chen2020")
parameters = pybop.Parameter(
"Negative electrode active material volume fraction",
Expand All @@ -119,7 +124,7 @@ def test_multi_fitting_problem(self):
],
)

model_1 = pybop.lithium_ion.SPM(parameter_set=parameter_set)
model_1 = pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)
experiment_1 = pybop.Experiment(
["Discharge at 1C until 3 V (4 seconds period)"]
)
Expand All @@ -132,7 +137,9 @@ def test_multi_fitting_problem(self):
}
)

model_2 = pybop.lithium_ion.SPMe(parameter_set=parameter_set.copy())
model_2 = pybop.lithium_ion.SPMe(
parameter_set=parameter_set.copy(), solver=solver
)
experiment_2 = pybop.Experiment(
["Discharge at 3C until 3 V (4 seconds period)"]
)
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_posterior.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import numpy as np
import pybamm
import pytest
import scipy.stats as st

Expand All @@ -12,7 +13,7 @@ class TestLogPosterior:

@pytest.fixture
def model(self):
return pybop.lithium_ion.SPM()
return pybop.lithium_ion.SPM(solver=pybamm.IDAKLUSolver(atol=1e-6, rtol=1e-6))

@pytest.fixture
def ground_truth(self):
Expand Down
Loading