From 08362dba0d72a7c78dbfb3f41690f0d5df5ee641 Mon Sep 17 00:00:00 2001
From: Shuang Chen <gechenshuang88@gmail.com>
Date: Mon, 16 Dec 2019 10:39:03 +0100
Subject: [PATCH] Fixing code following reviewer's suggestion

Add overriden control, modified data container
Rename the tespy converge control
remove unused bool variable
Passing this bool variable is not needed
---
 .../Python/BHEInflowPythonBoundaryCondition.h | 12 ++---
 ...BHEInflowPythonBoundaryConditionModule.cpp |  7 ++-
 ...thonBoundaryConditionPythonSideInterface.h | 50 +++++++++++--------
 .../CreateHeatTransportBHEProcess.cpp         | 15 ++++--
 .../HeatTransportBHEProcess.cpp               | 32 ++++++++----
 .../HeatTransportBHEProcessData.h             |  5 +-
 6 files changed, 71 insertions(+), 50 deletions(-)

diff --git a/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryCondition.h b/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryCondition.h
index 4430aac2148..f4ba293d3cb 100644
--- a/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryCondition.h
+++ b/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryCondition.h
@@ -45,7 +45,7 @@ public:
         const auto g_idx_T_out = in_out_global_indices.second;
 
         // store the bc node ids to BHE network dataframe
