From 0cf71adf767d4046b32791efbf748747b0233da0 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 30 Nov 2022 13:07:30 -0500 Subject: [PATCH] #2418 better conditioning for algebraic equations --- examples/scripts/compare_lithium_ion.py | 4 +- .../submodels/electrode/ohm/full_ohm.py | 2 +- .../full_conductivity.py | 2 +- pybamm/solvers/algebraic_solver.py | 166 ++++++++++-------- pybamm/solvers/casadi_algebraic_solver.py | 14 +- pybamm/solvers/casadi_solver.py | 2 +- tests/unit/test_solvers/test_base_solver.py | 6 +- 7 files changed, 115 insertions(+), 81 deletions(-) diff --git a/examples/scripts/compare_lithium_ion.py b/examples/scripts/compare_lithium_ion.py index cec78cb50a..140fc2f9ca 100644 --- a/examples/scripts/compare_lithium_ion.py +++ b/examples/scripts/compare_lithium_ion.py @@ -3,12 +3,12 @@ # import pybamm -pybamm.set_logging_level("SPAM") +pybamm.set_logging_level("INFO") # load models models = [ # pybamm.lithium_ion.SPM(), # pybamm.lithium_ion.SPMe(), - pybamm.lithium_ion.DFN() # {"dimensionality": 1}), + pybamm.lithium_ion.DFN({"dimensionality": 2}), # pybamm.lithium_ion.DFN( # {"particle": "uniform profile"} # ), diff --git a/pybamm/models/submodels/electrode/ohm/full_ohm.py b/pybamm/models/submodels/electrode/ohm/full_ohm.py index afffa4fa40..241c47d80d 100644 --- a/pybamm/models/submodels/electrode/ohm/full_ohm.py +++ b/pybamm/models/submodels/electrode/ohm/full_ohm.py @@ -75,7 +75,7 @@ def set_algebraic(self, variables): "interfacial current densities [A.m-3]" ] - self.algebraic[phi_s] = pybamm.div(i_s) + sum_a_j + self.algebraic[phi_s] = self.param.L_x**2 * (pybamm.div(i_s) + sum_a_j) def set_boundary_conditions(self, variables): Domain = self.domain.capitalize() diff --git a/pybamm/models/submodels/electrolyte_conductivity/full_conductivity.py b/pybamm/models/submodels/electrolyte_conductivity/full_conductivity.py index a76f24f1ee..3883f6c1d1 100644 --- a/pybamm/models/submodels/electrolyte_conductivity/full_conductivity.py +++ b/pybamm/models/submodels/electrolyte_conductivity/full_conductivity.py @@ -76,7 +76,7 @@ def set_algebraic(self, variables): # Override print_name sum_a_j.print_name = "aj" - self.algebraic = {phi_e: pybamm.div(i_e) - sum_a_j} + self.algebraic = {phi_e: self.param.L_x**2 * (pybamm.div(i_e) - sum_a_j)} def set_initial_conditions(self, variables): phi_e = variables["Electrolyte potential [V]"] diff --git a/pybamm/solvers/algebraic_solver.py b/pybamm/solvers/algebraic_solver.py index 870470bc6d..30e8a79bdc 100644 --- a/pybamm/solvers/algebraic_solver.py +++ b/pybamm/solvers/algebraic_solver.py @@ -135,82 +135,100 @@ def jac_fn(y_alg): else: jac_fn = None - # Methods which use least-squares are specified as either "lsq", - # which uses the default method, or with "lsq__methodname" - if self.method.startswith("lsq"): - - if self.method == "lsq": - method = "trf" - else: - method = self.method[5:] - if jac_fn is None: - jac_fn = "2-point" - timer.reset() - sol = optimize.least_squares( - root_fun, - y0_alg, - method=method, - ftol=self.tol, - jac=jac_fn, - bounds=model.bounds, - **self.extra_options, - ) - integration_time += timer.time() - # Methods which use minimize are specified as either "minimize", - # which uses the default method, or with "minimize__methodname" - elif self.method.startswith("minimize"): - # Adapt the root function for minimize - def root_norm(y): - return np.sum(root_fun(y) ** 2) - - if jac_fn is None: - jac_norm = None + itr = 0 + maxiter = 2 + success = False + while not success: + # Methods which use least-squares are specified as either "lsq", + # which uses the default method, or with "lsq__methodname" + if self.method.startswith("lsq"): + + if self.method == "lsq": + method = "trf" + else: + method = self.method[5:] + if jac_fn is None: + jac_fn = "2-point" + timer.reset() + sol = optimize.least_squares( + root_fun, + y0_alg, + method=method, + ftol=self.tol, + jac=jac_fn, + bounds=model.bounds, + **self.extra_options, + ) + integration_time += timer.time() + # Methods which use minimize are specified as either "minimize", + # which uses the default method, or with "minimize__methodname" + elif self.method.startswith("minimize"): + # Adapt the root function for minimize + def root_norm(y): + return np.sum(root_fun(y) ** 2) + + if jac_fn is None: + jac_norm = None + else: + + def jac_norm(y): + return np.sum(2 * root_fun(y) * jac_fn(y), 0) + + if self.method == "minimize": + method = None + else: + method = self.method[10:] + extra_options = self.extra_options + if np.any(model.bounds[0] != -np.inf) or np.any( + model.bounds[1] != np.inf + ): + bounds = [ + (lb, ub) for lb, ub in zip(model.bounds[0], model.bounds[1]) + ] + extra_options["bounds"] = bounds + timer.reset() + sol = optimize.minimize( + root_norm, + y0_alg, + method=method, + tol=self.tol, + jac=jac_norm, + **extra_options, + ) + integration_time += timer.time() else: - - def jac_norm(y): - return np.sum(2 * root_fun(y) * jac_fn(y), 0) - - if self.method == "minimize": - method = None + timer.reset() + sol = optimize.root( + root_fun, + y0_alg, + method=self.method, + tol=self.tol, + jac=jac_fn, + options=self.extra_options, + ) + integration_time += timer.time() + + if sol.success and np.all(abs(sol.fun) < self.tol): + # update initial guess for the next iteration + y0_alg = sol.x + # update solution array + y_alg[:, idx] = y0_alg + success = True + elif not sol.success: + raise pybamm.SolverError( + "Could not find acceptable solution: {}".format(sol.message) + ) else: - method = self.method[10:] - extra_options = self.extra_options - if np.any(model.bounds[0] != -np.inf) or np.any( - model.bounds[1] != np.inf - ): - bounds = [ - (lb, ub) for lb, ub in zip(model.bounds[0], model.bounds[1]) - ] - extra_options["bounds"] = bounds - timer.reset() - sol = optimize.minimize( - root_norm, - y0_alg, - method=method, - tol=self.tol, - jac=jac_norm, - **extra_options, - ) - integration_time += timer.time() - else: - timer.reset() - sol = optimize.root( - root_fun, - y0_alg, - method=self.method, - tol=self.tol, - jac=jac_fn, - options=self.extra_options, - ) - integration_time += timer.time() - - if sol.success: - # update solution array - y_alg[:, idx] = sol.x - else: - raise pybamm.SolverError( - "Could not find acceptable solution: {}".format(sol.message) - ) + y0_alg = sol.x + if itr > maxiter: + raise pybamm.SolverError( + "Could not find acceptable solution: solver terminated " + "successfully, but maximum solution error " + "({}) above tolerance ({})".format( + np.max(abs(sol.fun)), self.tol + ) + ) + itr += 1 # Concatenate differential part y_diff = np.r_[[y0_diff] * len(t_eval)].T diff --git a/pybamm/solvers/casadi_algebraic_solver.py b/pybamm/solvers/casadi_algebraic_solver.py index 53076ae67c..f726adc5da 100644 --- a/pybamm/solvers/casadi_algebraic_solver.py +++ b/pybamm/solvers/casadi_algebraic_solver.py @@ -128,7 +128,9 @@ def _integrate(self, model, t_eval, inputs_dict=None): # If there are no symbolic inputs, check the function is below the tol # Skip this check if there are symbolic inputs - if success and (not any(np.isnan(fun))): + if success and ( + (not any(np.isnan(fun)) and np.all(casadi.fabs(fun) < self.tol)) + ): # update initial guess for the next iteration y0_alg = y_alg_sol y0 = casadi.vertcat(y0_diff, y0_alg) @@ -145,6 +147,16 @@ def _integrate(self, model, t_eval, inputs_dict=None): raise pybamm.SolverError( "Could not find acceptable solution: solver returned NaNs" ) + else: + raise pybamm.SolverError( + """ + Could not find acceptable solution: solver terminated + successfully, but maximum solution error ({}) + above tolerance ({}) + """.format( + casadi.mmax(casadi.fabs(fun)), self.tol + ) + ) # Concatenate differential part y_diff = casadi.horzcat(*[y0_diff] * len(t_eval)) diff --git a/pybamm/solvers/casadi_solver.py b/pybamm/solvers/casadi_solver.py index d9ef958fa2..665c709fac 100644 --- a/pybamm/solvers/casadi_solver.py +++ b/pybamm/solvers/casadi_solver.py @@ -76,7 +76,7 @@ def __init__( mode="safe", rtol=1e-6, atol=1e-6, - root_method="lm", + root_method="casadi", root_tol=1e-6, max_step_decrease_count=5, dt_max=None, diff --git a/tests/unit/test_solvers/test_base_solver.py b/tests/unit/test_solvers/test_base_solver.py index 39b0bec4f3..4acf0434d4 100644 --- a/tests/unit/test_solvers/test_base_solver.py +++ b/tests/unit/test_solvers/test_base_solver.py @@ -203,7 +203,6 @@ def __init__(self): ) self.convert_to_format = "casadi" self.bounds = (np.array([-np.inf]), np.array([np.inf])) - self.events = [] def rhs_eval(self, t, y, inputs): return np.array([]) @@ -219,6 +218,11 @@ def algebraic_eval(self, t, y, inputs): "Could not find acceptable solution: The iteration is not making", ): solver.calculate_consistent_state(Model()) + solver = pybamm.BaseSolver(root_method="lm") + with self.assertRaisesRegex( + pybamm.SolverError, "Could not find acceptable solution: solver terminated" + ): + solver.calculate_consistent_state(Model()) # with casadi solver = pybamm.BaseSolver(root_method="casadi") with self.assertRaisesRegex(