Add node-local-aware EnsureDirectory helper

cmake/ExternalMFEM.cmake

@@ -400,6 +400,7 @@ set(MFEM_PATCH_FILES
   "${CMAKE_SOURCE_DIR}/extern/patch/mfem/patch_gmsh_parser_performance.diff"
   "${CMAKE_SOURCE_DIR}/extern/patch/mfem/mfem_pr5246.diff"
   "${CMAKE_SOURCE_DIR}/extern/patch/mfem/mfem_pr5353.diff"
+  "${CMAKE_SOURCE_DIR}/extern/patch/mfem/mfem_nodelocal_dirs.diff"
 )
 
 include(ExternalProject)

extern/patch/mfem/mfem_nodelocal_dirs.diff

@@ -0,0 +1,35 @@
+diff --git a/fem/datacollection.cpp b/fem/datacollection.cpp
+index a7abd160fa..64ec8f8f31 100644
+--- a/fem/datacollection.cpp
++++ b/fem/datacollection.cpp
+@@ -41,6 +41,21 @@ int DataCollection::create_directory(const std::string &dir_name,
+    int err_flag;
+ #ifdef MFEM_USE_MPI
+    const ParMesh *pmesh = dynamic_cast<const ParMesh*>(mesh);
++   // In addition to the global root, let the lowest rank on each shared-memory
++   // node create the directory too, so that node-local (non-shared) filesystems
++   // get it on every node rather than only where the global root lives. On a
++   // shared filesystem the extra mkdir() hits EEXIST and is tolerated below.
++   bool node_root = true;
++   if (pmesh)
++   {
++      MPI_Comm node_comm;
++      MPI_Comm_split_type(pmesh->GetComm(), MPI_COMM_TYPE_SHARED, myid,
++                          MPI_INFO_NULL, &node_comm);
++      int node_rank;
++      MPI_Comm_rank(node_comm, &node_rank);
++      node_root = (node_rank == 0);
++      MPI_Comm_free(&node_comm);
++   }
+ #endif
+
+    do
+@@ -52,7 +67,7 @@ int DataCollection::create_directory(const std::string &dir_name,
+       err_flag = mkdir(subdir.c_str(), 0777);
+       err_flag = (err_flag && (errno != EEXIST)) ? 1 : 0;
+ #else
+-      if (myid == 0 || pmesh == NULL)
++      if (myid == 0 || node_root || pmesh == NULL)
+       {
+          err_flag = mkdir(subdir.c_str(), 0777);
+          err_flag = (err_flag && (errno != EEXIST)) ? 1 : 0;

palace/models/postoperator.cpp

@@ -23,6 +23,7 @@
 #include "utils/constants.hpp"
 #include "utils/geodata.hpp"
 #include "utils/iodata.hpp"
+#include "utils/outputdir.hpp"
 #include "utils/tablecsv.hpp"
 #include "utils/timer.hpp"
 
@@ -806,11 +807,8 @@ void PostOperator<solver_t>::WriteMFEMGridFunctions(double time, int step)
 {
   BlockTimer bt(Timer::POSTPRO_GRIDFUNCTION);
 
-  // Create output directory if it doesn't exist.
-  if (Mpi::Root(fem_op->GetComm()))
-  {
-    fs::create_directories(gridfunction_output_dir);
-  }
+  // Create output directory if it doesn't exist (node-local filesystem aware).
+  EnsureDirectory(fs::path(gridfunction_output_dir), fem_op->GetComm());
 
   auto mesh_Lc0 = units.GetMeshLengthRelativeScale();
 
@@ -980,11 +978,8 @@ void PostOperator<solver_t>::WriteMFEMGridFunctionsFinal(const ErrorIndicator *i
   mfem::ParMesh &mesh = E ? *E->ParFESpace()->GetParMesh() : *B->ParFESpace()->GetParMesh();
   mesh::DimensionalizeMesh(mesh, mesh_Lc0);
 
-  // Create output directory if it doesn't exist.
-  if (Mpi::Root(fem_op->GetComm()))
-  {
-    fs::create_directories(gridfunction_output_dir);
-  }
+  // Create output directory if it doesn't exist (node-local filesystem aware).
+  EnsureDirectory(fs::path(gridfunction_output_dir), fem_op->GetComm());
 
   // Create piecewise constant finite element space for rank and error indicator.
   mfem::L2_FECollection pwconst_fec(0, mesh.Dimension());

palace/utils/outputdir.hpp

@@ -4,8 +4,8 @@
 #ifndef PALACE_UTILS_OUTPUTDIR_HPP
 #define PALACE_UTILS_OUTPUTDIR_HPP
 
+#include <system_error>
 #include <fmt/format.h>
-#include <fmt/os.h>
 #include "communication.hpp"
 #include "filesystem.hpp"
 #include "iodata.hpp"
@@ -14,10 +14,49 @@
 namespace palace
 {
 
+// Ensure an output directory exists on every node's filesystem. This is
+// node-local-filesystem aware: on a shared filesystem only the global root creates the
+// directory (all other creations hit an already-existing directory harmlessly), while on
+// a node-local filesystem each node's root process creates the node's own copy. The code
+// never classifies the filesystem topology; it only guarantees the invariant "the
+// directory exists here."
+inline void EnsureDirectory(const fs::path &dir, MPI_Comm comm)
+{
+  BlockTimer bt(Timer::IO);
+  // Global root creates the directory. Use the non-throwing overload and ignore both the
+  // error code and the bool return: neither is a reliable success signal (the bool is
+  // false for an already-existing directory, and a benign EEXIST race may populate the
+  // error code). A genuine write failure surfaces at the first real write.
+  if (Mpi::Root(comm))
+  {
+    std::error_code ec;
+    fs::create_directories(dir, ec);
+  }
+  Mpi::Barrier(comm);
+
+  // Split off a node-local communicator so one process per node can ensure the directory
+  // exists on that node's filesystem.
+  MPI_Comm node_comm = MPI_COMM_NULL;
+  MPI_Comm_split_type(comm, MPI_COMM_TYPE_SHARED, Mpi::Rank(comm), MPI_INFO_NULL,
+                      &node_comm);
+
+  // Each node-root that is not the global root creates the directory on its node
+  // (harmless no-op on a shared filesystem, a real create on a node-local filesystem).
+  // This metadata operation also forces per-node cache coherence.
+  if (Mpi::Root(node_comm) && !Mpi::Root(comm))
+  {
+    std::error_code ec;
+    fs::create_directories(dir, ec);
+  }
+
+  MPI_Comm_free(&node_comm);
+  Mpi::Barrier(comm);
+}
+
 inline void MakeOutputFolder(IoData &iodata, MPI_Comm &comm)
 {
   BlockTimer bt(Timer::IO);
-  // Validate and make folder on root.
+  // Validate output directory name and warn about overwrites on root.
   auto root = Mpi::Root(comm);
   auto &output_str = iodata.problem.output;
   if (root)
@@ -30,14 +69,8 @@ inline void MakeOutputFolder(IoData &iodata, MPI_Comm &comm)
       output_str.erase(output_str.end() - 1);
     }
     auto output_path = fs::path(output_str);
-    // Make folder if it does not exist.
-    if (!fs::exists(output_path))
-    {
-      MFEM_VERIFY(fs::create_directories(output_path),
-                  fmt::format("Error std::filesystem could not create a directory at {}",
-                              output_path.string()));
-    }
-    else
+    // Warn if the folder already exists and is not empty; content will be overwritten.
+    if (fs::exists(output_path))
     {
       MFEM_VERIFY(fs::is_directory(output_path),
                   fmt::format("Output path already exists but is not a directory: {}",
@@ -48,18 +81,6 @@ inline void MakeOutputFolder(IoData &iodata, MPI_Comm &comm)
                      output_path.string());
       }
     }
-    // Ensure we can write to folder by making test file.
-    {
-      fs::path tmp_ = output_path / "tmp_test_file.txt";
-      auto file_buf = fmt::output_file(
-          tmp_.string(), fmt::file::WRONLY | fmt::file::CREATE | fmt::file::TRUNC);
-      file_buf.print("Test Print");
-      file_buf.close();
-      MFEM_VERIFY(
-          fs::exists(tmp_) && fs::is_regular_file(tmp_),
-          fmt::format("Error creating test file in output folder: {}", tmp_.string()));
-      fs::remove(tmp_);
-    }
     output_str = output_path.string();
   }
 
@@ -76,7 +97,10 @@ inline void MakeOutputFolder(IoData &iodata, MPI_Comm &comm)
     output_str.resize(str_len);
     Mpi::Broadcast(str_len, output_str.data(), 0, comm);
   }
+
+  // Create the output directory (node-local aware).
+  EnsureDirectory(fs::path(output_str), comm);
 }
 
 }  // namespace palace
-#endif  // PALACE_UTILS_OUTPUTDIR_HPP
+#endif  // PALACE_UTILS_OUTPUTDIR_HPP

spack_repo/local/packages/palace/mfem_nodelocal_dirs.diff

@@ -0,0 +1 @@
+../../../../extern/patch/mfem/mfem_nodelocal_dirs.diff

spack_repo/local/packages/palace/package.py

@@ -183,6 +183,9 @@ class Palace(CMakePackage, CudaPackage, ROCmPackage):
                 patch("mfem_pr5246.diff", when="@:4.9"),
                 # mfem PR #5353; remove once merged upstream and mfem is bumped.
                 "mfem_pr5353.diff",
+                # Node-local-aware output directory creation; carried locally,
+                # shaped for possible upstream submission.
+                "mfem_nodelocal_dirs.diff",
             ],
         )
         depends_on("mfem+shared", when="+shared")