-        std::get<4>(_py_bc_object.dataframe_network).emplace_back(g_idx_T_out);
+        std::get<3>(_py_bc_object.dataframe_network).emplace_back(g_idx_T_out);
     }
 
     void getEssentialBCValues(
@@ -56,7 +56,7 @@ public:
         bc_values.values.resize(1);
         auto const& data_exchange = _py_bc_object.dataframe_network;
         // get the number of all boundary nodes
-        const std::size_t n_bc_nodes = std::get<4>(data_exchange).size();
+        const std::size_t n_bc_nodes = std::get<3>(data_exchange).size();
 
         // get T_in bc_id
         bc_values.ids[0] = _in_out_global_indices.first;
@@ -70,9 +70,9 @@ public:
         {
             // auto pair_flag_value =
             // _bc_data.bc_object->getDirichletBCValue(boundary_node_id);
-            auto const dataframe_node_id = std::get<4>(data_exchange);
-            auto const dataframe_Tin_val = std::get<2>(data_exchange);
-            auto const dataframe_BHE_flowrate = std::get<5>(data_exchange);
+            auto const dataframe_node_id = std::get<3>(data_exchange);
+            auto const dataframe_Tin_val = std::get<1>(data_exchange);
+            auto const dataframe_BHE_flowrate = std::get<4>(data_exchange);
             if (dataframe_node_id[i] == boundary_node_id)
             {
                 bc_values.values[0] = dataframe_Tin_val[i];
@@ -82,7 +82,7 @@ public:
         }
 
         // store the current time to network dataframe
-        std::get<1>(_py_bc_object.dataframe_network) = t;
+        std::get<0>(_py_bc_object.dataframe_network) = t;
     }
 
 private:
diff --git a/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionModule.cpp b/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionModule.cpp
index 045d40afe39..1f619cecc30 100644
--- a/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionModule.cpp
+++ b/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionModule.cpp
@@ -26,13 +26,12 @@ public:
     using BHEInflowPythonBoundaryConditionPythonSideInterface::
         BHEInflowPythonBoundaryConditionPythonSideInterface;
 
-    std::tuple<bool, double, std::vector<double>, std::vector<double>,
+    std::tuple<double, std::vector<double>, std::vector<double>,
                std::vector<int>, std::vector<double>>
     initializeDataContainer() const override
     {
-        using Ret =
-            std::tuple<bool, double, std::vector<double>, std::vector<double>,
-                       std::vector<int>, std::vector<double>>;
+        using Ret = std::tuple<double, std::vector<double>, std::vector<double>,
+                               std::vector<int>, std::vector<double>>;
         PYBIND11_OVERLOAD(Ret,
                           BHEInflowPythonBoundaryConditionPythonSideInterface,
                           initializeDataContainer);
diff --git a/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionPythonSideInterface.h b/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionPythonSideInterface.h
index dab8626ad34..0f60fcdec98 100644
--- a/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionPythonSideInterface.h
+++ b/ProcessLib/BoundaryCondition/Python/BHEInflowPythonBoundaryConditionPythonSideInterface.h
@@ -24,8 +24,7 @@ public:
      * temperature, BHE outflow bc node id, BHE flowrate)
      * set at that position and the parameters of the BHE network.
      */
-    virtual std::tuple<bool,
-                       double /*time*/,
+    virtual std::tuple<double /*time*/,
                        std::vector<double> /*Tin_val*/,
                        std::vector<double> /*Tout_val*/,
                        std::vector<int> /*bc_out_ids*/,
@@ -33,42 +32,43 @@ public:
     initializeDataContainer() const
     {
         _overridden_essential = false;
-        return std::tuple<bool,
-                          double,
+        return std::tuple<double,
                           std::vector<double>,
                           std::vector<double>,
                           std::vector<int>,
                           std::vector<double>>{
-            false, std::numeric_limits<double>::quiet_NaN(), {}, {}, {}, {}};
+            std::numeric_limits<double>::quiet_NaN(), {}, {}, {}, {}};
     }
 
     /*!
      * transfer BHE network dataframe to TESPy and get Tin from TESPy
      *
-     * \return a tuple (time, BHE Tin and Tout value from TESPy)
-     * indicating if a natural BC shall be set at that position and the new
-     * inflow temperature of all BHEs
+     * \return a tuple (if use tespyThermalSolver, if convergence achieved
+     * in tespy, BHE Tin and Tout value from TESPy)
+     * indicating if tespyThermalSolver shall be used at that position, if
+     * themal convergence has been achieved in the tespy and the new
+     * inflow temperature of all BHEs.
      */
     virtual std::tuple<bool, bool, std::vector<double>> tespyThermalSolver(
         double /*t*/,
         std::vector<double> const& /*Tin_val*/,
         std::vector<double> const& /*Tout_val*/) const
     {
-        _overridden_natural = false;
+        _overridden_tespyThermal = false;
         return std::tuple<bool, bool, std::vector<double>>{false, false, {}};
     }
 
     /*!
      * call Tespy hydraulic solver to get flow velocity in each pipe
      *
-     * \return a tuple (is_natural,  f_velocity ) indicating if a
-     * natural BC shall be set at that position and the flow velocity in each
-     * pipe of all BHEs
+     * \return a tuple (if use tespyHydroSolver,  f_velocity) indicating if
+     * tespyHydroSolver shall be used at that position and the flow velocity
+     * in each pipe of all BHEs.
      */
     virtual std::tuple<bool, std::vector<double>> tespyHydroSolver(
         double /*t*/) const
     {
-        _overridden_natural = false;
+        _overridden_tespyHydro = false;
         return std::tuple<bool, std::vector<double>>{false, {}};
     }
 
@@ -79,15 +79,20 @@ public:
     //! once.
     bool isOverriddenEssential() const { return _overridden_essential; }
 
-    //! Tells if tespySolver() has been overridden in the derived class in
-    //! Python.
+    //! Tells if tespyThermalSolver() has been overridden in the derived class
+    //! in Python.
+    //!
+    //! \pre tespyThermalSolver() must already have been called once.
+    bool isOverriddenTespyThermal() const { return _overridden_tespyThermal; }
+
+    //! Tells if tespyHydroSolver() has been overridden in the derived class
+    //! in Python.
     //!
-    //! \pre tespySolver() must already have been called once.
-    bool isOverriddenNatural() const { return _overridden_natural; }
+    //! \pre tespyHydroSolver() must already have been called once.
+    bool isOverriddenTespyHydro() const { return _overridden_tespyHydro; }
 
     // BHE network dataframe container
-    std::tuple<bool,
-               double,
+    std::tuple<double,
                std::vector<double>,
                std::vector<double>,
                std::vector<int>,
