Conversation
pbrubeck
force-pushed
the
pbrubeck/goal-adaptive-solver
branch
from
daf58a1 to
b28fbf5
Compare
pbrubeck
force-pushed
the
pbrubeck/goal-adaptive-solver
branch
from
f59bd8e to
b920833
Compare
pbrubeck
force-pushed
the
pbrubeck/goal-adaptive-solver
branch
from
645af70 to
37ab7d9
Compare
pbrubeck
force-pushed
the
pbrubeck/goal-adaptive-solver
branch
from
b3dcbb2 to
befe0ff
Compare
pbrubeck
force-pushed
the
pbrubeck/goal-adaptive-solver
branch
from
befe0ff to
b241c80
Compare
…roject/firedrake into pbrubeck/goal-adaptive-solver
pbrubeck
changed the title
Co-authored-by: Pablo Brubeck <brubeck@protonmail.com>
connorjward
left a comment
connorjward
left a comment
There was a problem hiding this comment.
This feels a little like we're reinventing a new interface to nonlinear solvers when we already have one: SNES. Logging things is done via monitors and passing variational forms can be done with the appctx.
@JHopeCollins is the expert here, but don't you think that this might be a use case for a Python SNES? I worry that the current approach isn't as composable as other parts of Firedrake.
| def __init__(self, base_mesh: MeshGeometry, nested: bool = True): | ||
| self.meshes = [] | ||
| self._meshes = [] | ||
| self._meshes = self.meshes |
There was a problem hiding this comment.
This just creates a reference to the same object
|
|
||
| @singledispatch | ||
| def refine(expr, self, coefficient_mapping=None): | ||
| return coarsen(expr, self, coefficient_mapping=coefficient_mapping) # fallback to original |
There was a problem hiding this comment.
This is very weird. Aren't they supposed to do the opposite things?
| __all__ = ["GoalAdaptiveNonlinearVariationalSolver"] | ||
|
|
||
|
|
||
| class GoalAdaptiveOptions: |
There was a problem hiding this comment.
Better as a frozen dataclass, much less boilerplate
| * ``False`` (default) – never write. | ||
| * ``True`` – write at every iteration. | ||
| * A positive integer ``k`` – write every ``k`` iterations. | ||
| verbose |
There was a problem hiding this comment.
I feel like all of these output options are usually set via PETSc options?
| problem: NonlinearVariationalProblem, | ||
| goal_functional: ufl.BaseForm, | ||
| tolerance: float, | ||
| goal_adaptive_options: dict | None = None, |
There was a problem hiding this comment.
| goal_adaptive_options: dict | None = None, | |
| goal_adaptive_options: GoalAdaptiveOptions | dict | None = None, |
|
|
||
| def compute_error_indicators(self, u_err, z_lo, z_err): | ||
| """Compute cell and facet residuals R_cell, R_facet""" | ||
| from firedrake.assemble import assemble |
There was a problem hiding this comment.
Does this import need to be inside?
|
|
||
| def print(self, *args, **kwargs): | ||
| if self.options.verbose: | ||
| PETSc.Sys.Print(*args, **kwargs) |
There was a problem hiding this comment.
Doesn't this need a comm?
| } | ||
|
|
||
|
|
||
| class GoalAdaptiveNonlinearVariationalSolver: |
There was a problem hiding this comment.
It feels to me like this should inherit from some sort of AbstractNonlinearVariationalSolver type, as it implicitly shares an interface with our other solver classes.
| else: | ||
| raise ValueError(f"Unrecognised dual_low_method {self.options.dual_low_method}") | ||
| z_err = z - z_lo | ||
| self.z = z |
There was a problem hiding this comment.
It can be dangerous to mutate attributes inside methods like this because it makes errors hard to track down. I wonder if we should make solve_dual into _solve_dual (i.e. make it private) because it only makes sense to call inside solve. The same for many other methods in this class.
| if self == coarsen: | ||
| V_new._fine = V | ||
| V._coarse = V_new | ||
| elif self == refine: | ||
| V_new._coarse = V | ||
| V._fine = V_new |
There was a problem hiding this comment.
I think that a function called coarsen_function_space that does this is going to cause a huge amount of confusion.
|
No, I don't think this is a job for a SNES. A SNES inherently works on a fixed finite-dimensional space: the size of the vectors involved is fixed. An adaptive loop is something that changes the space being used to approximate the PDE. The adaptive loop (solve -> estimate -> mark -> refine) must happen outside the solver loop. We use SNES on the inside, but SNES can't do what this class does. |
|
I see. I still wonder if there are ways that this work could be better integrated into the rest of Firedrake and made to be as composable as possible - I could be wrong though. @pbrubeck can you bring this to the next Firedrake meeting for discussion? |
|
For example we might want to have |
3 participants
Description
Implements a new
GoalAdaptiveNonlinearVariationalSolverclass to do adaptive refinement on aNonlinearVariationalProblemdefined on a coarse (netgen) mesh. The adaptive procedure relies on a user-specified functional of interest, refered to as goal functional. The solver will iteratively solve -> estimate -> mark -> refine until an automatic error estimate to the goal functional falls below a user specified tolerance.