test/unit/CMakeLists.txt

@@ -41,6 +41,7 @@ add_executable(unit-tests
   ${CMAKE_CURRENT_SOURCE_DIR}/test-memoryreporting.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test-nondimensionalize.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test-orthog.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test-outputdir.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test-postoperator.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test-postoperatorcsv.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test-rap.cpp

test/unit/test-outputdir.cpp

@@ -0,0 +1,135 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+#include <fstream>
+#include <string>
+#include <catch2/catch_test_macros.hpp>
+#include "fixtures.hpp"
+#include "utils/communication.hpp"
+#include "utils/filesystem.hpp"
+#include "utils/outputdir.hpp"
+
+using namespace palace;
+
+// The core-logic cases are tagged [Serial][Parallel] so they run in both the
+// single-rank and the multi-rank sweeps. In the parallel sweep they additionally
+// verify that the collective (barrier / split / free) is balanced across ranks
+// and does not deadlock, and that a directory created on the (shared) filesystem
+// is visible to every rank afterwards.
+
+TEST_CASE_METHOD(palace::test::SharedTempDir, "EnsureDirectory creates a new directory",
+                 "[outputdir][Serial][Parallel]")
+{
+  MPI_Comm comm = MPI_COMM_WORLD;
+  auto dir = temp_dir / "postpro";
+  REQUIRE(!fs::exists(dir));
+
+  EnsureDirectory(dir, comm);
+
+  CHECK(fs::is_directory(dir));
+  // The directory must be writable from this rank. Use a rank-unique filename so
+  // ranks sharing the directory (parallel sweep, shared filesystem) do not race
+  // on the same path.
+  auto written = dir / ("written_" + std::to_string(Mpi::Rank(comm)) + ".txt");
+  {
+    std::ofstream f(written);
+    f << "ok";
+  }
+  CHECK(fs::is_regular_file(written));
+}
+
+TEST_CASE_METHOD(palace::test::SharedTempDir,
+                 "EnsureDirectory succeeds on a pre-existing directory",
+                 "[outputdir][Serial][Parallel]")
+{
+  MPI_Comm comm = MPI_COMM_WORLD;
+  auto dir = temp_dir / "postpro";
+  // Pre-create the directory on the (shared) filesystem before the call.
+  if (Mpi::Root(comm))
+  {
+    fs::create_directories(dir);
+  }
+  Mpi::Barrier(comm);
+  REQUIRE(fs::is_directory(dir));
+
+  // Regression guard: EnsureDirectory must NOT treat an already-existing
+  // directory as an error. It relies on the non-throwing create_directories
+  // overload and ignores the bool return, so a pre-existing directory returns
+  // cleanly rather than aborting.
+  EnsureDirectory(dir, comm);
+
+  CHECK(fs::is_directory(dir));
+}
+
+TEST_CASE_METHOD(palace::test::SharedTempDir,
+                 "EnsureDirectory preserves existing directory contents",
+                 "[outputdir][Serial][Parallel]")
+{
+  MPI_Comm comm = MPI_COMM_WORLD;
+  auto dir = temp_dir / "postpro";
+  if (Mpi::Root(comm))
+  {
+    fs::create_directories(dir);
+    std::ofstream f(dir / "existing.txt");
+    f << "keep me";
+  }
+  Mpi::Barrier(comm);
+
+  EnsureDirectory(dir, comm);
+
+  // EnsureDirectory only ensures the directory exists; it must not wipe content.
+  CHECK(fs::is_regular_file(dir / "existing.txt"));
+  {
+    std::ifstream f(dir / "existing.txt");
+    std::string content{std::istreambuf_iterator<char>(f),
+                        std::istreambuf_iterator<char>()};
+    CHECK(content == "keep me");
+  }
+}
+
+TEST_CASE_METHOD(palace::test::SharedTempDir, "EnsureDirectory creates nested directories",
+                 "[outputdir][Serial][Parallel]")
+{
+  MPI_Comm comm = MPI_COMM_WORLD;
+  auto dir = temp_dir / "a" / "b" / "c";
+  REQUIRE(!fs::exists(dir));
+
+  EnsureDirectory(dir, comm);
+
+  CHECK(fs::is_directory(dir));
+}
+
+TEST_CASE_METHOD(palace::test::SharedTempDir, "EnsureDirectory is idempotent",
+                 "[outputdir][Serial][Parallel]")
+{
+  MPI_Comm comm = MPI_COMM_WORLD;
+  auto dir = temp_dir / "postpro";
+
+  EnsureDirectory(dir, comm);
+  // A second call on the now-existing directory must also succeed.
+  EnsureDirectory(dir, comm);
+
+  CHECK(fs::is_directory(dir));
+}
+
+// Node-local-filesystem simulation: with MPI_COMM_SELF every rank runs the full
+// creation path independently on its own (distinct) directory, as it would when
+// each node owns its filesystem. This exercises the per-node creation path on
+// every rank -- which the MPI_COMM_WORLD cases above cannot on a single physical
+// machine, where MPI_COMM_TYPE_SHARED groups all ranks into one node.
+TEST_CASE_METHOD(palace::test::PerRankTempDir,
+                 "EnsureDirectory creates a per-rank directory on MPI_COMM_SELF",
+                 "[outputdir][Serial][Parallel]")
+{
+  auto dir = temp_dir / "postpro";
+  REQUIRE(!fs::exists(dir));
+
+  EnsureDirectory(dir, MPI_COMM_SELF);
+
+  CHECK(fs::is_directory(dir));
+  {
+    std::ofstream f(dir / "written.txt");
+    f << "ok";
+  }
+  CHECK(fs::is_regular_file(dir / "written.txt"));
+}