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

Removing old arguments #3682

Merged
merged 8 commits into from
Jan 25, 2024
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
1 change: 0 additions & 1 deletion pybamm/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@
from .parameters.ecm_parameters import EcmParameters
from .parameters.size_distribution_parameters import *
from .parameters.parameter_sets import parameter_sets
from .parameters_cli import add_parameter, remove_parameter, edit_parameter

#
# Mesh and Discretisation classes
Expand Down
67 changes: 30 additions & 37 deletions pybamm/experiment/experiment.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
#
# Experiment class
#

from __future__ import annotations
import pybamm
from .step._steps_util import (
Expand Down Expand Up @@ -47,20 +43,7 @@ def __init__(
period: str = "1 minute",
temperature: float | None = None,
termination: list[str] | None = None,
drive_cycles=None,
cccv_handling=None,
):
if cccv_handling is not None:
raise ValueError(
"cccv_handling has been deprecated, use "
"`pybamm.step.cccv_ode(current, voltage)` instead to produce the "
"same behavior as the old `cccv_handling='ode'`"
)
if drive_cycles is not None:
raise ValueError(
"drive_cycles should now be passed as an experiment step object, e.g. "
"`pybamm.step.current(drive_cycle)`"
)
# Save arguments for copying
self.args = (
operating_conditions,
Expand All @@ -71,7 +54,6 @@ def __init__(

operating_conditions_cycles = []
for cycle in operating_conditions:
# Check types and convert to list
if not isinstance(cycle, tuple):
cycle = (cycle,)
operating_conditions_cycles.append(cycle)
Expand All @@ -84,26 +66,14 @@ def __init__(
)

# Convert strings to pybamm.step._Step objects
# We only do this once per unique step, do avoid unnecessary conversions
# We only do this once per unique step, to avoid unnecessary conversions
# Assign experiment period and temperature if not specified in step
self.period = _convert_time_to_seconds(period)
self.temperature = _convert_temperature_to_kelvin(temperature)

processed_steps = {}
for step in self.operating_conditions_steps_unprocessed:
if repr(step) in processed_steps:
continue
elif isinstance(step, str):
processed_step = pybamm.step.string(step)
elif isinstance(step, pybamm.step._Step):
processed_step = step

if processed_step.period is None:
processed_step.period = self.period
if processed_step.temperature is None:
processed_step.temperature = self.temperature

processed_steps[repr(step)] = processed_step
processed_steps = self.process_steps(
self.operating_conditions_steps_unprocessed, self.period, self.temperature
)

self.operating_conditions_steps = [
processed_steps[repr(step)]
Expand All @@ -129,6 +99,27 @@ def __init__(
self.termination_string = termination
self.termination = self.read_termination(termination)

@staticmethod
def process_steps(unprocessed_steps, period, temp):
processed_steps = {}
for step in unprocessed_steps:
if repr(step) in processed_steps:
continue
elif isinstance(step, str):
processed_step = pybamm.step.string(step)
elif isinstance(step, pybamm.step._Step):
processed_step = step
else:
raise TypeError("Operating conditions must be a Step object or string.")

if processed_step.period is None:
processed_step.period = period
if processed_step.temperature is None:
processed_step.temperature = temp

processed_steps[repr(step)] = processed_step
return processed_steps

Comment on lines +102 to +121
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rtimms I extracted this method so it was easily testable. I added a test for it.

def __str__(self):
return str(self.operating_conditions_cycles)

Expand All @@ -138,7 +129,8 @@ def copy(self):
def __repr__(self):
return f"pybamm.Experiment({self!s})"

def read_termination(self, termination):
@staticmethod
def read_termination(termination):
"""
Read the termination reason. If this condition is hit, the experiment will stop.
"""
Expand Down Expand Up @@ -197,7 +189,8 @@ def search_tag(self, tag):

return cycles

def _set_next_start_time(self, operating_conditions):
@staticmethod
def _set_next_start_time(operating_conditions):
if all(isinstance(i, str) for i in operating_conditions):
return operating_conditions

Expand All @@ -207,7 +200,7 @@ def _set_next_start_time(self, operating_conditions):
for op in reversed(operating_conditions):
if isinstance(op, str):
op = pybamm.step.string(op)
elif not isinstance(op, pybamm.step._Step):
if not isinstance(op, pybamm.step._Step):
raise TypeError(
"Operating conditions should be strings or _Step objects"
)
Expand Down
9 changes: 0 additions & 9 deletions pybamm/expression_tree/interpolant.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#
import numpy as np
from scipy import interpolate
import warnings

import pybamm

Expand Down Expand Up @@ -49,14 +48,6 @@ def __init__(
extrapolate=True,
entries_string=None,
):
# "cubic spline" has been renamed to "cubic"
if interpolator == "cubic spline":
interpolator = "cubic"
warnings.warn(
"The 'cubic spline' interpolator has been renamed to 'cubic'.",
DeprecationWarning,
)

# Check interpolator is valid
if interpolator not in ["linear", "cubic", "pchip"]:
raise ValueError(f"interpolator '{interpolator}' not recognised")
Expand Down
29 changes: 0 additions & 29 deletions pybamm/models/full_battery_models/lithium_ion/electrode_soh.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,35 +363,6 @@ def __get_electrode_soh_sims_split(self):
return [x100_sim, x0_sim]

def solve(self, inputs):
if "n_Li" in inputs:
warnings.warn(
"Input 'n_Li' has been replaced by 'Q_Li', which is 'n_Li * F / 3600'. "
"This will be automatically calculated for now. "
"Q_Li can be read from parameters as 'param.Q_Li_particles_init'",
DeprecationWarning,
)
n_Li = inputs.pop("n_Li")
inputs["Q_Li"] = n_Li * pybamm.constants.F.value / 3600
if "C_n" in inputs:
warnings.warn("Input 'C_n' has been renamed to 'Q_n'", DeprecationWarning)
inputs["Q_n"] = inputs.pop("C_n")
if "C_p" in inputs:
warnings.warn("Input 'C_p' has been renamed to 'Q_p'", DeprecationWarning)
inputs["Q_p"] = inputs.pop("C_p")
if inputs.pop("V_min", None) is not None:
warnings.warn(
"V_min has been removed from the inputs. "
"The 'Open-circuit voltage at 0% SOC [V]' "
"parameter is now used automatically.",
DeprecationWarning,
)
if inputs.pop("V_max", None) is not None:
warnings.warn(
"V_max has been removed from the inputs. "
"The 'Open-circuit voltage at 100% SOC [V]' "
"parameter is now used automatically.",
DeprecationWarning,
)
ics = self._set_up_solve(inputs)
try:
sol = self._solve_full(inputs, ics)
Expand Down
19 changes: 0 additions & 19 deletions pybamm/parameters_cli.py

This file was deleted.

3 changes: 0 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,6 @@ all = [
]

[project.scripts]
pybamm_edit_parameter = "pybamm.parameters_cli:edit_parameter"
pybamm_add_parameter = "pybamm.parameters_cli:add_parameter"
pybamm_rm_parameter = "pybamm.parameters_cli:remove_parameter"
pybamm_install_odes = "pybamm.install_odes:main"
pybamm_install_jax = "pybamm.util:install_jax"

Expand Down
25 changes: 14 additions & 11 deletions tests/unit/test_experiments/test_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ def test_cycle_unpacking(self):
)
self.assertEqual(experiment.cycle_lengths, [2, 1, 1])

def test_invalid_step_type(self):
unprocessed = {1.0}
period = 1
temperature = 300.0
with self.assertRaisesRegex(
TypeError, "Operating conditions must be a Step object or string."
):
pybamm.Experiment.process_steps(unprocessed, period, temperature)

def test_str_repr(self):
conds = ["Discharge at 1 C for 20 seconds", "Charge at 0.5 W for 10 minutes"]
experiment = pybamm.Experiment(conds)
Expand All @@ -91,38 +100,32 @@ def test_bad_strings(self):
):
pybamm.Experiment([(1, 2, 3)])

def test_deprecations(self):
with self.assertRaisesRegex(ValueError, "cccv_handling"):
pybamm.Experiment([], cccv_handling="something")
with self.assertRaisesRegex(ValueError, "drive_cycles"):
pybamm.Experiment([], drive_cycles="something")

def test_termination(self):
experiment = pybamm.Experiment(["Discharge at 1 C for 20 seconds"])
self.assertEqual(experiment.termination, {})

experiment = pybamm.Experiment(
["Discharge at 1 C for 20 seconds"], termination="80.7% capacity"
["Discharge at 1 C for 20 seconds"], termination=["80.7% capacity"]
)
self.assertEqual(experiment.termination, {"capacity": (80.7, "%")})

experiment = pybamm.Experiment(
["Discharge at 1 C for 20 seconds"], termination="80.7 % capacity"
["Discharge at 1 C for 20 seconds"], termination=["80.7 % capacity"]
)
self.assertEqual(experiment.termination, {"capacity": (80.7, "%")})

experiment = pybamm.Experiment(
["Discharge at 1 C for 20 seconds"], termination="4.1Ah capacity"
["Discharge at 1 C for 20 seconds"], termination=["4.1Ah capacity"]
)
self.assertEqual(experiment.termination, {"capacity": (4.1, "Ah")})

experiment = pybamm.Experiment(
["Discharge at 1 C for 20 seconds"], termination="4.1 A.h capacity"
["Discharge at 1 C for 20 seconds"], termination=["4.1 A.h capacity"]
)
self.assertEqual(experiment.termination, {"capacity": (4.1, "Ah")})

experiment = pybamm.Experiment(
["Discharge at 1 C for 20 seconds"], termination="3V"
["Discharge at 1 C for 20 seconds"], termination=["3V"]
)
self.assertEqual(experiment.termination, {"voltage": (3, "V")})

Expand Down
9 changes: 0 additions & 9 deletions tests/unit/test_expression_tree/test_interpolant.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,6 @@ def test_errors(self):
(np.ones(10), np.ones(12)), np.ones((10, 12)), pybamm.Symbol("a")
)

def test_warnings(self):
with self.assertWarnsRegex(Warning, "cubic spline"):
pybamm.Interpolant(
np.linspace(0, 1, 10),
np.ones(10),
pybamm.Symbol("a"),
interpolator="cubic spline",
)

def test_interpolation(self):
x = np.linspace(0, 1, 200)
y = pybamm.StateVector(slice(0, 2))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@ def test_known_solution(self):
energy = esoh_solver.theoretical_energy_integral(inputs)
self.assertAlmostEqual(sol[key], energy, places=5)

# should still work with old inputs
n_Li = parameter_values.evaluate(param.n_Li_particles_init)
inputs = {"V_min": 3, "V_max": 4.2, "n_Li": n_Li, "C_n": Q_n, "C_p": Q_p}

# Solve the model and check outputs
sol = esoh_solver.solve(inputs)
self.assertAlmostEqual(sol["Q_Li"], Q_Li, places=5)

def test_known_solution_cell_capacity(self):
param = pybamm.LithiumIonParameters()
parameter_values = pybamm.ParameterValues("Mohtat2020")
Expand Down
28 changes: 0 additions & 28 deletions tests/unit/test_parameters/test_parameters_cli.py

This file was deleted.

Loading