diff --git a/pybamm/models/base_model.py b/pybamm/models/base_model.py index 26bf77c613..9859364161 100644 --- a/pybamm/models/base_model.py +++ b/pybamm/models/base_model.py @@ -348,6 +348,10 @@ def _find_symbols(self, typ): def __getitem__(self, key): return self.rhs[key] + @property + def default_quick_plot_variables(self): + return None + def new_empty_copy(self): """ Create an empty copy of the model with the same name and "parameters" diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py index ca379b3611..99aa87de2c 100644 --- a/pybamm/models/full_battery_models/base_battery_model.py +++ b/pybamm/models/full_battery_models/base_battery_model.py @@ -400,12 +400,6 @@ def __init__(self, options=None, name="Unnamed battery model"): self._built = False self._built_fundamental_and_external = False - @property - def default_parameter_values(self): - # Default parameter values - # Lion parameters left as default parameter set for tests - return pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Marquis2019) - @property def default_geometry(self): return pybamm.battery_geometry( diff --git a/pybamm/models/full_battery_models/lead_acid/base_lead_acid_model.py b/pybamm/models/full_battery_models/lead_acid/base_lead_acid_model.py index 5be38366df..cb693a7d67 100644 --- a/pybamm/models/full_battery_models/lead_acid/base_lead_acid_model.py +++ b/pybamm/models/full_battery_models/lead_acid/base_lead_acid_model.py @@ -54,6 +54,17 @@ def default_var_pts(self): var = pybamm.standard_spatial_vars return {var.x_n: 25, var.x_s: 41, var.x_p: 34, var.y: 10, var.z: 10} + @property + def default_quick_plot_variables(self): + return [ + "Interfacial current density [A.m-2]", + "Electrolyte concentration [mol.m-3]", + "Current [A]", + "Porosity", + "Electrolyte potential [V]", + "Terminal voltage [V]", + ] + def set_soc_variables(self): """Set variables relating to the state of charge.""" # State of Charge defined as function of dimensionless electrolyte concentration diff --git a/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py b/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py index e491003a95..21cafbd817 100644 --- a/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py +++ b/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py @@ -44,6 +44,36 @@ def __init__(self, options=None, name="Unnamed lithium-ion model", build=False): ) self.set_standard_output_variables() + @property + def default_parameter_values(self): + if self.half_cell: + return pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) + else: + return pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Marquis2019) + + @property + def default_quick_plot_variables(self): + if self.half_cell: + return [ + "Electrolyte concentration [mol.m-3]", + "Positive particle surface concentration [mol.m-3]", + "Current [A]", + "Electrolyte potential [V]", + "Positive electrode potential [V]", + "Terminal voltage [V]", + ] + else: + return [ + "Negative particle surface concentration [mol.m-3]", + "Electrolyte concentration [mol.m-3]", + "Positive particle surface concentration [mol.m-3]", + "Current [A]", + "Negative electrode potential [V]", + "Electrolyte potential [V]", + "Positive electrode potential [V]", + "Terminal voltage [V]", + ] + def set_standard_output_variables(self): super().set_standard_output_variables() diff --git a/pybamm/plotting/quick_plot.py b/pybamm/plotting/quick_plot.py index f297082a5a..cabef416d3 100644 --- a/pybamm/plotting/quick_plot.py +++ b/pybamm/plotting/quick_plot.py @@ -143,26 +143,12 @@ def __init__( # Default output variables for lead-acid and lithium-ion if output_variables is None: - if isinstance(models[0], pybamm.lithium_ion.BaseModel): - output_variables = [ - "Negative particle surface concentration [mol.m-3]", - "Electrolyte concentration [mol.m-3]", - "Positive particle surface concentration [mol.m-3]", - "Current [A]", - "Negative electrode potential [V]", - "Electrolyte potential [V]", - "Positive electrode potential [V]", - "Terminal voltage [V]", - ] - elif isinstance(models[0], pybamm.lead_acid.BaseModel): - output_variables = [ - "Interfacial current density [A.m-2]", - "Electrolyte concentration [mol.m-3]", - "Current [A]", - "Porosity", - "Electrolyte potential [V]", - "Terminal voltage [V]", - ] + output_variables = models[0].default_quick_plot_variables + # raise error if still None + if output_variables is None: + raise ValueError( + f"No default output variables provided for {models[0].name}" + ) self.n_rows = n_rows or int( len(output_variables) // np.sqrt(len(output_variables)) diff --git a/pybamm/solvers/base_solver.py b/pybamm/solvers/base_solver.py index bdb118c9f5..ca2b43dc1a 100644 --- a/pybamm/solvers/base_solver.py +++ b/pybamm/solvers/base_solver.py @@ -50,9 +50,9 @@ def __init__( extrap_tol=0, max_steps="deprecated", ): - self._method = method - self._rtol = rtol - self._atol = atol + self.method = method + self.rtol = rtol + self.atol = atol self.root_tol = root_tol self.root_method = root_method self.extrap_tol = extrap_tol @@ -68,30 +68,6 @@ def __init__( self.ode_solver = False self.algebraic_solver = False - @property - def method(self): - return self._method - - @method.setter - def method(self, value): - self._method = value - - @property - def rtol(self): - return self._rtol - - @rtol.setter - def rtol(self, value): - self._rtol = value - - @property - def atol(self): - return self._atol - - @atol.setter - def atol(self, value): - self._atol = value - @property def root_method(self): return self._root_method @@ -112,14 +88,6 @@ def root_method(self, method): raise pybamm.SolverError("Root method must be an algebraic solver") self._root_method = method - @property - def root_tol(self): - return self._root_tol - - @root_tol.setter - def root_tol(self, tol): - self._root_tol = tol - def copy(self): """Returns a copy of the solver""" new_solver = copy.copy(self) diff --git a/pybamm/solvers/idaklu_solver.py b/pybamm/solvers/idaklu_solver.py index d18213f5b7..e7c2041752 100644 --- a/pybamm/solvers/idaklu_solver.py +++ b/pybamm/solvers/idaklu_solver.py @@ -86,7 +86,7 @@ def set_atol_by_variable(self, variables_with_tols, model): """ size = model.concatenated_initial_conditions.size - atol = self._check_atol_type(self._atol, size) + atol = self._check_atol_type(self.atol, size) for var, tol in variables_with_tols.items(): variable = model.variables[var] if isinstance(variable, pybamm.StateVector): @@ -99,7 +99,7 @@ def set_atol_by_variable(self, variables_with_tols, model): def set_state_vec_tol(self, atol, state_vec, tol): """ A method to set the tolerances in the atol vector of a specific - state variable. This method modifies self._atol + state variable. This method modifies self.atol Parameters ---------- @@ -140,7 +140,9 @@ def _check_atol_type(self, atol, size): if atol.size != size: raise pybamm.SolverError( """Absolute tolerances must be either a scalar or a numpy arrray - of the same shape as y0 ({})""".format(size) + of the same shape as y0 ({})""".format( + size + ) ) return atol @@ -178,13 +180,13 @@ def _integrate(self, model, t_eval, inputs_dict=None): try: atol = model.atol except AttributeError: - atol = self._atol + atol = self.atol y0 = model.y0 if isinstance(y0, casadi.DM): y0 = y0.full().flatten() - rtol = self._rtol + rtol = self.rtol atol = self._check_atol_type(atol, y0.size) if model.convert_to_format == "jax": diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py index 8380e82c5c..647c106987 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py @@ -11,8 +11,7 @@ class TestDFN(unittest.TestCase): def test_basic_processing(self): options = {"working electrode": "positive"} model = pybamm.lithium_ion.DFN(options) - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) - modeltest = tests.StandardModelTest(model, parameter_values=param) + modeltest = tests.StandardModelTest(model) modeltest.test_all(skip_output_tests=True) @@ -20,29 +19,25 @@ class TestDFNWithSEI(unittest.TestCase): def test_well_posed_constant(self): options = {"working electrode": "positive", "SEI": "constant"} model = pybamm.lithium_ion.DFN(options) - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) - modeltest = tests.StandardModelTest(model, parameter_values=param) + modeltest = tests.StandardModelTest(model) modeltest.test_all(skip_output_tests=True) def test_well_posed_reaction_limited(self): options = {"working electrode": "positive", "SEI": "reaction limited"} model = pybamm.lithium_ion.DFN(options) - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) - modeltest = tests.StandardModelTest(model, parameter_values=param) + modeltest = tests.StandardModelTest(model) modeltest.test_all(skip_output_tests=True) def test_well_posed_solvent_diffusion_limited(self): options = {"working electrode": "positive", "SEI": "solvent-diffusion limited"} model = pybamm.lithium_ion.DFN(options) - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) - modeltest = tests.StandardModelTest(model, parameter_values=param) + modeltest = tests.StandardModelTest(model) modeltest.test_all(skip_output_tests=True) def test_well_posed_electron_migration_limited(self): options = {"working electrode": "positive", "SEI": "electron-migration limited"} model = pybamm.lithium_ion.DFN(options) - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) - modeltest = tests.StandardModelTest(model, parameter_values=param) + modeltest = tests.StandardModelTest(model) modeltest.test_all(skip_output_tests=True) def test_well_posed_interstitial_diffusion_limited(self): @@ -51,8 +46,7 @@ def test_well_posed_interstitial_diffusion_limited(self): "SEI": "interstitial-diffusion limited", } model = pybamm.lithium_ion.DFN(options) - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) - modeltest = tests.StandardModelTest(model, parameter_values=param) + modeltest = tests.StandardModelTest(model) modeltest.test_all(skip_output_tests=True) def test_well_posed_ec_reaction_limited(self): @@ -61,8 +55,7 @@ def test_well_posed_ec_reaction_limited(self): "SEI": "ec reaction limited", } model = pybamm.lithium_ion.DFN(options) - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Xu2019) - modeltest = tests.StandardModelTest(model, parameter_values=param) + modeltest = tests.StandardModelTest(model) modeltest.test_all(skip_output_tests=True) diff --git a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py index 58e36383a2..be19240854 100644 --- a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py +++ b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py @@ -5,7 +5,6 @@ import pybamm import unittest import io -import os from contextlib import redirect_stdout OPTIONS_DICT = { @@ -291,23 +290,6 @@ def test_default_solver(self): model.algebraic = {a: a - 1} self.assertIsInstance(model.default_solver, pybamm.CasadiAlgebraicSolver) - def test_default_parameters(self): - # check parameters are read in ok - model = pybamm.BaseBatteryModel() - self.assertEqual( - model.default_parameter_values["Reference temperature [K]"], 298.15 - ) - - # change path and try again - - cwd = os.getcwd() - os.chdir("..") - model = pybamm.BaseBatteryModel() - self.assertEqual( - model.default_parameter_values["Reference temperature [K]"], 298.15 - ) - os.chdir(cwd) - def test_timescale(self): model = pybamm.BaseModel() self.assertEqual(model.timescale.evaluate(), 1) diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_base_lithium_ion_model.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_base_lithium_ion_model.py index 05860b0822..a2360ee66c 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_base_lithium_ion_model.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_base_lithium_ion_model.py @@ -3,6 +3,7 @@ # import pybamm import unittest +import os class TestBaseLithiumIonModel(unittest.TestCase): @@ -14,6 +15,23 @@ def test_incompatible_options(self): {"cell geometry": "arbitrary", "thermal": "x-lumped"} ) + def test_default_parameters(self): + # check parameters are read in ok + model = pybamm.lithium_ion.BaseModel() + self.assertEqual( + model.default_parameter_values["Reference temperature [K]"], 298.15 + ) + + # change path and try again + + cwd = os.getcwd() + os.chdir("..") + model = pybamm.lithium_ion.BaseModel() + self.assertEqual( + model.default_parameter_values["Reference temperature [K]"], 298.15 + ) + os.chdir(cwd) + if __name__ == "__main__": print("Add -v for more debug output") diff --git a/tests/unit/test_plotting/test_quick_plot.py b/tests/unit/test_plotting/test_quick_plot.py index a7cbc3e326..aca43c6e68 100644 --- a/tests/unit/test_plotting/test_quick_plot.py +++ b/tests/unit/test_plotting/test_quick_plot.py @@ -263,6 +263,15 @@ def test_simple_ode_model(self): pybamm.close_plots() + def test_plot_with_different_models(self): + model = pybamm.BaseModel() + a = pybamm.Variable("a") + model.rhs = {a: pybamm.Scalar(0)} + model.initial_conditions = {a: pybamm.Scalar(0)} + solution = pybamm.CasadiSolver("fast").solve(model, [0, 1]) + with self.assertRaisesRegex(ValueError, "No default output variables"): + pybamm.QuickPlot(solution) + def test_spm_simulation(self): # SPM model = pybamm.lithium_ion.SPM() @@ -292,7 +301,11 @@ def test_spm_simulation(self): def test_loqs_spme(self): t_eval = np.linspace(0, 10, 2) - for model in [pybamm.lithium_ion.SPMe(), pybamm.lead_acid.LOQS()]: + for model in [ + pybamm.lithium_ion.SPMe(), + pybamm.lead_acid.LOQS(), + pybamm.lithium_ion.DFN({"working electrode": "positive"}), + ]: geometry = model.default_geometry param = model.default_parameter_values param.process_model(model) diff --git a/tests/unit/test_solvers/test_processed_variable.py b/tests/unit/test_solvers/test_processed_variable.py index 87cb715dfc..b6814668d6 100644 --- a/tests/unit/test_solvers/test_processed_variable.py +++ b/tests/unit/test_solvers/test_processed_variable.py @@ -113,12 +113,12 @@ def test_processed_variable_0D_no_sensitivity(self): # with parameter t = pybamm.t y = pybamm.StateVector(slice(0, 1)) - a = pybamm.InputParameter('a') + a = pybamm.InputParameter("a") var = t * y * a var.mesh = None t_sol = np.linspace(0, 1) y_sol = np.array([np.linspace(0, 5)]) - inputs = {'a': np.array([1.0])} + inputs = {"a": np.array([1.0])} var_casadi = to_casadi(var, y_sol, inputs=inputs) processed_var = pybamm.ProcessedVariable( [var], @@ -128,7 +128,7 @@ def test_processed_variable_0D_no_sensitivity(self): ) # test no sensitivity raises error - with self.assertRaisesRegex(ValueError, 'Cannot compute sensitivities'): + with self.assertRaisesRegex(ValueError, "Cannot compute sensitivities"): print(processed_var.sensitivities) def test_processed_variable_1D(self): @@ -551,8 +551,7 @@ def test_processed_var_1D_interpolation(self): # On size domain R_n = pybamm.Matrix( - disc.mesh["negative particle size"].nodes, - domain="negative particle size" + disc.mesh["negative particle size"].nodes, domain="negative particle size" ) R_n.mesh = disc.mesh["negative particle size"] R_n_casadi = to_casadi(R_n, y_sol) @@ -851,57 +850,6 @@ def test_processed_var_2D_fixed_t_scikit_interpolation(self): # 2 scalars np.testing.assert_array_equal(processed_var(t=None, y=0.2, z=0.2).shape, ()) - def test_processed_variable_ode_pde_solution(self): - # without space - model = pybamm.BaseBatteryModel() - c = pybamm.Variable("conc") - model.rhs = {c: -c} - model.initial_conditions = {c: 1} - model.variables = {"c": c} - modeltest = tests.StandardModelTest(model) - modeltest.test_all() - sol = modeltest.solution - np.testing.assert_array_almost_equal(sol["c"](sol.t), np.exp(-sol.t)) - - # with space - # set up and solve model - whole_cell = ["negative electrode", "separator", "positive electrode"] - model = pybamm.BaseBatteryModel() - model.length_scales = { - "negative electrode": pybamm.Scalar(1), - "separator": pybamm.Scalar(1), - "positive electrode": pybamm.Scalar(1), - } - c = pybamm.Variable("conc", domain=whole_cell) - c_s = pybamm.Variable( - "particle conc", - domain="negative particle", - auxiliary_domains={"secondary": ["negative electrode"]}, - ) - model.rhs = {c: -c, c_s: 1 - c_s} - model.initial_conditions = {c: 1, c_s: 0.5} - model.boundary_conditions = { - c: {"left": (0, "Neumann"), "right": (0, "Neumann")}, - c_s: {"left": (0, "Neumann"), "right": (0, "Neumann")}, - } - model.variables = { - "c": c, - "N": pybamm.grad(c), - "c_s": c_s, - "N_s": pybamm.grad(c_s), - } - modeltest = tests.StandardModelTest(model) - modeltest.test_all() - # set up testing - sol = modeltest.solution - x = pybamm.SpatialVariable("x", domain=whole_cell) - x_sol = modeltest.disc.process_symbol(x).entries[:, 0] - - # test - np.testing.assert_array_almost_equal( - sol["c"](sol.t, x_sol), np.ones_like(x_sol)[:, np.newaxis] * np.exp(-sol.t) - ) - def test_call_failure(self): # x domain var = pybamm.Variable("var x", domain=["negative electrode", "separator"])