@@ -100,8 +105,11 @@ private:
     //! Tells if initializeDataContainer() has been overridden in the derived
     //! class in Python.
     mutable bool _overridden_essential = true;
-    //! Tells if tespySolver() has been overridden in the derived class in
+    //! Tells if tespyThermalSolver() has been overridden in the derived class
+    //! in Python.
+    mutable bool _overridden_tespyThermal = true;
+    //! Tells if tespyHydroSolver() has been overridden in the derived class in
     //! Python.
-    mutable bool _overridden_natural = true;
+    mutable bool _overridden_tespyHydro = true;
 };
 }  // namespace ProcessLib
diff --git a/ProcessLib/HeatTransportBHE/CreateHeatTransportBHEProcess.cpp b/ProcessLib/HeatTransportBHE/CreateHeatTransportBHEProcess.cpp
index 9c363d00c0d..b7e7ec8dbd3 100644
--- a/ProcessLib/HeatTransportBHE/CreateHeatTransportBHEProcess.cpp
+++ b/ProcessLib/HeatTransportBHE/CreateHeatTransportBHEProcess.cpp
@@ -180,15 +180,21 @@ std::unique_ptr<Process> createHeatTransportBHEProcess(
                 "file specified.");
 
         // create BHE network dataframe from Python
-        // here calls the tespyHydroSolver to get the pipe flow velocity in bhe
-        // network
         py_object->dataframe_network = py_object->initializeDataContainer();
+        if (!py_object->isOverriddenEssential())
+        {
+            DBUG(
+                "Method `initializeDataContainer' not overridden in Python "
+                "script.");
+        }
         // clear ogs bc_node_id memory in dataframe
-        std::get<4>(py_object->dataframe_network).clear();  // ogs_bc_node_id
+        std::get<3>(py_object->dataframe_network).clear();  // ogs_bc_node_id
 
