Skip to content

Commit

Permalink
fix: updates scipy bounds with keep_feasible, clipping groundtruth fo…
Browse files Browse the repository at this point in the history
…r integration tests, small fixes on examples
  • Loading branch information
BradyPlanden committed Aug 27, 2024
1 parent 1eebc18 commit 30b6101
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 3,803 deletions.
3,811 changes: 27 additions & 3,784 deletions examples/notebooks/ecm_trust-constr.ipynb

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions examples/scripts/ecm_tau_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def check_params(
)

# Fitting parameters
parameters = [
parameters = pybop.Parameters(
pybop.Parameter(
"R0 [Ohm]",
prior=pybop.Gaussian(0.0002, 0.0001),
Expand All @@ -150,7 +150,7 @@ def check_params(
prior=pybop.Gaussian(10000, 2500),
bounds=[2500, 5e4],
),
]
)

sigma = 0.001
t_eval = np.arange(0, 900, 3)
Expand All @@ -172,6 +172,7 @@ def check_params(
optim = pybop.XNES(
cost,
allow_infeasible_solutions=False,
max_iterations=100,
)

x, final_cost = optim.run()
Expand Down
23 changes: 14 additions & 9 deletions pybop/optimisers/scipy_optimisers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from typing import Union

import numpy as np
from scipy.optimize import OptimizeResult, differential_evolution, minimize
from scipy.optimize import Bounds, OptimizeResult, differential_evolution, minimize

from pybop import BaseOptimiser, Result

Expand Down Expand Up @@ -55,12 +55,18 @@ def _sanitise_inputs(self):

# Convert bounds to SciPy format
if isinstance(self.bounds, dict):
self._scipy_bounds = [
(lower, upper)
for lower, upper in zip(self.bounds["lower"], self.bounds["upper"])
]
else:
self._scipy_bounds = Bounds(
self.bounds["lower"], self.bounds["upper"], True
)
elif isinstance(self.bounds, list):
lb, ub = zip(*self.bounds)
self._scipy_bounds = Bounds(lb, ub, True)
elif isinstance(self.bounds, Bounds) or self.bounds is None:
self._scipy_bounds = self.bounds
else:
raise TypeError(

Check warning on line 67 in pybop/optimisers/scipy_optimisers.py

View check run for this annotation

Codecov / codecov/patch

pybop/optimisers/scipy_optimisers.py#L67

Added line #L67 was not covered by tests
"Bounds provided must be either type dict, list or SciPy.optimize.bounds object."
)

def _run(self):
"""
Expand Down Expand Up @@ -286,9 +292,8 @@ def _set_up_optimiser(self):
if self._scipy_bounds is None:
raise ValueError("Bounds must be specified for differential_evolution.")
else:
if not all(
np.isfinite(value) for pair in self._scipy_bounds for value in pair
):
bnds = self._scipy_bounds
if not (np.isfinite(bnds.lb).all() and np.isfinite(bnds.ub).all()):
raise ValueError("Bounds must be specified for differential_evolution.")

# Apply default maxiter and tolerance
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/test_thevenin_parameterisation.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ class TestTheveninParameterisation:

@pytest.fixture(autouse=True)
def setup(self):
self.ground_truth = np.asarray([0.05, 0.05]) + np.random.normal(
loc=0.0, scale=0.01, size=2
self.ground_truth = np.clip(
np.asarray([0.05, 0.05]) + np.random.normal(loc=0.0, scale=0.01, size=2),
a_min=0.0,
a_max=0.1,
)

@pytest.fixture
Expand Down
12 changes: 8 additions & 4 deletions tests/integration/test_transformation.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ class TestTransformation:

@pytest.fixture(autouse=True)
def setup(self):
self.ground_truth = np.array([0.5, 0.2]) + np.random.normal(
loc=0.0, scale=0.05, size=2
self.ground_truth = np.clip(
np.array([0.5, 0.1]) + np.random.normal(loc=0.0, scale=0.05, size=2),
a_min=[0.375, 0.02],
a_max=[0.7, 0.5],
)

@pytest.fixture
Expand All @@ -39,7 +41,7 @@ def parameters(self):
pybop.Parameter(
"Positive electrode conductivity [S.m-1]",
prior=pybop.Uniform(0.05, 0.45),
bounds=[0.04, 0.5],
bounds=[0.02, 0.5],
transformation=pybop.LogTransformation(),
),
)
Expand Down Expand Up @@ -78,10 +80,12 @@ def cost(self, model, parameters, init_soc):
@pytest.mark.integration
def test_spm_optimisers(self, optimiser, cost):
x0 = cost.parameters.initial_value()
sigma0 = None if optimiser is pybop.AdamW else 0.2
optim = optimiser(
cost=cost,
sigma0=sigma0,
max_unchanged_iterations=35,
max_iterations=125,
max_iterations=150,
)

initial_cost = optim.cost(x0)
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/test_weighted_cost.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ class TestWeightedCost:
@pytest.fixture(autouse=True)
def setup(self):
self.sigma0 = 0.002
self.ground_truth = np.asarray([0.55, 0.55]) + np.random.normal(
loc=0.0, scale=0.05, size=2
self.ground_truth = np.clip(
np.asarray([0.55, 0.55]) + np.random.normal(loc=0.0, scale=0.05, size=2),
a_min=0.4,
a_max=0.75,
)

@pytest.fixture
Expand Down

0 comments on commit 30b6101

Please sign in to comment.