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

Allow to reuse the preconditioner in the Newton solver #1102

Merged
merged 22 commits into from
Apr 24, 2024
Merged

Conversation

lpsaavedra
Copy link
Collaborator

@lpsaavedra lpsaavedra commented Apr 22, 2024

Description of the problem

The multigrid preconditioners were being setup at every non-linear iteration, however, we want to be able to reuse them to reduce the setup time.

Description of the solution

  • The MFNavierStokesPreconditionGMG class now sets the level constraints, operators and transfers in the constructor. Then, depending on the MG being used, there is an initialize function in charge of setting up the smoother and the coarse-grid solver.
  • Modifications were made in the Newton nonlinear solver to allow to reuse the preconditioner along non-linear newton iterations. A new parameter reuse preconditioner was introduced for this purpose. When this change was introduced, the assemble_matrix and setup_preconditioner functions of all the NavierStokesBase solvers had to be slightly modified. This change now allows to reuse the preconditioner in both lethe-fluid and lethe-fluid-matrix-free. Due to this change, also the auxiliary physics needs a setup_preconditioner function, however, it was explicitly written that is not used for those applications.
  • The constant modes are now also being used for the local smoothing approach using the recently introduced DoFTools::extract_level_constant_modes(...).
  • this-> was also included several times in the MFNavierStokesPreconditionGMG class to be able to differentiate between this class member variables and others.
  • The output of the coarse-grid iterations was finally corrected, and now prints all the information.

How Has This Been Tested?

  • All the existent test pass.
  • Ran several tests on my machine and observed a benefit in certain cases (e.g., the TGV) when reusing the preconditioner across all Newton iterations of a transient iteration.

Documentation

  • The new reuse preconditioner parameter was included in the non linear solver documentation page.

Future changes

  • This is the second step towards reusing parts of multigrid to improve the performance of the matrix-free solver.

include/core/newton_non_linear_solver.h Outdated Show resolved Hide resolved
source/solvers/mf_navier_stokes.cc Outdated Show resolved Hide resolved
source/solvers/mf_navier_stokes.cc Outdated Show resolved Hide resolved
source/solvers/mf_navier_stokes.cc Outdated Show resolved Hide resolved
source/solvers/mf_navier_stokes.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

Some small changes. I have one more file to review, will do it tomorrow :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure why this changed, but ok :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I was also not sure about this. It changed when I added the if to reuse the operators, transfer and constraints that are set in the constructor:

  if (!gmg_preconditioner)
    gmg_preconditioner = std::make_shared<MFNavierStokesPreconditionGMG<dim>>(
      this->simulation_parameters,
      this->dof_handler,
      this->mapping,
      this->cell_quadrature,
      this->forcing_function,
      this->simulation_control,
      this->mg_computing_timer,
      this->fe);

However, the number of newton iterations, linear solver iterations and coarse-grid solver iterations remain the same. It only change the residual in the 3rd/4th decimal place which caused these changes in the value of the forces. In my opinion, the change is minimal so it should not be an issue.

include/solvers/mf_navier_stokes.h Outdated Show resolved Hide resolved
include/solvers/mf_navier_stokes.h Outdated Show resolved Hide resolved
@@ -87,3 +90,4 @@ The Navier-Stokes equations (and other) are non-linear equations. The parameters
* The ``residual precision`` parameter enables to change the number of digits displayed when showing residuals (with ``set verbosity = verbose``).
* The ``force_rhs_calculation``: Force RHS recalculation at the beginning of every non-linear steps, This is required if there is a fixed point component to the non-linear solver that is changed at the beginning of every newton iteration. This is notably the case of the sharp edge method. The default value of this parameter is false.
* The ``abort at convergence failure`` allows the user to stop the simulation and throw an error if the non-linear solver has failed to converge. Setting ``abort at convergence failure = true`` will enable this feature. This is generally useful when launching a large batch of simulation to quickly identify which one have failed.
* The ``reuse preconditioner = true`` allows the simulation to use the same preconditioner between Newton iterations when using the Newton solver. This can reduce the overall time depending on the problem, and it is especially useful for the ``lethe-fluid-matrix-free`` application.
Copy link
Contributor

Choose a reason for hiding this comment

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

So in this case you are recreating the preconditioner between iterations time steps for example right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exactly... it reuses it for Newton iterations within one transient iteration...

Copy link
Contributor

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

One last comment. These are all minor things. ping me when everything has been incorporated then we can merge. Great work @lpsaavedra . Can you add a changelog entry?

