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

Let solve_stokes() return something more uniform. #2094

Merged
merged 1 commit into from Feb 16, 2018

Conversation

bangerth
Copy link
Contributor

Previously, solve_stokes() returned something in some contexts, and something else in others.
Make this more uniform by just returning a pair of values.

While there, also adjust some naming of variables to indicate that they refer to
nonlinear residuals.

Fixes #2082.

@bangerth
Copy link
Contributor Author

@MFraters -- I'd like you take a close look at this and tell me if this makes sense. Start with the documentation of solve_stokes() and its implementation, and then see whether I'm using things consistently in the places where we call this function.

Because solve_stokes() returned different things before, depending on context, and now returns two values at once, I'm not sure whether I'm picking the correct one of the two values in all of the contexts. I think you know that part of the code better than I do.

Copy link
Member

@MFraters MFraters left a comment

Choose a reason for hiding this comment

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

I read to part of the pull request, so here are my first comments. I will read to the rest tomorrow.

{
TimerOutput::Scope timer (computing_timer, " Solve Stokes system");
pcout << " Solving Stokes system... " << std::flush;

// extract Stokes parts of solution vector, without any ghost elements
LinearAlgebra::BlockVector distributed_stokes_solution (introspection.index_sets.stokes_partitioning, mpi_communicator);

double initial_residual = numbers::signaling_nan<double>();
double initial_nonlinear_residual = numbers::signaling_nan<double>();
// the final linear residual is for the direct solver zero,
// so we set it here to zero. For the iterative solver, we
Copy link
Member

Choose a reason for hiding this comment

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

please update this.

@@ -919,7 +926,8 @@ namespace aspect
if (parameters.include_melt_transport)
melt_handler->compute_melt_variables(solution);

return initial_residual;
return std::pair<double,double>(final_linear_residual,
initial_nonlinear_residual);
Copy link
Member

Choose a reason for hiding this comment

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

looking at the documentation in simulator.h and the code in solver_schemes, I think you mend to do this the other way around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, of course.

Previously, solve_stokes() returned something in some contexts, and something else in others.
Make this more uniform by just returning a pair of values.

While there, also adjust some naming of variables to indicate that they refer to
*nonlinear* residuals.
@bangerth
Copy link
Contributor Author

OK, let's see whether this round makes more sense now.

@MFraters
Copy link
Member

Looks good to me!

@MFraters MFraters merged commit 65c152e into geodynamics:master Feb 16, 2018
@bangerth bangerth deleted the fix-solve_stokes branch February 16, 2018 00:19
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

2 participants