GEOS-DEV / GEOS

GEOS Simulation Framework
GNU Lesser General Public License v2.1
203 stars 80 forks source link

Properly sync nonlinear solver params for coupled solver #3138

Closed paveltomin closed 3 weeks ago

paveltomin commented 1 month ago

Some solvers logic depends on the number of Newton iterations. synchronizeNonlinearSolverParameters was supposed to take care of that but it was not placed correctly. In particular, the issue is that currently number of iterations for coupled solvers does not propagate down to subsolvers. This PR tries to fix that. Also add some debug info output for time step selection.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 7.24638% with 64 lines in your changes missing coverage. Please review.

Project coverage is 53.76%. Comparing base (4de1337) to head (798f833).

Files Patch % Lines
...nents/physicsSolvers/NonlinearSolverParameters.cpp 0.00% 29 Missing :warning:
src/coreComponents/physicsSolvers/SolverBase.cpp 0.00% 21 Missing :warning:
...sSolvers/fluidFlow/CompositionalMultiphaseBase.cpp 0.00% 10 Missing :warning:
src/coreComponents/physicsSolvers/SolverBase.hpp 0.00% 2 Missing :warning:
...ents/physicsSolvers/multiphysics/CoupledSolver.hpp 66.66% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3138 +/- ## =========================================== + Coverage 53.74% 53.76% +0.01% =========================================== Files 1016 1016 Lines 85756 85781 +25 =========================================== + Hits 46092 46122 +30 + Misses 39664 39659 -5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

paveltomin commented 1 month ago

Looks good to me. My only question is: would we want to keep this as a "feature"? That is the user deliberately chooses different non-linear parameters for the different solvers?

this logic only applies for fully coupled solvers, in this case different params does not make much sense

for sequential it is not going to sync anything and there is flexibility to choose different settings