Implementation of HeatTransportBHE process #2221

HBShaoUFZ · 2018-09-26T09:12:28Z

In this pull request, the HeatTransport BHE process is implemented.

TODO:

~~Fix compilation (@endJunction)~~ compiles on jenkins now.
~~Provide project file tags' docu (WIP by Chaofan)~~

ProcessLib/HeatTransportBHE/BHE/BHE_Net_ELE_Pipe_Inner_2U.h

Applications/ApplicationsLib/ProjectData.cpp

ProcessLib/BoundaryCondition/BHEBottomDirichletBoundaryCondition.cpp

ProcessLib/BoundaryCondition/BHEBottomDirichletBoundaryCondition.h

ProcessLib/BoundaryCondition/BHEBottomDirichletBoundaryCondition.cpp

ProcessLib/BoundaryCondition/BHEBottomDirichletBoundaryCondition.h

ProcessLib/HeatTransportBHE/BHE/BHEAbstract.h

TomFischer · 2018-11-02T14:08:34Z

ProcessLib/BoundaryCondition/BHEBottomDirichletBoundaryCondition.cpp

+    }
+    // If only one of the global indices (in or out) is negative the
+    // implementation is not valid.
+    if (in_out_global_indices.first < 0 || in_out_global_indices.second < 0)


Is my understanding correct that the way the domain is partitioned is important for a successful simulation run? Do you have an idea to avoid partitioning such that in_out_global_indices.first and in_out_global_indices.second are always on the same partition or is it an open issue?

Yes. No. ;)

I added the checks just out of my head to be sure that partitioning does not go wrong. It still might go wrong, but for other reasons. I wouldn't try to prevent partitioning, but add communication, s.t. the values are passed between the partitions.

TomFischer · 2018-11-05T08:44:37Z

ProcessLib/HeatTransportBHE/BHE/BHE_1U.h

+           PipeConfiguration1U const& pipes)
+        : BHECommon{borehole, refrigerant, grout, flowAndTemperatureControl},
+          _pipes(pipes)
+    {


Why aren't the implementations of the constructor and some other methods of this class in the cpp file?

As suggested by TF, the implementation of the constructor and other functions have been moved to the cpp file. The one still staying in the h file is the "assembleRMatrices" function. This function is called in the constructor of local BHE element assembler to initialize the thermal resistance matrices.

TomFischer · 2018-11-05T08:46:48Z

ProcessLib/HeatTransportBHE/BHE/BHE_1U.h

+                    1.0 * matBHE_loc_R;  // K_ig
+                R_matrix.block(3 * NPoints, 3 * NPoints, NPoints, NPoints) +=
+                    1.0 * matBHE_loc_R;  // K_og
+                break;


Why no default branch?

A default case has been added to prevent the code from getting into undefined status.

TomFischer · 2018-11-05T09:08:28Z

ProcessLib/HeatTransportBHE/BHE/BHE_1U.h

+{
+namespace BHE
+{
+class BHE_1U final : public BHECommon


Brief documentation of the class would be nice.

Documentation has been added.

TomFischer

Looks in general good. Some minor things to discuss/correct.

TomFischer · 2018-11-05T12:05:05Z

ProcessLib/HeatTransportBHE/BHE/PipeConfiguration1U.h

+    /// Distance between pipes.
+    double const distance;
+
+    double const longitudinal_despersion_length;


Did you mean 'dispersion'?

It means the longitudinal thermal dispersivity, all the name has been replaced by the longitudinal_thermal_dispersivity.

@ChaofanChen there is typo in the variable name...

Yes, but I don't think this variable name is clear enough, not just the typo problem. So I changed a lot including parameters' documentation.

I will stick with "longitudinal_dispersion_length"

TomFischer · 2018-11-06T07:45:46Z

ProcessLib/HeatTransportBHE/HeatTransportBHEProcess.cpp

+            i;
+    }
+
+    _process_data._mesh_prop_materialIDs = material_ids;


Minor: I would move line 65 behind line 56.

It has been fixed.

TomFischer · 2018-11-06T09:34:57Z

ProcessLib/HeatTransportBHE/LocalAssemblers/HeatTransportBHELocalAssemblerBHE-impl.h

+{
+namespace HeatTransportBHE
+{
+using namespace BHE;


@endJunction
I'm not sure if using namespace BHE; is here in the header okay.

TomFischer · 2018-11-06T09:44:25Z

ProcessLib/HeatTransportBHE/LocalAssemblers/HeatTransportBHELocalAssemblerBHE.h

+
+    BHEType const& _bhe;
+
+    std::size_t const element_id;


Class attributes start with _.

TomFischer · 2018-11-06T09:50:15Z

ProcessLib/HeatTransportBHE/LocalAssemblers/HeatTransportBHELocalAssemblerSoil.h

+    std::vector<ShapeMatrices, Eigen::aligned_allocator<ShapeMatrices>>
+        _shape_matrices;
+
+    std::size_t const element_id;


Class attributes start with _.

TomFischer · 2018-11-06T10:11:16Z

ProcessLib/HeatTransportBHE/LocalAssemblers/SecondaryData.h

+{
+namespace HeatTransportBHE
+{
+/// Used by for extrapolation of the integration point values. It is ordered


Used by for ...

The extra word has been deleted.

wenqing · 2018-11-05T09:24:56Z

...s/HEAT_TRANSPORT_BHE/borehole_heat_exchangers/borehole_heat_exchanger/borehole/t_diameter.md

@@ -0,0 +1 @@
+The borehole diameter is one of the most important geometric parameters of borehole.


I think the explanation of a keyword should follow the style of what for i_borehole.md, which displays like the following

[tag] borehole It represents the well used to install pipes for heat transport.

With the present form, the displayed one is

[tag]diameter The borehole diameter is one of the most important geometric parameters of borehole.

It is better to change it to

[tag]diameter It is the borehole diameter, one of the most important geometric parameters of borehole.

or

[tag]diameter The borehole diameter, one of the most important geometric parameters of borehole.

The same for the following parameter documentations.

They are rephrased.

wenqing · 2018-11-05T16:34:18Z

ProcessLib/BoundaryCondition/BHEInflowDirichletBoundaryCondition.h

+public:
+    BHEInflowDirichletBoundaryCondition(
+        std::pair<GlobalIndexType, GlobalIndexType>&& in_out_global_indices,
+        BHEType& bhe)


with const?

Unfortunately not possible; The bhe has a global state which is updated in the getTinByTout call.
That member function (the name is not telling it) is not const and is modifying the flow rate and returns new temperature for the BC.

@HBShaoUFZ Could you please rename the function and its documentation, which mistakenly says something about "fixed power"?

wenqing · 2018-11-05T16:39:05Z

ProcessLib/HeatTransportBHE/BHE/BHECommon.h

+ *
+ * 1) Diersch_2011_CG
+ * Two very important references to understand this class implementations are:
+ * H.-J.G. Diersch, D. Bauer, W. Heidemann, W. R黨aak, P. Sch鋞zl,


It seems that there are Unicode characters in W. R ...aak, P. Sch...zl.

wenqing · 2018-11-06T16:20:11Z

ProcessLib/HeatTransportBHE/BHE/BHE_1U.h

+                OGS_FATAL(
+                    "Error!!! In the function BHE_1U::assembleRMatrices, "
+                    "the index of bhe unknowns is out of range! ");
+                break;


This break is not needed.

It's been removed.

wenqing · 2018-11-06T16:29:01Z

ProcessLib/HeatTransportBHE/BHE/MeshUtils.cpp

+    return {bhe_material_ids, bhe_elements, bhe_nodes};
+}
+}  // end of namespace HeatTransportBHE
+}  // namespace ProcessLib


Now new line at the end of the file. It is not important but github complains at here.

wenqing · 2018-11-06T16:34:06Z

ProcessLib/HeatTransportBHE/HeatTransportBHEProcess.cpp

+    {
+        OGS_FATAL(
+            "The number of the given BHE properties (%d) are not consistent "
+            "with the number of BHE groups in a mesh (%d).",


a mesh --> the mesh

wenqing · 2018-11-06T16:43:16Z

ProcessLib/HeatTransportBHE/LocalAssemblers/HeatTransportBHELocalAssemblerSoil-impl.h

+        auto const& K_ip = ip_data.dNdxTdNdx_product_times_w;
+
+        // auto const k_f = _process_data.thermal_conductivity_fluid(t, pos)[0];
+        // auto const k_g = _process_data.thermal_conductivity_gas(t, pos)[0];


Are k_f and k_g needed for future development?

Yes. Currently this is the fully saturated case. I leave the k_f and k_g here in case later on RichardsFlow or MultiphaseFlow process in integrated with the HeatTransportBHE process. For now these parameters are commented out.

wenqing · 2018-11-06T16:48:25Z

ProcessLib/HeatTransportBHE/LocalAssemblers/HeatTransportBHELocalAssemblerSoil.h

+
+    std::size_t const _element_id;
+
+    bool const _is_axially_symmetric;


Is _is_axially_symmetric needed? or another question, can the current implementation of BHE be applied for axis-symmetrical problem? I see that GlobalDim = 3.

Removed member; Kept in ctor for shape function initialization.

wenqing · 2018-11-06T16:51:45Z

ProcessLib/HeatTransportBHE/BHE/BHE_1U.h

+                    1.0 * matBHE_loc_R;  // K_i1
+                R_matrix.block(2 * NPoints, 2 * NPoints, NPoints, NPoints) +=
+                    1.0 * matBHE_loc_R;  // K_ig
+                break;


break --> return

wenqing

Looks good. Only minor stuffs.

Avoid duplicates. Remove unused code parts. Update source code docu. Use STL algorithms. Stronger assertions and improve error messages. etc.

1) move function bodies from .h to .cpp file. 2) add documentation of the BHE_1U class. 3) add the default case for the switch.

wenqing · 2018-11-20T13:42:18Z

ProcessLib/HeatTransportBHE/BHE/BHE_1U.cpp

+    double R_gg, R_gs;
+    std::tie(R_gg, R_gs) = thermalResistancesGroutSoil(chi, R_ar, R_g);
+
+    return {R_fig, R_fog, R_gg, R_gs};


Compilation warning:

BHE_1U.cpp:234:13: warning: suggest braces around initialization of subobject [-Wmissing-braces] return {R_fig, R_fog, R_gg, R_gs}; ^~~~~~~~~~~~~~~~~~~~~~~~ { }

Please change it to

return {{R_fig, R_fog, R_gg, R_gs}};

Fixed in 68af007

ogsbot · 2020-06-19T12:54:56Z

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

endJunction added WIP 🏗️ stop review 🛑 labels Sep 26, 2018