Skip to content

Remove CoupledSolutionsForStaggeredScheme

Christoph Lehmann requested to merge chleh/ogs:remove-coupled-solutions into master

Fixes #3432 (closed).

This MR...

  • removes the CoupledSolutionsForStaggeredScheme and LocalCoupledSolutions
  • passes all staggered solutions to postNonLinearSolver() callbacks

Thereby it fixes a possible dangling pointer issue (I'm wondering why that didn't cause problems/wasn't noticed in the past) and removes some state from the Process class.

Some analysis

The only code making use of the coupled solutions is in the PhaseField process in postTimestepConcreteProcess() and postNonLinearSolverConcreteProcess().
For postNonLinearSolverConcreteProcess() in the current implementation the pointer is not dangling but IMO the code is quite dangerous.
In OGS master we have in the TimeLoop:

NumLib::NonlinearSolverStatus
TimeLoop::solveCoupledEquationSystemsByStaggeredScheme(
    const double t, const double dt, const std::size_t timestep_id)
{
    ...
            CoupledSolutionsForStaggeredScheme coupled_solutions(
                _process_solutions);

            process_data->process.setCoupledSolutionsForStaggeredScheme(
                &coupled_solutions);
   ...
}

...

NumLib::NonlinearSolverStatus solveOneTimeStepOneProcess(...)
{
    ...

    process.postNonLinearSolver(*x[process_id], *x_prev[process_id], t, delta_t,
                                process_id);

    return nonlinear_solver_status;
}

I.e., during the runtime of postNonLinearSolver()/postNonLinearSolverConcreteProcess() CoupledSolutionsForStaggeredScheme coupled_solutions is alive and there is no dangling pointer.

For postTimestepConcreteProcess(), on the other hand, in the TimeLoop we have:

void postTimestepForAllProcesses(
    double const t, double const dt,
    std::vector<std::unique_ptr<ProcessData>> const& per_process_data,
    std::vector<GlobalVector*> const& process_solutions,
    std::vector<GlobalVector*> const& process_solutions_prev)
{
    ...
        if (is_staggered_coupling)
        {
            CoupledSolutionsForStaggeredScheme coupled_solutions(
                process_solutions);
            pcs.setCoupledSolutionsForStaggeredScheme(&coupled_solutions);
        }
        pcs.computeSecondaryVariable(t, dt, process_solutions,
                                     *process_solutions_prev[process_id],
                                     process_id);
        pcs.postTimestep(process_solutions, process_solutions_prev, t, dt,
                         process_id);
    ...
}

I.e., CoupledSolutionsForStaggeredScheme coupled_solutions is immidiately out of scope. Why this didn't cause trouble, so far, might be luck.

Does this MR have observable effects?

Apparently not. The only "delicate" code is in the PhaseField process.
Tests still run and the logged crack integrals and energies did not change either:

[lehmannc@envinf3 release-petsc]$ ctest -R PhaseField -E LARGE -N
Test project /home/lehmannc/prog/ogs/build/release-petsc
  Test #222: ogs-PhaseField_3D_beam_tens_AT1_iso-mpirun
  Test #223: ogs-PhaseField_3D_beam_tens_AT1_iso-mpirun-vtkdiff
  Test #226: ogs-PhaseField_3D_beam_tens_AT1_vd-mpirun
  Test #227: ogs-PhaseField_3D_beam_tens_AT1_vd-mpirun-vtkdiff
  Test #233: ogs-PhaseField_2D_surfing_AT1_vd-mpirun
  Test #234: ogs-PhaseField_2D_surfing_AT1_vd-mpirun-vtkdiff
  Test #235: ogs-PhaseField_2D_K_regime_HF_2cores-mpirun
  Test #236: ogs-PhaseField_2D_K_regime_HF_2cores-mpirun-vtkdiff

Total Tests: 8

... run tests for master and feature branch, save log files ...

[lehmannc@envinf3 release-petsc]$ ls logs-master
ogs-PhaseField_2D_K_regime_HF_2cores-mpirun.txt  ogs-PhaseField_3D_beam_tens_AT1_iso-mpirun.txt
ogs-PhaseField_2D_surfing_AT1_vd-mpirun.txt      ogs-PhaseField_3D_beam_tens_AT1_vd-mpirun.txt

[lehmannc@envinf3 release-petsc]$ for f in `ls logs-master`; do echo "### $f"; diff -us <(grep -iE 'crack|energy' "log
s-master/$f") <(grep -iE 'crack|energy' "logs-remove-coupled-solutions/$f"); done
### ogs-PhaseField_2D_K_regime_HF_2cores-mpirun.txt
--- /dev/fd/63  2023-11-03 08:50:25.470025392 +0100
+++ /dev/fd/62  2023-11-03 08:50:25.470025392 +0100
@@ -1,13 +1,13 @@
 [1] info: Integral of crack: 0.0744794
 [0] info: Integral of crack: 0.0744794
-[1] info: Integral of crack: 0.0794819
 [0] info: Integral of crack: 0.0794819
+[1] info: Integral of crack: 0.0794819
 [0] info: Integral of crack: 0.0794882
 [1] info: Integral of crack: 0.0794882
-[1] info: Elastic energy: 0.000585436 Surface energy: 0.274336 Pressure work: 0.00930705  at time: 0.01 
 [0] info: Elastic energy: 0.000585436 Surface energy: 0.274336 Pressure work: 0.00930705  at time: 0.01 
-[1] info: Integral of crack: 0.0794882
+[1] info: Elastic energy: 0.000585436 Surface energy: 0.274336 Pressure work: 0.00930705  at time: 0.01 
 [0] info: Integral of crack: 0.0794882
+[1] info: Integral of crack: 0.0794882
 [1] info: Integral of crack: 0.0795633
 [0] info: Integral of crack: 0.0795633
 [0] info: Elastic energy: 0.00233959 Surface energy: 0.274338 Pressure work: 0.0186146  at time: 0.02 
### ogs-PhaseField_2D_surfing_AT1_vd-mpirun.txt
Files /dev/fd/63 and /dev/fd/62 are identical
### ogs-PhaseField_3D_beam_tens_AT1_iso-mpirun.txt
Files /dev/fd/63 and /dev/fd/62 are identical
### ogs-PhaseField_3D_beam_tens_AT1_vd-mpirun.txt
Files /dev/fd/63 and /dev/fd/62 are identical
  1. Feature description was added to the changelog
  2. Tests covering your feature were added?
  3. Any new feature or behavior change was documented?
Edited by Christoph Lehmann

Merge request reports