From a7b65ffab701a2c0e9f47c1d1356d6965153872a Mon Sep 17 00:00:00 2001
From: Wenqing Wang <wenqing.wang@ufz.de>
Date: Tue, 24 Jan 2017 14:51:53 +0100
Subject: [PATCH] [Coupling] Some minor improvements according to the Dima's
 further comments

---
 ProcessLib/LocalAssemblerInterface.cpp    |  2 +-
 ProcessLib/StaggeredCouplingTerm.cpp      |  4 +-
 ProcessLib/StaggeredCouplingTerm.h        | 31 +++++++++------
 ProcessLib/UncoupledProcessesTimeLoop.cpp | 48 +++++++++++------------
 ProcessLib/UncoupledProcessesTimeLoop.h   | 12 +++++-
 ProcessLib/VectorMatrixAssembler.cpp      | 31 +++++++--------
 6 files changed, 70 insertions(+), 58 deletions(-)

diff --git a/ProcessLib/LocalAssemblerInterface.cpp b/ProcessLib/LocalAssemblerInterface.cpp
index ab83fdaef16..aeaca5e2121 100644
--- a/ProcessLib/LocalAssemblerInterface.cpp
+++ b/ProcessLib/LocalAssemblerInterface.cpp
@@ -50,7 +50,7 @@ void LocalAssemblerInterface::coupling_assembleWithJacobian(
 {
     OGS_FATAL(
         "The coupling_assembleWithJacobian() function is not implemented in"
-        " the local.");
+        " the local assembler.");
 }
 
 void LocalAssemblerInterface::computeSecondaryVariable(
diff --git a/ProcessLib/StaggeredCouplingTerm.cpp b/ProcessLib/StaggeredCouplingTerm.cpp
index 1a16e880868..0e17509afd4 100644
--- a/ProcessLib/StaggeredCouplingTerm.cpp
+++ b/ProcessLib/StaggeredCouplingTerm.cpp
@@ -17,10 +17,10 @@ namespace ProcessLib
 {
 const StaggeredCouplingTerm createVoidStaggeredCouplingTerm()
 {
-    std::map<ProcessType, Process const&> coupled_pcsss;
+    std::map<ProcessType, Process const&> coupled_processes;
     std::map<ProcessType, GlobalVector const&> coupled_xs;
     const bool empty = true;
-    return StaggeredCouplingTerm(coupled_pcsss, coupled_xs, 0.0, empty);
+    return StaggeredCouplingTerm(coupled_processes, coupled_xs, 0.0, empty);
 }
 
 } // end of ProcessLib
diff --git a/ProcessLib/StaggeredCouplingTerm.h b/ProcessLib/StaggeredCouplingTerm.h
index a9f31147bea..88e27222099 100644
--- a/ProcessLib/StaggeredCouplingTerm.h
+++ b/ProcessLib/StaggeredCouplingTerm.h
@@ -35,8 +35,10 @@ struct StaggeredCouplingTerm
         std::map<ProcessType, Process const&> const& coupled_processes_,
         std::map<ProcessType, GlobalVector const&> const& coupled_xs_,
         const double dt_, const bool empty_ = false)
-    : coupled_processes(coupled_processes_), coupled_xs(coupled_xs_),
-      dt(dt_), empty(empty_)
+        : coupled_processes(coupled_processes_),
+          coupled_xs(coupled_xs_),
+          dt(dt_),
+          empty(empty_)
     {
     }
 
@@ -48,8 +50,8 @@ struct StaggeredCouplingTerm
     /// The coupled solutions are distinguished by the keys of process types.
     std::map<ProcessType, GlobalVector const&> const& coupled_xs;
 
-    const double dt;  ///< Time step size.
-    const bool empty; ///< Flag to indicate whether the couping term is empty.
+    const double dt;   ///< Time step size.
+    const bool empty;  ///< Flag to indicate whether the couping term is empty.
 };
 
 /**
@@ -62,17 +64,19 @@ struct StaggeredCouplingTerm
  */
 struct LocalCouplingTerm
 {
-    LocalCouplingTerm(const double dt_,
+    LocalCouplingTerm(
+        const double dt_,
         std::map<ProcessType, Process const&> const& coupled_processes_,
         std::map<ProcessType, const std::vector<double>>&& local_coupled_xs0_,
         std::map<ProcessType, const std::vector<double>>&& local_coupled_xs_)
-    : dt(dt_), coupled_processes(coupled_processes_),
-      local_coupled_xs0(std::move(local_coupled_xs0_)),
-      local_coupled_xs(std::move(local_coupled_xs_))
+        : dt(dt_),
+          coupled_processes(coupled_processes_),
+          local_coupled_xs0(std::move(local_coupled_xs0_)),
+          local_coupled_xs(std::move(local_coupled_xs_))
     {
     }
 
-    const double dt; ///< Time step size.
+    const double dt;  ///< Time step size.
 
     /// References to the coupled processes are distinguished by the keys of
     /// process types.
@@ -84,8 +88,11 @@ struct LocalCouplingTerm
     std::map<ProcessType, const std::vector<double>> const local_coupled_xs;
 };
 
-/// A function to create a void instance of StaggeredCouplingTerm;
+/**
+ *  A function to create a void instance of StaggeredCouplingTerm. The void
+ *  instance is for the StaggeredCouplingTerm reference type of argument
+ *  function or class member to indicate no coupling.
+ */
 const StaggeredCouplingTerm createVoidStaggeredCouplingTerm();
 
-} // end of ProcessLib
-
+}  // end of ProcessLib
diff --git a/ProcessLib/UncoupledProcessesTimeLoop.cpp b/ProcessLib/UncoupledProcessesTimeLoop.cpp
index 6b3fa5784a4..05bd6ae7174 100644
--- a/ProcessLib/UncoupledProcessesTimeLoop.cpp
+++ b/ProcessLib/UncoupledProcessesTimeLoop.cpp
@@ -306,16 +306,16 @@ std::vector<std::unique_ptr<SingleProcessData>> createPerProcessData(
             //! \ogs_file_param{prj__time_loop__processes__process__convergence_criterion}
             pcs_config.getConfigSubtree("convergence_criterion"));
 