+        // here calls the tespyHydroSolver to get the pipe flow velocity in bhe
+        // network
         /* for 2U type the flowrate initialization process below causes conflict
         // replace the value in flow velocity Matrix _u
-        auto const tespy_flow_rate = std::get<5>(py_object->dataframe_network);
+        auto const tespy_flow_rate = std::get<4>(py_object->dataframe_network);
         const std::size_t n_bhe = tespy_flow_rate.size();
         if (bhes.size() != n_bhe)
             OGS_FATAL(
@@ -215,7 +221,6 @@ std::unique_ptr<Process> createHeatTransportBHEProcess(
 
     HeatTransportBHEProcessData process_data(std::move(media_map),
                                              std::move(bhes),
-                                             has_network_python_bc,
                                              py_object);
 
     SecondaryVariableCollection secondary_variables;
diff --git a/ProcessLib/HeatTransportBHE/HeatTransportBHEProcess.cpp b/ProcessLib/HeatTransportBHE/HeatTransportBHEProcess.cpp
index 30ec7782944..f8afac79f43 100644
--- a/ProcessLib/HeatTransportBHE/HeatTransportBHEProcess.cpp
+++ b/ProcessLib/HeatTransportBHE/HeatTransportBHEProcess.cpp
@@ -204,26 +204,32 @@ NumLib::IterationResult HeatTransportBHEProcess::postIterationConcreteProcess(
     GlobalVector const& x)
 {
     // if the process use python boundary conditon
-    if (_process_data.has_network_python_bc == false)
+    if (_process_data.py_bc_object == nullptr)
         return NumLib::IterationResult::SUCCESS;
 
     // Here the task is to get current time flowrate and flow temperature from
     // TESPy
     auto const Tout_nodes_id =
-        std::get<4>(_process_data.py_bc_object->dataframe_network);
+        std::get<3>(_process_data.py_bc_object->dataframe_network);
     const std::size_t n_bc_nodes = Tout_nodes_id.size();
 
     // update flowrate in network if network exist a dynamic flowrate in time
     auto const cur_time =
-        std::get<1>(_process_data.py_bc_object->dataframe_network);
+        std::get<0>(_process_data.py_bc_object->dataframe_network);
     if (std::get<0>(_process_data.py_bc_object->tespyHydroSolver(cur_time)))
     {
         // calculate the current flowrate in each BHE from TESPy
         auto const cur_flowrate =
             std::get<1>(_process_data.py_bc_object->tespyHydroSolver(cur_time));
         for (std::size_t i = 0; i < n_bc_nodes; i++)
-            std::get<5>(_process_data.py_bc_object->dataframe_network)[i] =
+            std::get<4>(_process_data.py_bc_object->dataframe_network)[i] =
                 cur_flowrate[i];
+        if (!_process_data.py_bc_object->isOverriddenTespyHydro())
+        {
+            DBUG(
+                "Method `tespyHydroSolver' not overridden in Python "
+                "script.");
+        }
     }
 
     // get the outflow temperature,
@@ -232,23 +238,29 @@ NumLib::IterationResult HeatTransportBHEProcess::postIterationConcreteProcess(
     for (std::size_t i = 0; i < n_bc_nodes; i++)
     {
         // read the T_out and store them in dataframe
-        std::get<3>(_process_data.py_bc_object->dataframe_network)[i] =
+        std::get<2>(_process_data.py_bc_object->dataframe_network)[i] =
             x[Tout_nodes_id[i]];
     }
     // Tout transfer to Python
     auto const tespy_result = _process_data.py_bc_object->tespyThermalSolver(
+        std::get<0>(_process_data.py_bc_object->dataframe_network),
         std::get<1>(_process_data.py_bc_object->dataframe_network),
-        std::get<2>(_process_data.py_bc_object->dataframe_network),
-        std::get<3>(_process_data.py_bc_object->dataframe_network));
+        std::get<2>(_process_data.py_bc_object->dataframe_network));
+    if (!_process_data.py_bc_object->isOverriddenTespyThermal())
+    {
+        DBUG(
+            "Method `tespyThermalSolver' not overridden in Python "
+            "script.");
+    }
     auto const cur_Tin = std::get<2>(tespy_result);
 
     // update the T_in
     for (std::size_t i = 0; i < n_bc_nodes; i++)
-        std::get<2>(_process_data.py_bc_object->dataframe_network)[i] =
+        std::get<1>(_process_data.py_bc_object->dataframe_network)[i] =
             cur_Tin[i];
 
-    auto const if_convergence = std::get<1>(tespy_result);
-    if (if_convergence == true)
+    auto const tespy_has_converged = std::get<1>(tespy_result);
+    if (tespy_has_converged == true)
         return NumLib::IterationResult::SUCCESS;
 
     return NumLib::IterationResult::REPEAT_ITERATION;
diff --git a/ProcessLib/HeatTransportBHE/HeatTransportBHEProcessData.h b/ProcessLib/HeatTransportBHE/HeatTransportBHEProcessData.h
index cc196f522e1..77be1fff583 100644
--- a/ProcessLib/HeatTransportBHE/HeatTransportBHEProcessData.h
+++ b/ProcessLib/HeatTransportBHE/HeatTransportBHEProcessData.h
@@ -29,12 +29,10 @@ struct HeatTransportBHEProcessData final
         std::unique_ptr<MaterialPropertyLib::MaterialSpatialDistributionMap>&&
             media_map_,
         std::vector<BHE::BHETypes>&& vec_BHEs_,
-        bool const& has_network_python_bc_,
         BHEInflowPythonBoundaryConditionPythonSideInterface* py_bc_object_ =
             nullptr)
         : media_map(std::move(media_map_)),
           _vec_BHE_property(std::move(vec_BHEs_)),
-          has_network_python_bc(has_network_python_bc_),
           py_bc_object(py_bc_object_)
     {
     }
@@ -45,8 +43,7 @@ struct HeatTransportBHEProcessData final
 
     MeshLib::PropertyVector<int> const* _mesh_prop_materialIDs = nullptr;
     std::unordered_map<int, int> _map_materialID_to_BHE_ID{};
-    // if bhe network exists python boundary condition
-    bool has_network_python_bc;
+
     //! Python object computing BC values.
     BHEInflowPythonBoundaryConditionPythonSideInterface* py_bc_object;
 };
-- 
GitLab