From 8d5f83633b8bec76c5d288e9ef6320e6f02b44ef Mon Sep 17 00:00:00 2001 From: Tyler Sax Date: Thu, 28 May 2026 11:34:55 -0500 Subject: [PATCH 1/2] phys/parameters: throw on missing output directory (issue #300 bug #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-existent output.directory previously surfaced only as a cryptic HDF5 crash. Add OutputParameters::validate(), called from Parameters::readInput on first rank only, which throws std::invalid_argument with a clear message naming the missing directory. Validation is kept separate from OutputParameters::readWrite so the parsing path stays disk-free and unit-testable; this also avoids needing the existing ReadAll fixture to point at a real path. Follow-up PRs for bugs #2–#4 are expected to add validate() to other parameter sections under the same convention. --- .../dca/phys/parameters/output_parameters.hpp | 12 ++++++++ include/dca/phys/parameters/parameters.hpp | 1 + test/unit/phys/parameters/CMakeLists.txt | 1 + .../input_validation/CMakeLists.txt | 5 ++++ .../input_validation_test.cpp | 29 +++++++++++++++++++ .../input_validation/missing_directory.json | 5 ++++ 6 files changed, 53 insertions(+) create mode 100644 test/unit/phys/parameters/input_validation/CMakeLists.txt create mode 100644 test/unit/phys/parameters/input_validation/input_validation_test.cpp create mode 100644 test/unit/phys/parameters/input_validation/missing_directory.json diff --git a/include/dca/phys/parameters/output_parameters.hpp b/include/dca/phys/parameters/output_parameters.hpp index 864a52421..c1b870074 100644 --- a/include/dca/phys/parameters/output_parameters.hpp +++ b/include/dca/phys/parameters/output_parameters.hpp @@ -14,6 +14,7 @@ #ifndef DCA_PHYS_PARAMETERS_OUTPUT_PARAMETERS_HPP #define DCA_PHYS_PARAMETERS_OUTPUT_PARAMETERS_HPP +#include #include #include #include "dca/io/io_types.hpp" @@ -64,6 +65,12 @@ class OutputParameters { template void readWrite(ReaderOrWriter& reader_or_writer); + // Throws std::invalid_argument if the parsed parameters describe state that cannot be + // used in a run (e.g. the configured output directory does not exist on disk). Intended + // to be called by Parameters::readInput on the rank that parsed the input file. + // Issue #300: surface bad input at parse time instead of crashing later in HDF5. + void validate() const; + const std::string& get_directory() const { return directory_; } @@ -272,6 +279,11 @@ void OutputParameters::readWrite(ReaderOrWriter& reader_or_writer) { } } +inline void OutputParameters::validate() const { + if (!std::filesystem::exists(directory_)) + throw std::invalid_argument("Output directory does not exist: " + directory_); +} + } // namespace params } // namespace phys } // namespace dca diff --git a/include/dca/phys/parameters/parameters.hpp b/include/dca/phys/parameters/parameters.hpp index 56c4be6ff..57636ab0d 100644 --- a/include/dca/phys/parameters/parameters.hpp +++ b/include/dca/phys/parameters/parameters.hpp @@ -281,6 +281,7 @@ void ParametersreadWrite(read_obj); read_obj.close_file(); + OutputParameters::validate(); } } diff --git a/test/unit/phys/parameters/CMakeLists.txt b/test/unit/phys/parameters/CMakeLists.txt index 066c09b83..fa214ce5f 100644 --- a/test/unit/phys/parameters/CMakeLists.txt +++ b/test/unit/phys/parameters/CMakeLists.txt @@ -4,6 +4,7 @@ add_subdirectory(domains_parameters) add_subdirectory(double_counting_parameters) add_subdirectory(ed_solver_parameters) add_subdirectory(four_point_parameters) +add_subdirectory(input_validation) add_subdirectory(mc_solver_parameters) add_subdirectory(mci_parameters) add_subdirectory(model_parameters) diff --git a/test/unit/phys/parameters/input_validation/CMakeLists.txt b/test/unit/phys/parameters/input_validation/CMakeLists.txt new file mode 100644 index 000000000..f0a8dc0dd --- /dev/null +++ b/test/unit/phys/parameters/input_validation/CMakeLists.txt @@ -0,0 +1,5 @@ +# Input validation unit tests (issue #300) + +dca_add_gtest(input_validation_test + GTEST_MAIN + LIBS dca_io json) diff --git a/test/unit/phys/parameters/input_validation/input_validation_test.cpp b/test/unit/phys/parameters/input_validation/input_validation_test.cpp new file mode 100644 index 000000000..9519cf054 --- /dev/null +++ b/test/unit/phys/parameters/input_validation/input_validation_test.cpp @@ -0,0 +1,29 @@ +// Copyright (C) 2026 ETH Zurich +// Copyright (C) 2026 UT-Battelle, LLC +// All rights reserved. +// +// See LICENSE for terms of usage. +// See CITATION.md for citation guidelines, if DCA++ is used for scientific publications. +// +// Tests for issue #300: surface bad user input instead of silently substituting defaults. + +#include "dca/config/haves_defines.hpp" +#include "dca/phys/parameters/output_parameters.hpp" +#include "dca/testing/gtest_h_w_warning_blocking.h" +#include "dca/io/json/json_reader.hpp" + +// Bug #1: a non-existent output.directory should produce a clear error at parameter-read +// time, not a cryptic HDF5 crash later in the run. OutputParameters::readWrite parses the +// directory string without touching the disk; the new validate() method is where the +// existence check lives and where the throw is expected to come from. +TEST(InputValidationTest, MissingOutputDirectoryThrows) { + dca::io::JSONReader reader; + dca::phys::params::OutputParameters pars; + + reader.open_file(DCA_SOURCE_DIR + "/test/unit/phys/parameters/input_validation/missing_directory.json"); + pars.readWrite(reader); + reader.close_file(); + + EXPECT_THROW(pars.validate(), std::invalid_argument); +} diff --git a/test/unit/phys/parameters/input_validation/missing_directory.json b/test/unit/phys/parameters/input_validation/missing_directory.json new file mode 100644 index 000000000..1ff57317f --- /dev/null +++ b/test/unit/phys/parameters/input_validation/missing_directory.json @@ -0,0 +1,5 @@ +{ + "output": { + "directory": "/nonexistent/path/for/dca_input_validation_test" + } +} From bcfaecd1e457842ec0679df81f115c790b883368 Mon Sep 17 00:00:00 2001 From: Tyler Sax Date: Fri, 29 May 2026 13:03:29 -0500 Subject: [PATCH 2/2] move missing-directory test into output_parameters_test --- test/unit/phys/parameters/CMakeLists.txt | 1 - .../input_validation/CMakeLists.txt | 5 ---- .../input_validation_test.cpp | 29 ------------------- .../missing_directory.json | 0 .../output_parameters_test.cpp | 17 +++++++++++ 5 files changed, 17 insertions(+), 35 deletions(-) delete mode 100644 test/unit/phys/parameters/input_validation/CMakeLists.txt delete mode 100644 test/unit/phys/parameters/input_validation/input_validation_test.cpp rename test/unit/phys/parameters/{input_validation => output_parameters}/missing_directory.json (100%) diff --git a/test/unit/phys/parameters/CMakeLists.txt b/test/unit/phys/parameters/CMakeLists.txt index fa214ce5f..066c09b83 100644 --- a/test/unit/phys/parameters/CMakeLists.txt +++ b/test/unit/phys/parameters/CMakeLists.txt @@ -4,7 +4,6 @@ add_subdirectory(domains_parameters) add_subdirectory(double_counting_parameters) add_subdirectory(ed_solver_parameters) add_subdirectory(four_point_parameters) -add_subdirectory(input_validation) add_subdirectory(mc_solver_parameters) add_subdirectory(mci_parameters) add_subdirectory(model_parameters) diff --git a/test/unit/phys/parameters/input_validation/CMakeLists.txt b/test/unit/phys/parameters/input_validation/CMakeLists.txt deleted file mode 100644 index f0a8dc0dd..000000000 --- a/test/unit/phys/parameters/input_validation/CMakeLists.txt +++ /dev/null @@ -1,5 +0,0 @@ -# Input validation unit tests (issue #300) - -dca_add_gtest(input_validation_test - GTEST_MAIN - LIBS dca_io json) diff --git a/test/unit/phys/parameters/input_validation/input_validation_test.cpp b/test/unit/phys/parameters/input_validation/input_validation_test.cpp deleted file mode 100644 index 9519cf054..000000000 --- a/test/unit/phys/parameters/input_validation/input_validation_test.cpp +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (C) 2026 ETH Zurich -// Copyright (C) 2026 UT-Battelle, LLC -// All rights reserved. -// -// See LICENSE for terms of usage. -// See CITATION.md for citation guidelines, if DCA++ is used for scientific publications. -// -// Tests for issue #300: surface bad user input instead of silently substituting defaults. - -#include "dca/config/haves_defines.hpp" -#include "dca/phys/parameters/output_parameters.hpp" -#include "dca/testing/gtest_h_w_warning_blocking.h" -#include "dca/io/json/json_reader.hpp" - -// Bug #1: a non-existent output.directory should produce a clear error at parameter-read -// time, not a cryptic HDF5 crash later in the run. OutputParameters::readWrite parses the -// directory string without touching the disk; the new validate() method is where the -// existence check lives and where the throw is expected to come from. -TEST(InputValidationTest, MissingOutputDirectoryThrows) { - dca::io::JSONReader reader; - dca::phys::params::OutputParameters pars; - - reader.open_file(DCA_SOURCE_DIR - "/test/unit/phys/parameters/input_validation/missing_directory.json"); - pars.readWrite(reader); - reader.close_file(); - - EXPECT_THROW(pars.validate(), std::invalid_argument); -} diff --git a/test/unit/phys/parameters/input_validation/missing_directory.json b/test/unit/phys/parameters/output_parameters/missing_directory.json similarity index 100% rename from test/unit/phys/parameters/input_validation/missing_directory.json rename to test/unit/phys/parameters/output_parameters/missing_directory.json diff --git a/test/unit/phys/parameters/output_parameters/output_parameters_test.cpp b/test/unit/phys/parameters/output_parameters/output_parameters_test.cpp index a8f19a75b..7a9ac7cfb 100644 --- a/test/unit/phys/parameters/output_parameters/output_parameters_test.cpp +++ b/test/unit/phys/parameters/output_parameters/output_parameters_test.cpp @@ -12,6 +12,8 @@ // TODO: Add tests for get_buffer_size, pack, unpack and writing. +#include + #include "dca/config/haves_defines.hpp" #include "dca/phys/parameters/output_parameters.hpp" #include "dca/testing/gtest_h_w_warning_blocking.h" @@ -88,3 +90,18 @@ TEST(OutputParametersTest, ReadAll) { EXPECT_TRUE(pars.dump_Gamma_lattice()); EXPECT_TRUE(pars.dump_chi_0_lattice()); } + +// Issue #300, bug #1: a non-existent output.directory should produce a clear error at +// parameter-read time. readWrite() parses the directory string without touching the +// disk; validate() is where the existence check and the throw live. +TEST(OutputParametersTest, MissingDirectoryThrows) { + dca::io::JSONReader reader; + dca::phys::params::OutputParameters pars; + + reader.open_file(DCA_SOURCE_DIR + "/test/unit/phys/parameters/output_parameters/missing_directory.json"); + pars.readWrite(reader); + reader.close_file(); + + EXPECT_THROW(pars.validate(), std::invalid_argument); +}