mg_time_derivative_previous_solutions[level].update_ghost_values();
mg_operators[level]->evaluate_time_derivative_previous_solutions(
mg_time_derivative_previous_solutions[level]);
AssertThrow(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the best approach here to throw or would it be to just print a warning and use the max level as the min level (e.g. no GMG)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure about this... I will think about it and then maybe we can do this in the next MG PR

@lpsaavedra lpsaavedra requested a review from peterrum April 24, 2024 16:20
@blaisb blaisb merged commit f70ae1e into master Apr 24, 2024
8 checks passed
@blaisb blaisb deleted the reusing_gmg branch April 24, 2024 23:11
blaisb pushed a commit that referenced this pull request May 2, 2024
Description of the problem
After the changes in #1102 where the architecture of the Newton solver was changed to be able to reuse the preconditioner, the initial conditions related to viscosity were not working as expected in the matrix-free application when using multigrid preconditioners.

Description of the solution
Now the kinematic viscosity of all the operators of the different multigrid levels is set to the temporary viscosity when needed to ensure the correct preconditioning.

How Has This Been Tested?
The flow around a sphere benchmark, which uses the viscosity ramp as initial condition, obtains now the expected results.
yashuuzi pushed a commit that referenced this pull request May 6, 2024
Description of the problem
After the changes in #1102 where the architecture of the Newton solver was changed to be able to reuse the preconditioner, the initial conditions related to viscosity were not working as expected in the matrix-free application when using multigrid preconditioners.

Description of the solution
Now the kinematic viscosity of all the operators of the different multigrid levels is set to the temporary viscosity when needed to ensure the correct preconditioning.

How Has This Been Tested?
The flow around a sphere benchmark, which uses the viscosity ramp as initial condition, obtains now the expected results.
M-Badri pushed a commit to M-Badri/lethe that referenced this pull request Sep 29, 2024
…#1102)

Description of the problem
The multigrid preconditioners were being setup at every non-linear iteration, however, we want to be able to reuse them to reduce the setup time.

Description of the solution
The MFNavierStokesPreconditionGMG class now sets the level constraints, operators and transfers in the constructor. Then, depending on the MG being used, there is an initialize function in charge of setting up the smoother and the coarse-grid solver.
Modifications were made in the Newton nonlinear solver to allow to reuse the preconditioner along non-linear newton iterations. A new parameter reuse preconditioner was introduced for this purpose. When this change was introduced, the assemble_matrix and setup_preconditioner functions of all the NavierStokesBase solvers had to be slightly modified. This change now allows to reuse the preconditioner in both lethe-fluid and lethe-fluid-matrix-free. Due to this change, also the auxiliary physics needs a setup_preconditioner function, however, it was explicitly written that is not used for those applications.
The constant modes are now also being used for the local smoothing approach using the recently introduced DoFTools::extract_level_constant_modes(...).
this-> was also included several times in the MFNavierStokesPreconditionGMG class to be able to differentiate between this class member variables and others.
The output of the coarse-grid iterations was finally corrected, and now prints all the information.
How Has This Been Tested?
All the existent test pass.
Ran several tests on my machine and observed a benefit in certain cases (e.g., the TGV) when reusing the preconditioner across all Newton iterations of a transient iteration.
Documentation
The new reuse preconditioner parameter was included in the non linear solver documentation page.
Future changes
This is the second step towards reusing parts of multigrid to improve the performance of the matrix-free solver.


Former-commit-id: f70ae1e
M-Badri pushed a commit to M-Badri/lethe that referenced this pull request Sep 29, 2024
Description of the problem
After the changes in chaos-polymtl#1102 where the architecture of the Newton solver was changed to be able to reuse the preconditioner, the initial conditions related to viscosity were not working as expected in the matrix-free application when using multigrid preconditioners.

Description of the solution
Now the kinematic viscosity of all the operators of the different multigrid levels is set to the temporary viscosity when needed to ensure the correct preconditioning.

How Has This Been Tested?
The flow around a sphere benchmark, which uses the viscosity ramp as initial condition, obtains now the expected results.

Former-commit-id: c56e372
blaisb pushed a commit that referenced this pull request Sep 30, 2024
Description of the problem
The multigrid preconditioners were being setup at every non-linear iteration, however, we want to be able to reuse them to reduce the setup time.

Description of the solution
The MFNavierStokesPreconditionGMG class now sets the level constraints, operators and transfers in the constructor. Then, depending on the MG being used, there is an initialize function in charge of setting up the smoother and the coarse-grid solver.
Modifications were made in the Newton nonlinear solver to allow to reuse the preconditioner along non-linear newton iterations. A new parameter reuse preconditioner was introduced for this purpose. When this change was introduced, the assemble_matrix and setup_preconditioner functions of all the NavierStokesBase solvers had to be slightly modified. This change now allows to reuse the preconditioner in both lethe-fluid and lethe-fluid-matrix-free. Due to this change, also the auxiliary physics needs a setup_preconditioner function, however, it was explicitly written that is not used for those applications.
The constant modes are now also being used for the local smoothing approach using the recently introduced DoFTools::extract_level_constant_modes(...).
this-> was also included several times in the MFNavierStokesPreconditionGMG class to be able to differentiate between this class member variables and others.
The output of the coarse-grid iterations was finally corrected, and now prints all the information.
How Has This Been Tested?
All the existent test pass.
Ran several tests on my machine and observed a benefit in certain cases (e.g., the TGV) when reusing the preconditioner across all Newton iterations of a transient iteration.
Documentation
The new reuse preconditioner parameter was included in the non linear solver documentation page.
Future changes
This is the second step towards reusing parts of multigrid to improve the performance of the matrix-free solver.


Former-commit-id: f70ae1e
blaisb pushed a commit that referenced this pull request Sep 30, 2024
Description of the problem
After the changes in #1102 where the architecture of the Newton solver was changed to be able to reuse the preconditioner, the initial conditions related to viscosity were not working as expected in the matrix-free application when using multigrid preconditioners.

Description of the solution
Now the kinematic viscosity of all the operators of the different multigrid levels is set to the temporary viscosity when needed to ensure the correct preconditioning.

How Has This Been Tested?
The flow around a sphere benchmark, which uses the viscosity ramp as initial condition, obtains now the expected results.

Former-commit-id: c56e372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants