Skip to content
Merged
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
4 changes: 2 additions & 2 deletions .github/workflows/test_on_pull_request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-13, macos-14]
os: [ubuntu-latest, windows-latest]
python-version: ["3.12"]

name: Integration tests (${{ matrix.os }} / Python ${{ matrix.python-version }})
Expand Down Expand Up @@ -90,7 +90,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-13, macos-14]
os: [ubuntu-latest, windows-latest, macos-14]
Comment thread
BradyPlanden marked this conversation as resolved.
python-version: ["3.12"]

name: Test examples (${{ matrix.os }} / Python ${{ matrix.python-version }})
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

## Bug Fixes

- [#705](https://github.com/pybop-team/PyBOP/pull/705) - Bug fix `fitting_problem.evaulate()` failure return type alongside fixes for Pybamm `v25.4`.

## Breaking Changes

# [v25.3](https://github.com/pybop-team/PyBOP/tree/v25.3) - 2025-03-28
Expand Down
6 changes: 6 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import matplotlib
import numpy as np
import plotly
import pytest

Expand Down Expand Up @@ -76,3 +77,8 @@ def pytest_collection_modifyitems(config, items):
reason=f"Test does not match the selected options: {', '.join(selected_markers)}"
)
item.add_marker(skip_this)


@pytest.fixture(autouse=True)
def set_random_seed():
np.random.seed(40)
Comment thread
BradyPlanden marked this conversation as resolved.
25 changes: 20 additions & 5 deletions pybop/problems/fitting_problem.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import warnings
from typing import Optional
from typing import Optional, Union

import numpy as np
from pybamm import IDAKLUJax, SolverError
Expand Down Expand Up @@ -89,6 +89,12 @@ def __init__(
initial_state=self.initial_state,
)

self.error_out = {var: self.failure_output for var in self.output_variables}
self.error_sense = {
param: {var: self.failure_output for var in self.output_variables}
for param in self.parameters.keys()
}

def set_initial_state(self, initial_state: Optional[dict] = None):
"""
Set the initial state to be applied for every problem evaluation.
Expand All @@ -113,7 +119,12 @@ def set_initial_state(self, initial_state: Optional[dict] = None):

self.initial_state = initial_state

def evaluate(self, inputs: Inputs) -> dict[str, np.ndarray[np.float64]]:
def evaluate(
self, inputs: Inputs, eis=False
) -> Union[
dict[str, np.ndarray],
tuple[dict[str, np.ndarray], dict[str, dict[str, np.ndarray]]],
]:
"""
Evaluate the model with the given parameters and return the signal.

Expand All @@ -136,7 +147,10 @@ def evaluate(self, inputs: Inputs) -> dict[str, np.ndarray[np.float64]]:

def _evaluate(
self, func, inputs, calculate_grad=False
) -> dict[str, np.ndarray[np.float64]]:
) -> Union[
dict[str, np.ndarray],
tuple[dict[str, np.ndarray], dict[str, dict[str, np.ndarray]]],
]:
"""
Perform simulation using the specified method and handle exceptions.

Expand All @@ -163,8 +177,9 @@ def _evaluate(
except (SolverError, ZeroDivisionError, RuntimeError, ValueError) as e:
if isinstance(e, ValueError) and str(e) not in self.exception:
raise # Raise the error if it doesn't match the expected list
error_out = {s: self.failure_output for s in self.signal}
return (error_out, self.failure_output) if calculate_grad else error_out
if calculate_grad:
return self.error_out, self.error_sense
return self.error_out

if self.eis:
return sol
Expand Down
26 changes: 15 additions & 11 deletions tests/integration/test_model_experiment_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,19 @@ def final_cost(self, solution, model, parameters):

def test_multi_fitting_problem(self):
parameter_set = pybop.ParameterSet("Chen2020")
parameters = pybop.Parameter(
"Negative electrode active material volume fraction",
prior=pybop.Gaussian(0.68, 0.05),
true_value=parameter_set[
"Negative electrode active material volume fraction"
],
parameters = pybop.Parameters(
pybop.Parameter(
"Negative electrode active material volume fraction",
prior=pybop.Gaussian(0.68, 0.05),
true_value=parameter_set[
"Negative electrode active material volume fraction"
],
)
)

model_1 = pybop.lithium_ion.SPM(parameter_set=parameter_set)
experiment_1 = pybop.Experiment(
["Discharge at 1C until 3 V (4 seconds period)"]
["Discharge at 0.5C for 5 minutes (10 seconds period)"]
)
solution_1 = model_1.predict(experiment=experiment_1)
dataset_1 = pybop.Dataset(
Expand All @@ -133,7 +135,7 @@ def test_multi_fitting_problem(self):

model_2 = pybop.lithium_ion.SPMe(parameter_set=parameter_set.copy())
experiment_2 = pybop.Experiment(
["Discharge at 3C until 3 V (4 seconds period)"]
["Discharge at 1C for 3 minutes (10 seconds period)"]
)
solution_2 = model_2.predict(experiment=experiment_2)
dataset_2 = pybop.Dataset(
Expand All @@ -152,7 +154,9 @@ def test_multi_fitting_problem(self):

# Test with a gradient and non-gradient-based optimiser
for optimiser in [pybop.SNES, pybop.IRPropMin]:
optim = optimiser(cost)
optim = optimiser(
cost, sigma0=0.05, max_iterations=100, max_unchanged_iterations=30
)
results = optim.run()
np.testing.assert_allclose(results.x, parameters.true_value, atol=2e-5)
np.testing.assert_allclose(results.final_cost, 0, atol=2e-5)
np.testing.assert_allclose(results.x, parameters.true_value(), atol=2e-5)
np.testing.assert_allclose(results.final_cost, 0, atol=3e-5)
6 changes: 4 additions & 2 deletions tests/integration/test_spm_parameterisations.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 @@ -30,7 +31,8 @@ def model(self):
"Positive electrode active material volume fraction": x[1],
}
)
return pybop.lithium_ion.SPM(parameter_set=parameter_set)
solver = pybamm.IDAKLUSolver()
return pybop.lithium_ion.SPM(parameter_set=parameter_set, solver=solver)

@pytest.fixture
def parameters(self):
Expand Down Expand Up @@ -183,7 +185,7 @@ def test_optimisers(self, optim):

np.testing.assert_allclose(results.x, self.ground_truth, atol=1.5e-2)
if isinstance(optim.cost, pybop.GaussianLogLikelihood):
np.testing.assert_allclose(results.x[-1], self.sigma0, atol=5e-4)
np.testing.assert_allclose(results.x[-1], self.sigma0, atol=1e-3)

@pytest.fixture
def two_signal_cost(self, parameters, model, cost_cls):
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/test_thevenin_parameterisation.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ def model(self):
"R1 [Ohm]": self.ground_truth[1],
}
)
return pybop.empirical.Thevenin(parameter_set=parameter_set)
solver = IDAKLUSolver()
return pybop.empirical.Thevenin(parameter_set=parameter_set, solver=solver)

@pytest.fixture
def parameters(self):
Expand Down Expand Up @@ -78,11 +79,10 @@ def dataset(self, model):
(pybop.PSO, ""),
],
)
def test_optimisers_on_simple_model(
def test_optimisers_on_thevenin_model(
self, model, parameters, dataset, cost_class, optimiser, method
):
# Define the cost to optimise
model.solver = IDAKLUSolver()
problem = pybop.FittingProblem(model, parameters, dataset)
cost = cost_class(problem)

Expand Down Expand Up @@ -127,5 +127,5 @@ def get_data(self, model):
),
]
)
sim = model.predict(experiment=experiment)
return sim
sol = model.predict(experiment=experiment)
return sol
4 changes: 3 additions & 1 deletion tests/integration/test_transformation.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import itertools

import numpy as np
import pybamm
import pytest

import pybop
Expand Down Expand Up @@ -46,7 +47,8 @@ def model(self):
"R1 [Ohm]": self.ground_truth[1],
}
)
return pybop.empirical.Thevenin(parameter_set=parameter_set)
solver = pybamm.IDAKLUSolver()
return pybop.empirical.Thevenin(parameter_set=parameter_set, solver=solver)

@pytest.fixture
def parameters(self, transformation_r0, transformation_r1):
Expand Down
Loading