Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use nonlinear solver tolerance for "Stokes only" solver scheme #1276

Merged
merged 3 commits into from Nov 30, 2016

Conversation

gassmoeller
Copy link
Member

See changes.h entry. We used a hard-coded value instead of an input parameter, and we computed the residual not consistently with the other nonlinear solver schemes. @MFraters maybe you can take a look? You have the most experience with these solver schemes.

@tjhei
Copy link
Member

tjhei commented Nov 23, 2016

Looks good.

I would assume all broken tests are only due to slightly different output, but please take a look at them.

@MFraters
Copy link
Member

Hey Rene,

I completely agree with these changes. While you are changing the test output, I would suggest 2 more changes:

  1. Model the output after iterated Stokes, because this one also shows the nonlinear iteration number, which I personally always find very useful. Also a bit more consistency between the output of the nonlinear solvers might also be a good thing.
    To have one output style for the nonlinear solvers, with only minor variations between them, might also be a good thing to discuss for version 2 of ASPECT.
  2. Add the free surface. Just add at the top:
    if (parameters.free_surface_enabled) free_surface->execute ();
    and allow it at the line // Initialize the free surface handler in core.cc

@anne-glerum Do you have any other suggestions?

const double stokes_residual = solve_stokes();
current_linearization_point = solution;

pcout << " Nonlinear Stokes residual: " << stokes_residual << std::endl;
if (stokes_residual < 1e-8)
pcout << " Nonlinear Stokes residual: " << stokes_residual/initial_stokes_residual << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this how the other solvers do it, i.e., output the relative residual? That makes no sense to me, but if that's what they all do, then I guess it makes sense to do the same here...

@gassmoeller
Copy link
Member Author

@bangerth: I do not quite recall why that decision was made, but every other nonlinear solver (iterated IMPES and iterated Stokes) currently interprets the nonlinear solver tolerance to be a relative tolerance compared to the residual of/before the first iteration. With this PR the interpretation (and output) is at least consistent.

@MFraters: I have added the iteration number and made the output more uniform like you asked. Is there an application for the free surface? 'Stokes only' only solves one timestep and the surface is not changed anyway. Or are you interested because it changes the top boundary condition (and therefore the solution)?

@gassmoeller gassmoeller force-pushed the nonlinear_tolerance_Stokes_only branch from ff084a0 to 3860401 Compare November 25, 2016 23:43
@bangerth
Copy link
Contributor

bangerth commented Nov 25, 2016 via email

@gassmoeller gassmoeller force-pushed the nonlinear_tolerance_Stokes_only branch from 257f79e to 111828b Compare November 26, 2016 03:13
@MFraters
Copy link
Member

@gassmoeller No, I wasn't personally interested in it, I just thought it could be useful to have it in there. But I am not really familiar with Stokes only, so leave it out if you think that makes more sense.

@gassmoeller gassmoeller force-pushed the nonlinear_tolerance_Stokes_only branch from 413b496 to 73896f3 Compare November 29, 2016 17:11
@gassmoeller
Copy link
Member Author

Ok then let us keep the free surface out for now (we can handle that together with #1279 if we want). Otherwise this PR is ready from my side.

@tjhei
Copy link
Member

tjhei commented Nov 30, 2016

looks good, thanks!

@tjhei tjhei merged commit 6be85a5 into geodynamics:master Nov 30, 2016
@gassmoeller gassmoeller deleted the nonlinear_tolerance_Stokes_only branch November 30, 2016 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants