Skip to content

Commit

Permalink
#709 deprecate param.update_model
Browse files Browse the repository at this point in the history
  • Loading branch information
valentinsulzer committed Jan 31, 2020
1 parent f6b0ca9 commit 07d6c34
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 272 deletions.
3 changes: 2 additions & 1 deletion examples/scripts/compare_comsol/compare_comsol_DFN.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ def myinterp(t):
"Positive electrode potential [V]": comsol_phi_p,
"Terminal voltage [V]": comsol_voltage,
}
comsol_solution = pybamm.CasadiSolver(mode="fast").solve(pybamm_model, time)
# Make new solution with same t and y
comsol_solution = pybamm.Solution(pybamm_solution.t, pybamm_solution.y)
comsol_solution.model = comsol_model
# plot
plot = pybamm.QuickPlot(
Expand Down
16 changes: 11 additions & 5 deletions examples/scripts/compare_comsol/discharge_curve.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,16 @@
model = pybamm.lithium_ion.DFN()
geometry = model.default_geometry


# load parameters and process model and geometry
def current_function(t):
return pybamm.InputParameter("current")


param = model.default_parameter_values
param["Electrode width [m]"] = 1
param["Electrode height [m]"] = 1
param["Current function [A]"] = current_function
param.process_model(model)
param.process_geometry(geometry)

Expand Down Expand Up @@ -48,7 +54,7 @@
plt.ylabel(r"$\vert V - V_{comsol} \vert$", fontsize=20)

for key, C_rate in C_rates.items():

current = 24 * C_rate
# load the comsol results
comsol_variables = pickle.load(
open("input/comsol_results/comsol_{}C.pickle".format(key), "rb")
Expand All @@ -57,8 +63,6 @@
comsol_voltage = comsol_variables["voltage"]

# update current density
param["Current function [A]"] = 24 * C_rate
param.update_model(model, disc)

# discharge timescale
tau = param.process_symbol(
Expand All @@ -67,12 +71,14 @@

# solve model at comsol times
t = comsol_time / tau
solution = pybamm.CasadiSolver(mode="fast").solve(model, t)
solution = pybamm.CasadiSolver(mode="fast").solve(
model, t, inputs={"current": current}
)

# discharge capacity
discharge_capacity = solution["Discharge capacity [A.h]"]
discharge_capacity_sol = discharge_capacity(solution.t)
comsol_discharge_capacity = comsol_time * param["Current function [A]"] / 3600
comsol_discharge_capacity = comsol_time * current / 3600

# extract the voltage
voltage = solution["Terminal voltage [V]"]
Expand Down
102 changes: 20 additions & 82 deletions pybamm/parameters/parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,21 +294,14 @@ def check_and_update_parameter_values(self, values):

return values

def process_model(self, unprocessed_model, processing="process", inplace=True):
def process_model(self, unprocessed_model, inplace=True):
"""Assign parameter values to a model.
Currently inplace, could be changed to return a new model.
Parameters
----------
unprocessed_model : :class:`pybamm.BaseModel`
Model to assign parameter values for
processing : str, optional
Flag to indicate how to process model (default 'process')
* 'process': Calls :meth:`process_symbol()` (walk through the symbol \
and replace any Parameter with a Value)
* 'update': Calls :meth:`update_scalars()` for use on already-processed \
model (update the value of any Scalars in the expression tree.)
inplace: bool, optional
If True, replace the parameters in the model in place. Otherwise, return a
new model with parameter values set. Default is True.
Expand All @@ -335,32 +328,21 @@ def process_model(self, unprocessed_model, processing="process", inplace=True):
if len(unprocessed_model.rhs) == 0 and len(unprocessed_model.algebraic) == 0:
raise pybamm.ModelError("Cannot process parameters for empty model")

if processing == "process":
processing_function = self.process_symbol
elif processing == "update":
processing_function = self.update_scalars

for variable, equation in model.rhs.items():
pybamm.logger.debug(
"{} parameters for {!r} (rhs)".format(processing.capitalize(), variable)
)
model.rhs[variable] = processing_function(equation)
pybamm.logger.debug("Processing parameters for {!r} (rhs)".format(variable))
model.rhs[variable] = self.process_symbol(equation)

for variable, equation in model.algebraic.items():
pybamm.logger.debug(
"{} parameters for {!r} (algebraic)".format(
processing.capitalize(), variable
)
"Processing parameters for {!r} (algebraic)".format(variable)
)
model.algebraic[variable] = processing_function(equation)
model.algebraic[variable] = self.process_symbol(equation)

for variable, equation in model.initial_conditions.items():
pybamm.logger.debug(
"{} parameters for {!r} (initial conditions)".format(
processing.capitalize(), variable
)
"Processing parameters for {!r} (initial conditions)".format(variable)
)
model.initial_conditions[variable] = processing_function(equation)
model.initial_conditions[variable] = self.process_symbol(equation)

# Boundary conditions are dictionaries {"left": left bc, "right": right bc}
# in general, but may be imposed on the tabs (or *not* on the tab) for a
Expand All @@ -369,17 +351,15 @@ def process_model(self, unprocessed_model, processing="process", inplace=True):
new_boundary_conditions = {}
sides = ["left", "right", "negative tab", "positive tab", "no tab"]
for variable, bcs in model.boundary_conditions.items():
processed_variable = processing_function(variable)
processed_variable = self.process_symbol(variable)
new_boundary_conditions[processed_variable] = {}
for side in sides:
try:
bc, typ = bcs[side]
pybamm.logger.debug(
"{} parameters for {!r} ({} bc)".format(
processing.capitalize(), variable, side
)
"Processing parameters for {!r} ({} bc)".format(variable, side)
)
processed_bc = (processing_function(bc), typ)
processed_bc = (self.process_symbol(bc), typ)
new_boundary_conditions[processed_variable][side] = processed_bc
except KeyError as err:
# don't raise error if the key error comes from the side not being
Expand All @@ -394,42 +374,24 @@ def process_model(self, unprocessed_model, processing="process", inplace=True):

for variable, equation in model.variables.items():
pybamm.logger.debug(
"{} parameters for {!r} (variables)".format(
processing.capitalize(), variable
)
"Processing parameters for {!r} (variables)".format(variable)
)
model.variables[variable] = processing_function(equation)
model.variables[variable] = self.process_symbol(equation)
for event, equation in model.events.items():
pybamm.logger.debug(
"{} parameters for event '{}''".format(processing.capitalize(), event)
)
model.events[event] = processing_function(equation)
pybamm.logger.debug("Processing parameters for event '{}''".format(event))
model.events[event] = self.process_symbol(equation)

pybamm.logger.info("Finish setting parameters for {}".format(model.name))

return model

def update_model(self, model, disc):
"""Process a discretised model.
Currently inplace, could be changed to return a new model.
Parameters
----------
model : :class:`pybamm.BaseModel`
Model to assign parameter values for
disc : :class:`pybamm.Discretisation`
The class that was used to discretise
"""
# process parameter values for the model
self.process_model(model, processing="update")

# update discretised quantities using disc
model.concatenated_rhs = disc._concatenate_in_order(model.rhs)
model.concatenated_algebraic = disc._concatenate_in_order(model.algebraic)
model.concatenated_initial_conditions = disc._concatenate_in_order(
model.initial_conditions
).evaluate(0, None)
raise NotImplementedError(
"""
update_model functionality has been deprecated.
Use pybamm.InputParameter to quickly change a parameter value instead
"""
)

def process_geometry(self, geometry):
"""
Expand Down Expand Up @@ -561,30 +523,6 @@ def _process_symbol(self, symbol):
)
)

def update_scalars(self, symbol):
"""Update the value of any Scalars in the expression tree.
Parameters
----------
symbol : :class:`pybamm.Symbol`
Symbol or Expression tree to update
Returns
-------
symbol : :class:`pybamm.Symbol`
Symbol with Scalars updated
"""
for x in symbol.pre_order():
if isinstance(x, pybamm.Scalar):
# update any Scalar nodes if their name is in the parameter dict
if x.name in self._dict_items.keys():
x.value = self._dict_items[x.name]
# update id
x.set_id()

return symbol

def evaluate(self, symbol):
"""
Process and evaluate a symbol.
Expand Down
22 changes: 0 additions & 22 deletions tests/integration/test_models/standard_model_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,28 +103,6 @@ def test_all(
):
self.test_outputs()

def test_update_parameters(self, param):
# check if geometry has changed, throw error if so (need to re-discretise)
if any(
[
length in param.keys()
and param[length] != self.parameter_values[length]
for length in [
"Negative electrode thickness [m]",
"Separator thickness [m]",
"Positive electrode thickness [m]",
]
]
):
raise ValueError(
"geometry has changed, the orginal model needs to be re-discretised"
)
# otherwise update self.param and change the parameters in the discretised model
self.param = param
param.update_model(self.model, self.disc)
# Model should still be well-posed after processing
self.model.check_well_posedness(post_discretisation=True)


class OptimisationsTest(object):
""" Test that the optimised models give the same result as the original model. """
Expand Down
7 changes: 0 additions & 7 deletions tests/integration/test_quick_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,6 @@ def test_plot_lithium_ion(self):

quick_plot.update(0.01)

# Update parameters, solve, plot again
param.update({"Current function [A]": 0})
param.update_model(spm, disc_spm)
solution_spm = spm.default_solver.solve(spm, t_eval)
quick_plot = pybamm.QuickPlot(solution_spm)
quick_plot.plot(0)

# Test with different output variables
output_vars = [
"Negative particle surface concentration",
Expand Down
31 changes: 23 additions & 8 deletions tests/unit/test_parameters/test_parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,18 +303,18 @@ def test_process_inline_function_parameters(self):
def D(c):
return c ** 2

parameter_values = pybamm.ParameterValues({"a": 3, "Diffusivity": D})
parameter_values = pybamm.ParameterValues({"Diffusivity": D})

a = pybamm.Parameter("a")
a = pybamm.InputParameter("a")
func = pybamm.FunctionParameter("Diffusivity", a)

processed_func = parameter_values.process_symbol(func)
self.assertEqual(processed_func.evaluate(), 9)
self.assertEqual(processed_func.evaluate(u={"a": 3}), 9)

# process differentiated function parameter
diff_func = func.diff(a)
processed_diff_func = parameter_values.process_symbol(diff_func)
self.assertEqual(processed_diff_func.evaluate(), 6)
self.assertEqual(processed_diff_func.evaluate(u={"a": 3}), 6)

def test_multi_var_function_with_parameters(self):
def D(a, b):
Expand Down Expand Up @@ -362,7 +362,7 @@ def test_process_interpolant(self):
self.assertEqual(processed_diff_func.evaluate(), 2)

def test_interpolant_against_function(self):
parameter_values = pybamm.ParameterValues({"a": 0.6})
parameter_values = pybamm.ParameterValues({})
parameter_values.update(
{
"function": "[function]lico2_ocp_Dualfoil1998",
Expand All @@ -379,14 +379,16 @@ def test_interpolant_against_function(self):
check_already_exists=False,
)

a = pybamm.Parameter("a")
a = pybamm.InputParameter("a")
func = pybamm.FunctionParameter("function", a)
interp = pybamm.FunctionParameter("interpolation", a)

processed_func = parameter_values.process_symbol(func)
processed_interp = parameter_values.process_symbol(interp)
np.testing.assert_array_almost_equal(
processed_func.evaluate(), processed_interp.evaluate(), decimal=4
processed_func.evaluate(u={"a": 0.6}),
processed_interp.evaluate(u={"a": 0.6}),
decimal=4,
)

# process differentiated function parameter
Expand All @@ -395,7 +397,9 @@ def test_interpolant_against_function(self):
processed_diff_func = parameter_values.process_symbol(diff_func)
processed_diff_interp = parameter_values.process_symbol(diff_interp)
np.testing.assert_array_almost_equal(
processed_diff_func.evaluate(), processed_diff_interp.evaluate(), decimal=2
processed_diff_func.evaluate(u={"a": 0.6}),
processed_diff_interp.evaluate(u={"a": 0.6}),
decimal=2,
)

def test_process_complex_expression(self):
Expand Down Expand Up @@ -500,6 +504,17 @@ def test_process_model(self):
with self.assertRaises(KeyError):
parameter_values.process_model(model)

def test_inplace(self):
model = pybamm.lithium_ion.SPM()
param = model.default_parameter_values
new_model = param.process_model(model, inplace=False)

for val in list(model.rhs.values()):
self.assertTrue(val.has_symbol_of_classes(pybamm.Parameter))

for val in list(new_model.rhs.values()):
self.assertFalse(val.has_symbol_of_classes(pybamm.Parameter))

def test_process_empty_model(self):
model = pybamm.BaseModel()
parameter_values = pybamm.ParameterValues({"a": 1, "b": 2, "c": 3, "d": 42})
Expand Down
Loading

0 comments on commit 07d6c34

Please sign in to comment.