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

Improved option checking for tuples #4707

Draft
wants to merge 15 commits into
base: develop
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@

## Optimizations

- Enhanced option checking by converting string-based options into tuples for consistent handling.([#4707](https://github.com/pybamm-team/PyBaMM/pull/4707)
- Performance improvements to `IDAKLUSolver` initialization and processed variables. ([#4878](https://github.com/pybamm-team/PyBaMM/pull/4878))
- Improved search to handle cases with shorter input strings and provide more relevant results. ([#4735](https://github.com/pybamm-team/PyBaMM/pull/4735))

40 changes: 23 additions & 17 deletions src/pybamm/models/full_battery_models/base_battery_model.py
Original file line number Diff line number Diff line change
@@ -474,7 +474,7 @@ def __init__(self, extra_options):
# half-cells where you must pass a tuple of options to only set MSMR models in
# the working electrode
msmr_check_list = [
options[opt] == "MSMR"
"MSMR" in options[opt]
for opt in ["open-circuit potential", "particle", "intercalation kinetics"]
]
if (
@@ -531,7 +531,7 @@ def __init__(self, extra_options):
)

# Options not yet compatible with particle-size distributions
if options["particle size"] == "distribution":
if "distribution" in options["particle size"]:
if options["lithium plating porosity change"] != "false":
raise pybamm.OptionError(
"Lithium plating porosity change not yet supported for particle-size"
@@ -547,12 +547,15 @@ def __init__(self, extra_options):
"Heat of mixing submodels do not yet support particle-size "
"distributions."
)
if options["particle"] in ["quadratic profile", "quartic profile"]:
if (
"quadratic profile" in options["particle"]
or "quartic profile" in options["particle"]
):
raise NotImplementedError(
"'quadratic' and 'quartic' concentration profiles have not yet "
"been implemented for particle-size ditributions"
)
if options["particle shape"] != "spherical":
if "spherical" not in options["particle shape"]:
raise NotImplementedError(
"Particle shape must be 'spherical' for particle-size distribution"
" submodels."
@@ -620,8 +623,8 @@ def __init__(self, extra_options):

if options["particle phases"] not in ["1", ("1", "1")]:
if not (
options["surface form"] != "false"
and options["particle"] == "Fickian diffusion"
"false" not in options["surface form"]
and "Fickian diffusion" in options["particle"]
):
raise pybamm.OptionError(
"If there are multiple particle phases: 'surface form' cannot be "
@@ -641,7 +644,7 @@ def __init__(self, extra_options):
if isinstance(p_mechanics, str) and isinstance(sei_on_cr, tuple):
p_mechanics = (p_mechanics, p_mechanics)
if any(
sei == "true" and mech != "swelling and cracking"
"true" in sei and "swelling and cracking" not in mech
for mech, sei in zip(p_mechanics, sei_on_cr)
):
raise pybamm.OptionError(
@@ -984,11 +987,14 @@ def options(self, extra_options):
"Lead-acid models can only have thermal "
"effects if dimensionality is 0."
)
if options["SEI"] != "none" or options["SEI film resistance"] != "none":
if (
"none" not in options["SEI"]
or "none" not in options["SEI film resistance"]
):
raise pybamm.OptionError("Lead-acid models cannot have SEI formation")
if options["lithium plating"] != "none":
raise pybamm.OptionError("Lead-acid models cannot have lithium plating")
if options["open-circuit potential"] == "MSMR":
if "MSMR" in options["open-circuit potential"]:
raise pybamm.OptionError(
"Lead-acid models cannot use the MSMR open-circuit potential model"
)
@@ -1094,21 +1100,21 @@ def set_summary_variables(self):

def get_intercalation_kinetics(self, domain):
options = getattr(self.options, domain)
if options["intercalation kinetics"] == "symmetric Butler-Volmer":
if "symmetric Butler-Volmer" in options["intercalation kinetics"]:
return pybamm.kinetics.SymmetricButlerVolmer
elif options["intercalation kinetics"] == "asymmetric Butler-Volmer":
elif "asymmetric Butler-Volmer" in options["intercalation kinetics"]:
return pybamm.kinetics.AsymmetricButlerVolmer
elif options["intercalation kinetics"] == "linear":
elif "linear" in options["intercalation kinetics"]:
return pybamm.kinetics.Linear
elif options["intercalation kinetics"] == "Marcus":
return pybamm.kinetics.Marcus
elif options["intercalation kinetics"] == "Marcus-Hush-Chidsey":
elif "Marcus-Hush-Chidsey" in options["intercalation kinetics"]:
return pybamm.kinetics.MarcusHushChidsey
elif options["intercalation kinetics"] == "MSMR":
elif "Marcus" in options["intercalation kinetics"]:
return pybamm.kinetics.Marcus
elif "MSMR" in options["intercalation kinetics"]:
return pybamm.kinetics.MSMRButlerVolmer

def get_inverse_intercalation_kinetics(self):
if self.options["intercalation kinetics"] == "symmetric Butler-Volmer":
if "symmetric Butler-Volmer" in self.options["intercalation kinetics"]:
return pybamm.kinetics.InverseButlerVolmer
else:
raise pybamm.OptionError(
Original file line number Diff line number Diff line change
@@ -432,6 +432,62 @@ def test_get_coupled_variables(self):
with pytest.raises(pybamm.ModelError, match="Missing variable"):
model.build_model()

def test_different_input_types(self):
# Test invalid string input
with pytest.raises(
NotImplementedError,
match="SEI porosity change submodels do not yet support particle-size distributions.",
):
BatteryModelOptions(
{
"particle size": "distribution",
"SEI porosity change": "true",
}
)

# Test valid string input
opts = BatteryModelOptions(
{
"particle": "Fickian diffusion",
"loss of active material": "stress-driven",
}
)
assert opts.negative["particle"] == "Fickian diffusion"
assert opts.positive["particle"] == "Fickian diffusion"
assert opts["loss of active material"] == "stress-driven"

# Test invalid tuple input
with pytest.raises(
NotImplementedError,
match="SEI porosity change submodels do not yet support particle-size distributions.",
):
BatteryModelOptions(
{
"particle size": ("single", "distribution"),
"SEI porosity change": "true",
}
)

# Test valid tuple input
opts = BatteryModelOptions(
{
"particle": ("Fickian diffusion", "quadratic profile"),
}
)
assert opts.negative["particle"] == "Fickian diffusion"
assert opts.positive["particle"] == "quadratic profile"

with pytest.raises(pybamm.OptionError, match="loss of active material"):
BatteryModelOptions(
{
"loss of active material": (
"bad LAM model",
"bad LAM model",
"bad LAM model",
)
}
)

def test_default_solver(self):
model = pybamm.BaseBatteryModel()
assert isinstance(model.default_solver, pybamm.CasadiSolver)