Merge branch '3174-follow-up-from-mel-io-xdmf-writer-without-xdmf-library' into 'master'

Review XDMF Writer follow up

See merge request ogs/ogs!3722

MeshLib/IO/XDMF/XdmfHdfWriter.cpp

@@ -42,10 +42,12 @@ XdmfHdfWriter::XdmfHdfWriter(MeshLib::Mesh const& mesh,
     // list
     std::function<bool(HdfData)> is_variable_hdf_attribute =
         [&variable_output_names](
-            bool outputnames) -> std::function<bool(HdfData)> {
+            bool outputnames) -> std::function<bool(HdfData)>
+    {
         if (outputnames)
         {
-            return [&variable_output_names](HdfData data) -> bool {
+            return [&variable_output_names](HdfData data) -> bool
+            {
                 return std::find(variable_output_names.begin(),
                                  variable_output_names.end(),
                                  data.name) != variable_output_names.end();
@@ -88,17 +90,19 @@ XdmfHdfWriter::XdmfHdfWriter(MeshLib::Mesh const& mesh,
     std::vector<XdmfData> xdmf_attributes;
     std::transform(attributes.begin(), attributes.end(),
                    std::back_inserter(xdmf_attributes),
-                   [&attributes](AttributeMeta& att) -> XdmfData
-                   {
-                       size_t const i = &att - &attributes.front();
-                       // index 1 geo, index 2 topology, attributes start at
-                       // index 3
-                       att.xdmf.index = i + 4;
-                       return att.xdmf;
-                   });
+                   [](AttributeMeta const& att) -> XdmfData
+                   { return att.xdmf; });
+
+    for (size_t i = 0; i < attributes.size(); ++i)
+    {
+        // index 1 time,  index 2 geo, index 3 topology, attributes start at
+        // index 4
+        xdmf_attributes[i].index = i + 4;
+    }
 
     std::function<bool(XdmfData)> is_variable_xdmf_attribute =
-        [&variable_output_names](XdmfData data) -> bool {
+        [&variable_output_names](XdmfData data) -> bool
+    {
         return std::find(variable_output_names.begin(),
                          variable_output_names.end(),
                          data.name) != variable_output_names.end();
@@ -115,18 +119,18 @@ XdmfHdfWriter::XdmfHdfWriter(MeshLib::Mesh const& mesh,
 
     if (isFileManager())
     {
-        auto xdmf_writer_fn =
-            write_xdmf(geometry.xdmf, topology.xdmf, xdmf_constant_attributes,
-                       xdmf_variable_attributes, hdf_filepath.string(),
-                       GitInfoLib::GitInfo::ogs_version);
+        auto xdmf_writer_fn = write_xdmf(
+            geometry.xdmf, topology.xdmf, xdmf_constant_attributes,
+            xdmf_variable_attributes, hdf_filepath.filename().string(),
+            GitInfoLib::GitInfo::ogs_version);
         _xdmf_writer = std::make_unique<XdmfWriter>(xdmf_filepath.string(),
                                                     xdmf_writer_fn);
         _xdmf_writer->addTimeStep(initial_time);
     }
 }
 
-void XdmfHdfWriter::writeStep([[maybe_unused]] int const& time_step,
-                              double const& time)
+void XdmfHdfWriter::writeStep([[maybe_unused]] int const time_step,
+                              double const time)
 {
     // ToDo (tm) time_step will be used for simulation continuation (restart)
     _hdf_writer->writeStep();

MeshLib/IO/XDMF/XdmfHdfWriter.h

@@ -48,7 +48,7 @@ public:
      * @param time_step number of the step (temporal collection)
      * @param time time value of the current time_step
      */
-    void writeStep(int const& time_step, double const& time);
+    void writeStep(int time_step, double time);
 
 private:
     // hdf_writer must be destructed before xdmf_writer

MeshLib/IO/XDMF/XdmfWriter.cpp

@@ -8,10 +8,10 @@
 
 namespace MeshLib::IO
 {
-XdmfWriter::XdmfWriter(std::string const& xdmf_filename,
+XdmfWriter::XdmfWriter(std::string xdmf_filename,
                        std::function<std::string(std::vector<double>)>
                            xdmf_writer_fn)
-    : filename(xdmf_filename), xdmf_writer(xdmf_writer_fn)
+    : filename(std::move(xdmf_filename)), xdmf_writer(xdmf_writer_fn)
 {
 }
 

MeshLib/IO/XDMF/XdmfWriter.h

@@ -25,7 +25,7 @@ public:
      * @param xdmf_filename absolute or relative filepath to the xdmf file
      * @param xdmf_writer_fn function that generates xdmf string
      */
-    XdmfWriter(std::string const& xdmf_filename,
+    XdmfWriter(std::string xdmf_filename,
                std::function<std::string(std::vector<double>)>
                    xdmf_writer_fn);
     XdmfWriter(XdmfWriter&&) = default;

MeshLib/IO/XDMF/transformData.cpp

@@ -36,7 +36,7 @@ struct XdmfTopology
     unsigned int number_of_nodes;
 };
 
-constexpr auto elemOGSTypeToXDMFType()
+static constexpr auto elemOGSTypeToXDMFType()
 {
     std::array<XdmfTopology, to_underlying(MeshLib::CellType::enum_length)>
         elem_type{};
@@ -62,9 +62,10 @@ constexpr auto elemOGSTypeToXDMFType()
     return elem_type;
 }
 
-auto cellTypeOGS2XDMF(MeshLib::CellType const& cell_type)
+constexpr auto elem_type_ogs2xdmf = elemOGSTypeToXDMFType();
+
+constexpr auto cellTypeOGS2XDMF(MeshLib::CellType const& cell_type)
 {
-    constexpr auto elem_type_ogs2xdmf = elemOGSTypeToXDMFType();
     return elem_type_ogs2xdmf[to_underlying(cell_type)];
 }
 
@@ -80,7 +81,8 @@ std::optional<AttributeMeta> transformAttribute(
     // (and overwrites) data that can only be collected via the typed property.
     // It has boolean return type to allow kind of pipe using || operator.
     auto f = [&data_type, &num_of_tuples, &data_ptr,
-              &property_pair](auto basic_type) -> bool {
+              &property_pair](auto basic_type) -> bool
+    {
         auto const property_base = property_pair.second;
         auto const typed_property =
             dynamic_cast<PropertyVector<decltype(basic_type)> const*>(

MeshLib/IO/XDMF/writeXdmf.cpp

@@ -6,6 +6,7 @@
 #include <spdlog/fmt/bundled/format.h>
 
 #include <algorithm>
+#include <array>
 #include <fstream>
 #include <iostream>
 #include <iterator>
@@ -15,6 +16,7 @@
 #include <string_view>
 #include <vector>
 
+#include "BaseLib/Error.h"
 #include "BaseLib/cpp23.h"
 #include "MeshLib/Location.h"
 
@@ -34,18 +36,14 @@ constexpr char const* mesh_item_type_strings[] = {"Node", "Edge", "Face",
                                                   "Cell", "IntegrationPoint"};
 
 // Transforms MeshItemType into string
-static std::string mesh_item_type_string(
+static std::string meshItemTypeString(
     std::optional<MeshItemType> const& item_type)
 {
-    /// Char array names for all of MeshItemType values.
     if (item_type)
     {
         return mesh_item_type_strings[static_cast<int>(item_type.value())];
     }
-    else
-    {
-        return "";
-    }
+    OGS_FATAL("Cannot convert an empty optional mesh item type.");
 }
 
 // Transforms MeshPropertyDataType into string
@@ -69,8 +67,7 @@ static auto meshPropertyDatatypeString()
 static std::string getPropertyDataTypeString(
     MeshPropertyDataType const& ogs_data_type)
 {
-    auto ogs_to_xdmf_type = meshPropertyDatatypeString();
-    return ogs_to_xdmf_type[to_underlying(ogs_data_type)];
+    return meshPropertyDatatypeString()[to_underlying(ogs_data_type)];
 }
 
 // Returns MeshPropertyDataType-to-Size (in bytes) lookup-table
@@ -89,11 +86,12 @@ static constexpr auto meshPropertyDatatypeSize()
     return property_sizes;
 }
 
+inline auto ogs_to_xdmf_type = meshPropertyDatatypeSize();
+
 // Transform MeshPropertyDataType into type_sizes (in bytes)
 static std::string getPropertyDataTypeSize(
     MeshPropertyDataType const& ogs_data_type)
 {
-    constexpr auto ogs_to_xdmf_type = meshPropertyDatatypeSize();
     return fmt::format("{}", ogs_to_xdmf_type[to_underlying(ogs_data_type)]);
 }
 
@@ -148,7 +146,7 @@ std::function<std::string(std::vector<double>)> write_xdmf(
         return [join, transform](auto const& collection)
         {
             std::vector<std::string> temp;
-            temp.resize(collection.size());
+            temp.reserve(collection.size());
             std::transform(collection.begin(), collection.end(),
                            std::back_inserter(temp), transform);
             return join(temp);
@@ -175,7 +173,7 @@ std::function<std::string(std::vector<double>)> write_xdmf(
             "ElementDegree=\"0\" "
             "ElementFamily=\"\" ItemType=\"\" Name=\"{name}\" "
             "Type=\"None\">{dataitem}\n\t</Attribute>",
-            "center"_a = mesh_item_type_string(attribute.attribute_center),
+            "center"_a = meshItemTypeString(attribute.attribute_center),
             "name"_a = attribute.name,
             "dataitem"_a = dataitem_transform(attribute));
     };
@@ -235,7 +233,7 @@ std::function<std::string(std::vector<double>)> write_xdmf(
                 variable, time_step, max_step, h5filename](XdmfData const& attr)
         {
             // new data arrived
-            bool changed =
+            bool const changed =
                 ((time_step > 0 && (variable == time_attribute::variable)) ||
                  time_step == 0);
             if (changed)
@@ -246,8 +244,8 @@ std::function<std::string(std::vector<double>)> write_xdmf(
             }
             else
             {
-                std::vector<unsigned int> d = {1, 1, 2, 1, attr.index};
-                return pointer_transfrom(d);
+                std::array<unsigned int, 5> position = {1, 1, 2, 1, attr.index};
+                return pointer_transfrom(position);
             };
         };
     };
@@ -297,7 +295,7 @@ std::function<std::string(std::vector<double>)> write_xdmf(
         // c++20 ranges zip missing
         auto const max_step = times.size();
         std::vector<std::string> grids;
-        grids.resize(max_step);
+        grids.reserve(max_step);
         for (size_t time_step = 0; time_step < max_step; ++time_step)
         {
             grids.push_back(time_grid_transform(
@@ -312,15 +310,16 @@ std::function<std::string(std::vector<double>)> write_xdmf(
 
     // Generator function that return a function that represents complete xdmf
     // structure
-    auto const xdmf_writer_fn =
-        [temporal_grid_collection_transform](
-            auto const& ogs_version, auto const& geometry, auto const& topology,
-            auto const& constant_attributes, auto const& variable_attributes)
+    auto const xdmf_writer_fn = [temporal_grid_collection_transform](
+                                    auto ogs_version, auto geometry,
+                                    auto topology, auto constant_attributes,
+                                    auto variable_attributes)
     {
         // This function will be the return of the surrounding named function
         // capture by copy - it is the function that will survive this scope!!
         // Use generalized lambda capture to move for optimization
-        return [temporal_grid_collection_transform, ogs_version,
+        return [temporal_grid_collection_transform,
+                ogs_version = std::move(ogs_version),
                 geometry = std::move(geometry), topology = std::move(topology),
                 constant_attributes = std::move(constant_attributes),
                 variable_attributes = std::move(variable_attributes)](

MeshLib/IO/writeMeshToFile.cpp

@@ -50,7 +50,7 @@ int writeMeshToFile(const MeshLib::Mesh& mesh,
         return 0;
     }
     ERR("writeMeshToFile(): Unknown file extension '{:s}'. Can not write file "
-        "'{'s}'.",
+        "'{:s}'.",
         file_path.extension().string(), file_path.string());
     return 0;
 }

ProcessLib/LiquidFlow/Tests.cmake

@@ -482,9 +482,7 @@ AddTest(
     WRAPPER time
     TESTER xdmfdiff
     # See https://gitlab.opengeosys.org/ogs/ogs/-/merge_requests/3184#note_85104
-    # TODO: re-enable this, see #3177.
-    # REQUIREMENTS NOT OGS_USE_MPI AND NOT COMPILER_IS_APPLE_CLANG
-    REQUIREMENTS OFF
+    REQUIREMENTS NOT OGS_USE_MPI AND NOT COMPILER_IS_APPLE_CLANG
     DIFF_DATA
     square_5x5_tris_32.xdmf square_5x5_tris_32.xdmf pressure pressure 1e-7 1e-13
     square_5x5_tris_32.xdmf square_5x5_tris_32.xdmf HydraulicFlow HydraulicFlow 1e-7 1e-13

ProcessLib/Output/Output.cpp

@@ -179,7 +179,8 @@ struct Output::OutputFile
                std::string const& prefix, std::string const& suffix,
                std::string const& mesh_name, int const timestep, double const t,
                int const iteration, int const data_mode_,
-               bool const compression_, std::set<std::string> const& outputnames)
+               bool const compression_,
+               std::set<std::string> const& outputnames)
         : name(constructFilename(type, prefix, suffix, mesh_name, timestep, t,
                                  iteration)),
           path(BaseLib::joinPaths(directory, name)),
@@ -307,7 +308,8 @@ void Output::doOutputAlways(Process const& process,
         return;
     }
 
-    auto output_bulk_mesh = [&](MeshLib::Mesh const& mesh) {
+    auto output_bulk_mesh = [&](MeshLib::Mesh const& mesh)
+    {
         MeshLib::IO::PVDFile* pvd_file = nullptr;
         if (_output_file_type == ProcessLib::OutputType::vtk)
         {
@@ -322,7 +324,6 @@ void Output::doOutputAlways(Process const& process,
         }
         else if (_output_file_type == ProcessLib::OutputType::xdmf)
         {
-
             OutputFile const file(
                 _output_directory, _output_file_type, _output_file_prefix, "",
                 mesh.getName(), timestep, t, iteration, _output_file_data_mode,
@@ -330,9 +331,6 @@ void Output::doOutputAlways(Process const& process,
                 _output_data_specification.output_variables);
 
             outputMeshXdmf(file, mesh, timestep, t);
-
-
-
         }
     };
 
@@ -347,9 +345,8 @@ void Output::doOutputAlways(Process const& process,
         // mesh related output
         auto& non_bulk_mesh = *BaseLib::findElementOrError(
             begin(_meshes), end(_meshes),
-            [&mesh_output_name](auto const& m) {
-                return m->getName() == mesh_output_name;
-            },
+            [&mesh_output_name](auto const& m)
+            { return m->getName() == mesh_output_name; },
             "Need mesh '" + mesh_output_name + "' for the output.");
 
         std::vector<MeshLib::Node*> const& nodes = non_bulk_mesh.getNodes();
@@ -370,9 +367,8 @@ void Output::doOutputAlways(Process const& process,
         mesh_dof_table_pointers.reserve(mesh_dof_tables.size());
         transform(cbegin(mesh_dof_tables), cend(mesh_dof_tables),
                   back_inserter(mesh_dof_table_pointers),
-                  [](std::unique_ptr<NumLib::LocalToGlobalIndexMap> const& p) {
-                      return p.get();
-                  });
+                  [](std::unique_ptr<NumLib::LocalToGlobalIndexMap> const& p)
+                  { return p.get(); });
 
         output_secondary_variable = false;
         addProcessDataToMesh(