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

[NL] Add damping factor to global Newton. #2004

Merged
merged 1 commit into from Dec 5, 2017

Conversation

Projects
None yet
4 participants
@endJunction
Member

endJunction commented Dec 3, 2017

This adds damping factor to the global newton to the constructor and the prj file parser.
Is it OK to have this one to have a default value 1.0? Or shell I add <damping>1<\damping> to all projects?

@@ -89,9 +89,11 @@ class NonlinearSolver<NonlinearSolverTag::Newton> final
*/
explicit NonlinearSolver(
GlobalLinearSolver& linear_solver,
const unsigned maxiter)
const unsigned maxiter,
double const damping = 1.0)

This comment has been minimized.

@TomFischer

TomFischer Dec 4, 2017

Member

Indentation looks defect.

@@ -321,10 +321,12 @@ createNonlinearSolver(GlobalLinearSolver& linear_solver,
}
if (type == "Newton")
{
//! \ogs_file_param{prj__nonlinear_solvers__nonlinear_solver__damping}

This comment has been minimized.

@TomFischer

TomFischer Dec 4, 2017

Member

Is there already a corresponding documentation?

@chleh

chleh approved these changes Dec 4, 2017

I'd vote for the default argument of damping = 1.

@@ -117,8 +119,7 @@ class NonlinearSolver<NonlinearSolverTag::Newton> final
ConvergenceCriterion* _convergence_criterion = nullptr;
const unsigned _maxiter; //!< maximum number of iterations
double const _alpha =
1; //!< Damping factor. \todo Add constructor parameter.
double const _damping; //!< Damping factor \in (0, 1]

This comment has been minimized.

@chleh

chleh Dec 4, 2017

Contributor

Should the range be chacked in the code?

@wenqing

wenqing approved these changes Dec 4, 2017

default damping factor = 1.

@endJunction endJunction force-pushed the endJunction:DampedNewton branch from e793948 to 6a610ee Dec 4, 2017

@endJunction endJunction added the WIP 🏗 label Dec 4, 2017

@endJunction endJunction force-pushed the endJunction:DampedNewton branch from 6a610ee to 5871fde Dec 4, 2017

@endJunction

This comment has been minimized.

Member

endJunction commented Dec 4, 2017

Formatted, docu provided, added a check for the factor being positive, removed the condition <=1 because someone (@nagelt ) might want to overdo it a little ;)

Thanks for input, I tend to be sloppy around midnight...

@endJunction endJunction removed the WIP 🏗 label Dec 4, 2017

@TomFischer

@endJunction endJunction merged commit ac9b6e2 into ufz:master Dec 5, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@endJunction endJunction deleted the endJunction:DampedNewton branch Dec 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment