On Wed, Jul 24, 2013 at 3:24 PM, Jed Brown <[email protected]> wrote:
Dmitry Karpeyev <[email protected]> writes:
It appears that SNESConvergedDefault() sets snes->ttol here
https://bitbucket.org/petsc/petsc/src/40bc6eb114e6a8e7f27fd216b5c1226efddf33...
That should be harmless, albeit unnecessary, since snes->ttol is also set at the beginning of the solve in SNESSolve_NEWTONLS() (and other SNESSolve_XXX) right before calling the convergence test:
https://bitbucket.org/petsc/petsc/src/40bc6eb114e6a8e7f27fd216b5c1226efddf33...
However, this might make for some fragile code and may confuse the user modeling a custom convergence test on SNESConvergedDefault(). In particular, a local reading of SNESConvergedDefault() creates the impression that it is the convergence monitor that is responsible for setting snes->ttol on the zeroth iteration -- snes->ttol is used outside
of
SNES, for example, in one of the variants of the EW code (maybe only there). That would require access to SNES internals by user code.
Well, when using SNESSkipConverged (should have been named SNESConvergedSkip when changing to SNESConvergedDefault!), we don't need an initial function norm, so setting ttol outside the convergence test is wasteful. (I'm thinking in terms of reductions when using an inexpensive SNES iteration like a point smoother.) If the convergence test implementation wants to use a relative tolerance, then it should store the target tolerance (or equivalently, the initial residual) in its own context. I think all reference to ttol should be removed from outside the convergence tests.
Yes. How should EW version 3 be implemented then? Currently EW3 piggybacks on snes->ttol to get a measure of the initial norm of the residual, and relies on it having been set elsewhere. If no references to snes->ttol in SNESKSPEW_PreSolve() are allowed, what should take its place?
More generally, should this sort of thing be handled everywhere using snes->norm_init for consistency and to prevent potential breakage?
snes->norm_init goes with SNESSetInitialFunctionNorm and I don't think it's used otherwise.
I was just thinking of a single cache for the initial residual norm. Currently there are at least two, it seems to me. Dmitry.
At the very least, should snes->ttol = ... be removed from SNESConvergedDefault()?
Thanks. Dmitry.