-        auto const& cpl_pcses
+        auto const& coupled_process_tree
             //! \ogs_file_param{prj__time_loop__processes__process__coupled_processes}
             = pcs_config.getConfigSubtreeOptional("coupled_processes");
         std::map<ProcessType, Process const&> coupled_processes;
-        if (cpl_pcses)
+        if (coupled_process_tree)
         {
             for (
                 auto const cpl_pcs_name :
                 //! \ogs_file_param{prj__time_loop__processes__process__coupled_processes__coupled_process}
-                cpl_pcses->getConfigParameterList<std::string>(
+                coupled_process_tree->getConfigParameterList<std::string>(
                     "coupled_process"))
             {
                 auto& cpl_pcs_ptr = BaseLib::getOrError(
@@ -488,15 +488,6 @@ UncoupledProcessesTimeLoop::UncoupledProcessesTimeLoop(
 {
 }
 
-/**
- *  This function fills the vector of solutions of coupled processes of
- *  processes, _solutions_of_coupled_processes, and initializes the vector of
- *  solutions of the previous coupling iteration,
- *  _solutions_of_last_cpl_iteration.
- *
- *  \return a boolean value as a flag to indicate there should be a coupling
- *          among process or not.
- */
 bool UncoupledProcessesTimeLoop::setCoupledSolutions()
 {
     if (!_global_coupling_conv_crit && _global_coupling_max_iterations == 1)
@@ -511,10 +502,10 @@ bool UncoupledProcessesTimeLoop::setCoupledSolutions()
 
         auto const& coupled_processes = spd->coupled_processes;
         std::map<ProcessType, GlobalVector const&> coupled_xs;
-        auto it = coupled_processes.begin();
-        while (it != coupled_processes.end())
+        for (auto const& coupled_process_map : coupled_processes)
         {
-            ProcessLib::Process const& coupled_process = it->second;
+            ProcessLib::Process const& coupled_process =
+                coupled_process_map.second;
             auto const found_item = std::find_if(
                 _per_process_data.begin(),
                 _per_process_data.end(),
@@ -530,20 +521,21 @@ bool UncoupledProcessesTimeLoop::setCoupledSolutions()
                 const std::size_t c_id = found_item - _per_process_data.begin();
 
                 BaseLib::insertIfKeyUniqueElseError(
-                    coupled_xs, it->first, *_process_solutions[c_id],
-                    "global_coupled_x");
+                    coupled_xs, coupled_process_map.first,
+                    *_process_solutions[c_id], "global_coupled_x");
             }
-            it++;
         }
         _solutions_of_coupled_processes.emplace_back(coupled_xs);
 
         const auto x = _process_solutions[pcs_idx];
 
-        auto x1 = &NumLib::GlobalVectorProvider::provider.getVector(*x);
-        MathLib::LinAlg::copy(*x, *x1);
+        // Create a vector to store the solution of the last coupling iteration
+        auto x_coupling0 =
+            &NumLib::GlobalVectorProvider::provider.getVector(*x);
+        MathLib::LinAlg::copy(*x, *x_coupling0);
 
         // append a solution vector of suitable size
-        _solutions_of_last_cpl_iteration.emplace_back(x1);
+        _solutions_of_last_cpl_iteration.emplace_back(x_coupling0);
 
         ++pcs_idx;
     }
@@ -620,14 +612,14 @@ bool UncoupledProcessesTimeLoop::loop()
             nonlinear_solver_succeeded =
                 solveUncoupledEquationSystems(t, delta_t, timestep);
 
-        INFO("[time] Timestep #%u took %g s.", timestep,
+        INFO("[time] Time step #%u took %g s.", timestep,
              time_timestep.elapsed());
 
         if (!nonlinear_solver_succeeded)
             break;
     }
 
-    // output last timestep
+    // output last time step
     if (nonlinear_solver_succeeded)
     {
         unsigned pcs_idx = 0;
@@ -671,8 +663,7 @@ bool UncoupledProcessesTimeLoop::solveUncoupledEquationSystems(
         if (!nonlinear_solver_succeeded)
         {
             ERR("The nonlinear solver failed in time step #%u at t = %g "
-                "s"
-                " for process #%u.",
+                "s for process #%u.",
                 timestep_id, t, pcs_idx);
 
             // save unsuccessful solution
@@ -711,13 +702,18 @@ bool UncoupledProcessesTimeLoop::solveCoupledEquationSystemsByStaggeredScheme(
             auto& x = *_process_solutions[pcs_idx];
             if (i == 0)
             {
+                // Copy the solution of the previous time step to a vector that
+                // belongs to process. For some problems, both of the current
+                // solution and the solution of the previous time step are
+                // required for the coupling computation.
                 pcs.preTimestep(x, t, dt);
 
-                // Create a coupling term
+                // Set the flag of the first iteration be true.
                 _global_coupling_conv_crit->preFirstIteration();
             }
             else
             {
+                // Set the flag of the first iteration be false.
                 _global_coupling_conv_crit->setNoFirstIteration();
             }
             StaggeredCouplingTerm coupled_term(
diff --git a/ProcessLib/UncoupledProcessesTimeLoop.h b/ProcessLib/UncoupledProcessesTimeLoop.h
index 2ce1510f777..dac90c5919c 100644
--- a/ProcessLib/UncoupledProcessesTimeLoop.h
+++ b/ProcessLib/UncoupledProcessesTimeLoop.h
@@ -42,6 +42,16 @@ public:
 
     ~UncoupledProcessesTimeLoop();
 
+    /**
+     *  This function fills the vector of solutions of coupled processes of
+     *  processes, _solutions_of_coupled_processes, and initializes the vector
+     * of
+     *  solutions of the previous coupling iteration,
+     *  _solutions_of_last_cpl_iteration.
+     *
+     *  \return a boolean value as a flag to indicate there should be a coupling
+     *          among processes or not.
+     */
     bool setCoupledSolutions();
 
 private:
@@ -68,7 +78,7 @@ private:
     std::vector<GlobalVector*> _solutions_of_last_cpl_iteration;
 
     /**
-     * \brief Member to solver uncoupled systems of equations, which can be
+     * \brief Member to solver non coupled systems of equations, which can be
      *        a single system of equations, or several systems of equations
      *        without any dependency among the different systems.
      *
diff --git a/ProcessLib/VectorMatrixAssembler.cpp b/ProcessLib/VectorMatrixAssembler.cpp
index a243c471bed..bf257e72943 100644
--- a/ProcessLib/VectorMatrixAssembler.cpp
+++ b/ProcessLib/VectorMatrixAssembler.cpp
@@ -26,26 +26,25 @@ getPreviousLocalSolutionsOfCoupledProcesses(
 {
     std::map<ProcessLib::ProcessType, const std::vector<double>>
         local_coupled_xs0;
-    auto it = coupled_term.coupled_processes.begin();
-    while (it != coupled_term.coupled_processes.end())
+
+    for (auto const& coupled_process_map : coupled_term.coupled_processes)
     {
-        auto const& coupled_pcs = it->second;
+        auto const& coupled_pcs = coupled_process_map.second;
         auto const prevous_time_x = coupled_pcs.getPreviousTimeStepSolution();
         if (prevous_time_x)
         {
             auto const local_coupled_x0 = prevous_time_x->get(indices);
-            BaseLib::insertIfKeyUniqueElseError(local_coupled_xs0, it->first,
-                                                local_coupled_x0,
-                                                "local_coupled_x0");
+            BaseLib::insertIfKeyUniqueElseError(
+                local_coupled_xs0, coupled_process_map.first, local_coupled_x0,
+                "local_coupled_x0");
         }
         else
         {
             const std::vector<double> local_coupled_x0;
-            BaseLib::insertIfKeyUniqueElseError(local_coupled_xs0, it->first,
-                                                local_coupled_x0,
-                                                "local_coupled_x0");
+            BaseLib::insertIfKeyUniqueElseError(
+                local_coupled_xs0, coupled_process_map.first, local_coupled_x0,
+                "local_coupled_x0");
         }
-        it++;
     }
     return local_coupled_xs0;
 }
@@ -57,14 +56,14 @@ getCurrentLocalSolutionsOfCoupledProcesses(
 {
     std::map<ProcessLib::ProcessType, const std::vector<double>>
         local_coupled_xs;
-    auto it = global_coupled_xs.begin();
-    while (it != global_coupled_xs.end())
+
+    for (auto const& global_coupled_x_map : global_coupled_xs)
     {
-        auto const coupled_x = it->second;
+        auto const coupled_x = global_coupled_x_map.second;
         auto const local_coupled_x = coupled_x.get(indices);
-        BaseLib::insertIfKeyUniqueElseError(
-            local_coupled_xs, it->first, local_coupled_x, "local_coupled_x");
-        it++;
+        BaseLib::insertIfKeyUniqueElseError(local_coupled_xs,
+                                            global_coupled_x_map.first,
+                                            local_coupled_x, "local_coupled_x");
     }
     return local_coupled_xs;
 }
-- 
GitLab