Generalise linear solvers #648
Conversation
tommbendall
added
enhancement
…set to 0 with a flag
tommbendall
left a comment
tommbendall
left a comment
There was a problem hiding this comment.
Main thoughts:
- we need comments in
solver_presets.pyto explain what the dictionaries are doing - I would ideally prefer the if statements in the
HybridisedSolverParametersto not depend on thetypeofequation_set. I think we want to group them into (a) two variables, (b) equations with tracers in which we ignore some variables, (c) equations with elimination of thermodynamic variable (either with 3D transport or vertical transport). Maybe this is better done as a follow-up task?
Co-authored-by: Thomas Bendall <14180399+tommbendall@users.noreply.github.com>
…roject/gusto into auxiliary-equation-pc
gusto/solvers/preconditioners.py
Outdated
| self.W = equations.function_space | ||
| self.W_hyb = MixedFunctionSpace((self.Vu_broken, self.Vrho, self.Vtrace)) | ||
|
|
||
| # Define |
There was a problem hiding this comment.
do we need more detail here? define what?
| def update(self, pc): | ||
| PETSc.Sys.Print("[PC.update] Updating hybridized PC") | ||
|
|
||
| if hasattr(self, "rho_avg_solver"): |
There was a problem hiding this comment.
when would it not have a rho_avg_solver?
There was a problem hiding this comment.
This is part of the current hack - update is called before init the first time, so we need to make sure the solvers have been set up
|
|
||
| self.alpha = appctx.get('alpha') | ||
| tau_values = appctx.get('tau_values') | ||
| self.tau_values = tau_values if tau_values is not None else {} |
There was a problem hiding this comment.
is this the right default for the tau values? how does this interact with alpha?
There was a problem hiding this comment.
Yes - above the dttau is selected based on whether tau is defined, if not it takes dtalpha
| """ | ||
| This module provides PETSc dictionaries for nonlinear and linear problems. | ||
| The solvers provided here are used for solving linear problems on mixed |
There was a problem hiding this comment.
I"m a little bit confused by these two sentences - the first mentions nonlinear and linear problems but the second just says this is for linear problems...
There was a problem hiding this comment.
I was future proofing for nonlinear solvers in the future, but have left as linear for now
| # BCs, Forcing and Linear Solver --------------------------------------- | ||
| self.bcs = equation_set.bcs | ||
| self.forcing = Forcing(equation_set, self.implicit_terms, self.alpha) | ||
| if linear_solver is None: |
There was a problem hiding this comment.
I don't think we want to keep this argument
| passed in. Defaults to False. | ||
| reference_dependent: this indicates that the solver Jacobian should | ||
| be rebuilt if the reference profiles have been updated. | ||
| appctx: appctx for the underlying :class:`LinearVariationalSolver`. |
There was a problem hiding this comment.
more doc changes required above to remove obsolete arguments and put docs in same order as args
| self.equation.update_reference_profiles() | ||
| self.solver.invalidate_jacobian() | ||
|
|
||
| # TODO: Issue X is to address this reference profile update bug (pythonPC update not called) |
There was a problem hiding this comment.
do we have an issue number?
| for field_name in field_names: | ||
|
|
||
| if field_name not in prognostic_names: | ||
| logger.debug(f'Semi-Implicit Quasi Newton: Zeroing xrhs for {field_name}') |
Co-authored-by: Dr Jemma Shipton <j.shipton@exeter.ac.uk>
Co-authored-by: Dr Jemma Shipton <j.shipton@exeter.ac.uk>
Co-authored-by: Dr Jemma Shipton <j.shipton@exeter.ac.uk>
Co-authored-by: Dr Jemma Shipton <j.shipton@exeter.ac.uk>
…roject/gusto into auxiliary-equation-pc
Co-authored-by: Dr Jemma Shipton <j.shipton@exeter.ac.uk>
No description provided.