From b78fe51f0a541839291d1d575f1bf00573d07e9f Mon Sep 17 00:00:00 2001 From: Tyler Sax Date: Wed, 3 Jun 2026 11:17:37 -0500 Subject: [PATCH 1/2] detect model-section input mistakes (issue #300 bugs #2 & #3) A DCA++ executable is compiled for one model and reads one "*-model" section. Previously a typo'd or missing model section was silently ignored: ModelParameters::readWrite swallows the missing-group exception and the run proceeds on default model parameters. Detect this at parse time, while the parsed tree is still live: * Instrument the JSON reader to track which top-level groups were read. JSONObject gains a mutable accessed_ flag (set on successful getGroup); JSONGroup::childGroupAccess() / JSONReader:: topLevelGroupAccess() report {name, accessed} as ChildGroupStatus. getGroup also switched from operator[] to find(), so a lookup miss no longer inserts a null entry. * Add checkModelSections (model_section_check.hpp), a standalone helper invoked from readInput for the JSON path. It reacts to three cases: - typo / wrong file (model section present, none read) -> throw std::invalid_argument; - multi-model file (built model read, others present) -> warn; - no model section at all -> warn, so model-agnostic uses of Parameters keep working. The check is compile-time guarded to JSONReader, since the HDF5 reader does not implement a model-section input guard (justified, since HDF5 input is, presumably, machine-generated. Tests: reader-level access tracking in json_reader_test, and unit coverage of all checkModelSections branches in a new model_section_check_test. --- include/dca/io/json/details/json_group.hpp | 11 +++ include/dca/io/json/details/json_object.hpp | 13 +++ include/dca/io/json/json_reader.hpp | 8 ++ .../phys/parameters/model_section_check.hpp | 89 +++++++++++++++++++ include/dca/phys/parameters/parameters.hpp | 10 +++ src/io/json/details/json_group.cpp | 25 +++++- test/unit/io/json_reader_test.cpp | 30 +++++++ test/unit/io/model_sections_input.json | 15 ++++ test/unit/phys/parameters/CMakeLists.txt | 1 + .../model_section_check/CMakeLists.txt | 4 + .../model_section_check_test.cpp | 77 ++++++++++++++++ 11 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 include/dca/phys/parameters/model_section_check.hpp create mode 100644 test/unit/io/model_sections_input.json create mode 100644 test/unit/phys/parameters/model_section_check/CMakeLists.txt create mode 100644 test/unit/phys/parameters/model_section_check/model_section_check_test.cpp diff --git a/include/dca/io/json/details/json_group.hpp b/include/dca/io/json/details/json_group.hpp index 394191a5d..84b95a3e7 100644 --- a/include/dca/io/json/details/json_group.hpp +++ b/include/dca/io/json/details/json_group.hpp @@ -22,6 +22,13 @@ namespace dca::io::details { +// A child group together with whether it was ever read. Returned by childGroupAccess() to let +// callers detect input sections the executable silently ignored (issue #300). +struct ChildGroupStatus { + std::string name; + bool accessed; +}; + class JSONGroup : public JSONObject { public: JSONGroup() = default; @@ -30,6 +37,10 @@ class JSONGroup : public JSONObject { JSONGroup* addGroup(const std::string& name); JSONGroup* getGroup(const std::string& name); + // Returns the access state of each direct child that is itself a group, sorted by name. Used to + // report sections of the input that the executable never read (issue #300). + std::vector childGroupAccess() const; + template void addEntry(const std::string& name, const T& val) { const auto it = objects_.find(name); diff --git a/include/dca/io/json/details/json_object.hpp b/include/dca/io/json/details/json_object.hpp index a60994957..ac5f1d242 100644 --- a/include/dca/io/json/details/json_object.hpp +++ b/include/dca/io/json/details/json_object.hpp @@ -23,6 +23,19 @@ class JSONObject { virtual void write(std::ostream& stream, int ident) const = 0; virtual bool read(std::istream& stream) = 0; + + // Tracks whether this object was ever looked up during reading. Used to detect input that the + // executable silently ignores (issue #300). Marking is logically const (it does not change the + // parsed value), hence the mutable flag and const markAccessed(). + bool wasAccessed() const noexcept { + return accessed_; + } + void markAccessed() const noexcept { + accessed_ = true; + } + +private: + mutable bool accessed_ = false; }; } // namespace dca::io::details diff --git a/include/dca/io/json/json_reader.hpp b/include/dca/io/json/json_reader.hpp index 9afbd024b..cb41883a0 100644 --- a/include/dca/io/json/json_reader.hpp +++ b/include/dca/io/json/json_reader.hpp @@ -15,6 +15,7 @@ #include #include #include +#include #include "dca/platform/dca_gpu.h" #include "dca/io/json/details/json_group.hpp" @@ -43,6 +44,13 @@ class JSONReader { // the root group. bool close_group() noexcept; + // Returns the access state of each top-level group of the currently loaded file, sorted by name. + // A group is "accessed" if it was ever opened via open_group. Lets callers detect sections the + // executable silently ignored (issue #300). Must be called before close_file(). + std::vector topLevelGroupAccess() const { + return root_.childGroupAccess(); + } + long getStepCount() { return 0; } std::string get_path() { return {}; } diff --git a/include/dca/phys/parameters/model_section_check.hpp b/include/dca/phys/parameters/model_section_check.hpp new file mode 100644 index 000000000..a95ee2180 --- /dev/null +++ b/include/dca/phys/parameters/model_section_check.hpp @@ -0,0 +1,89 @@ +// 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. +// +// Author: Tyler Sax (tylersax@gmail.com) +// +// Issue #300: surface model-section input mistakes at parse time. +// +// A DCA++ executable is compiled for exactly one model and reads exactly one "*-model" section. +// This standalone helper inspects the top-level groups of a parsed JSON input file (as +// ChildGroupStatus records) and reacts to the three situations: +// * Typo: one or more "*-model" sections are present but none is the one this executable +// reads -- likely a misspelled section name or the wrong input file. Throws +// std::invalid_argument, since the run would otherwise silently use default model parameters. +// * Multi-model file: the executable's section was read, but other "*-model" sections +// are also present (e.g. a shared input file used by several model builds). Warns, since this +// is a legitimate pattern currently used in testing. +// * No model section at all: some non-simulation uses of Parameters (domain/FFT setup) do not +// need model parameters. Warns rather than throwing, so those uses keep working. +// +// Out: out (warnings are written here; defaults to std::cerr) + +#ifndef DCA_PHYS_PARAMETERS_MODEL_SECTION_CHECK_HPP +#define DCA_PHYS_PARAMETERS_MODEL_SECTION_CHECK_HPP + +#include +#include +#include +#include +#include +#include + +#include "dca/io/json/details/json_group.hpp" + +namespace dca { +namespace phys { +namespace params { + +inline void checkModelSections( + const std::vector& top_level_groups, + const std::string& filename, std::ostream& out = std::cerr) { + constexpr std::string_view suffix = "-model"; + + std::vector unused; + bool model_section_read = false; + + for (const auto& [name, accessed] : top_level_groups) { + const bool is_model_section = + name.size() >= suffix.size() && std::equal(suffix.rbegin(), suffix.rend(), name.rbegin()); + if (!is_model_section) + continue; + + if (accessed) + model_section_read = true; + else + unused.push_back(name); + } + + if (!model_section_read) { + if (!unused.empty()) { + // Model sections exist but none matches this executable's model -> typo / wrong file. + std::string message = "Input '" + filename + + "' contains model section(s) ["; + for (std::size_t i = 0; i < unused.size(); ++i) + message += (i ? ", " : "") + unused[i]; + message += + "] but none matches this executable's model. Check for a typo in the model section name."; + throw std::invalid_argument(message); + } + // No model section at all: a non-simulation use of Parameters. Warn, but allow defaults. + out << "Warning: input '" << filename + << "' contains no model section; this executable's model will use default parameters.\n"; + return; + } + + // The executable's model was read; flag any other model sections present but unused. + for (const auto& name : unused) + out << "Warning: input model section '" << name + << "' is not used by this executable and will be ignored.\n"; +} + +} // namespace params +} // namespace phys +} // namespace dca + +#endif // DCA_PHYS_PARAMETERS_MODEL_SECTION_CHECK_HPP diff --git a/include/dca/phys/parameters/parameters.hpp b/include/dca/phys/parameters/parameters.hpp index 57636ab0d..b9bf52383 100644 --- a/include/dca/phys/parameters/parameters.hpp +++ b/include/dca/phys/parameters/parameters.hpp @@ -18,10 +18,14 @@ #include #include +#include #include +#include "dca/io/json/json_reader.hpp" + // its expected that dca::config::McOptions will be provided in some manner before parameters.hpp is // included +#include "dca/phys/parameters/model_section_check.hpp" #include "dca/phys/parameters/num_traits.hpp" #include "dca/function/domains/dmn_0.hpp" #include "dca/phys/parameters/analysis_parameters.hpp" @@ -280,6 +284,12 @@ void ParametersreadWrite(read_obj); + // Detect model-section input mistakes while the parsed tree is still live (before close_file). + // Only the JSON reader tracks which sections were read; HDF5 as input is realistically + // machine-generated, so the access check is not implemented for that path + if constexpr (std::is_same_v) { + checkModelSections(read_obj.topLevelGroupAccess(), filename); + } read_obj.close_file(); OutputParameters::validate(); } diff --git a/src/io/json/details/json_group.cpp b/src/io/json/details/json_group.cpp index 18d0eb0e0..ce1ba5361 100644 --- a/src/io/json/details/json_group.cpp +++ b/src/io/json/details/json_group.cpp @@ -10,6 +10,9 @@ // JSON group. #include "dca/io/json/details/json_group.hpp" + +#include + #include "dca/io/json/details/util.hpp" namespace dca::io::details { @@ -26,7 +29,27 @@ JSONGroup* JSONGroup::addGroup(const std::string& name) { } JSONGroup* JSONGroup::getGroup(const std::string& name) { - return dynamic_cast(objects_[name].get()); + // Note: use find() rather than operator[], which would insert a null entry on a miss. + const auto it = objects_.find(name); + if (it == objects_.end()) + return nullptr; + + auto* group = dynamic_cast(it->second.get()); + if (group) + group->markAccessed(); + return group; +} + +std::vector JSONGroup::childGroupAccess() const { + std::vector child_groups; + for (const auto& [name, object] : objects_) { + if (dynamic_cast(object.get())) + child_groups.push_back({name, object->wasAccessed()}); + } + // objects_ is an unordered_map; sort for deterministic output (e.g. warning messages). + std::sort(child_groups.begin(), child_groups.end(), + [](const ChildGroupStatus& a, const ChildGroupStatus& b) { return a.name < b.name; }); + return child_groups; } void JSONGroup::write(std::ostream& stream, int ident) const { diff --git a/test/unit/io/json_reader_test.cpp b/test/unit/io/json_reader_test.cpp index f7e2658b7..9a7e262c9 100644 --- a/test/unit/io/json_reader_test.cpp +++ b/test/unit/io/json_reader_test.cpp @@ -92,6 +92,36 @@ TEST(ReadTest, All) { EXPECT_EQ(vc, vc_check); } +// Issue #300: the reader must report which top-level sections were read, so callers can detect +// input that the executable silently ignores. +TEST(ReadTest, TopLevelGroupAccessTracking) { + dca::io::JSONReader reader; + reader.open_file(directory + "model_sections_input.json"); + + // Before any access, every top-level group is reported as unread. + auto groups = reader.topLevelGroupAccess(); + // Sorted by name: bilayer-Hubbard-model, domains, single-band-Hubbard-model. + ASSERT_EQ(groups.size(), 3u); + EXPECT_EQ(groups[0].name, "bilayer-Hubbard-model"); + EXPECT_EQ(groups[1].name, "domains"); + EXPECT_EQ(groups[2].name, "single-band-Hubbard-model"); + for (const auto& group : groups) + EXPECT_FALSE(group.accessed) << group.name << " should not be accessed yet"; + + // Open one of them, as the executable's ModelParameters would. + EXPECT_TRUE(reader.open_group("single-band-Hubbard-model")); + reader.close_group(); + + // A failed open (e.g. a typo'd section name) must not mark anything accessed. + EXPECT_FALSE(reader.open_group("single-band-Hubard-model")); + + groups = reader.topLevelGroupAccess(); + ASSERT_EQ(groups.size(), 3u); + EXPECT_FALSE(groups[0].accessed); // bilayer-Hubbard-model: present but never opened + EXPECT_FALSE(groups[1].accessed); // domains: never opened + EXPECT_TRUE(groups[2].accessed); // single-band-Hubbard-model: opened +} + TEST(ReadTest, InvalidInput) { auto test_file = [](const std::string& name, int err_line) { dca::io::JSONReader reader; diff --git a/test/unit/io/model_sections_input.json b/test/unit/io/model_sections_input.json new file mode 100644 index 000000000..0a0396bd5 --- /dev/null +++ b/test/unit/io/model_sections_input.json @@ -0,0 +1,15 @@ +{ + "single-band-Hubbard-model" : { + "t" : 1.0, + "U" : 8.0 + }, + + "bilayer-Hubbard-model" : { + "t" : 1.0, + "U" : 6.0 + }, + + "domains" : { + "vec int" : [1, 2, 3] + } +} diff --git a/test/unit/phys/parameters/CMakeLists.txt b/test/unit/phys/parameters/CMakeLists.txt index 066c09b83..1acf66cc9 100644 --- a/test/unit/phys/parameters/CMakeLists.txt +++ b/test/unit/phys/parameters/CMakeLists.txt @@ -7,5 +7,6 @@ add_subdirectory(four_point_parameters) add_subdirectory(mc_solver_parameters) add_subdirectory(mci_parameters) add_subdirectory(model_parameters) +add_subdirectory(model_section_check) add_subdirectory(output_parameters) add_subdirectory(physics_parameters) diff --git a/test/unit/phys/parameters/model_section_check/CMakeLists.txt b/test/unit/phys/parameters/model_section_check/CMakeLists.txt new file mode 100644 index 000000000..65fb8cc7e --- /dev/null +++ b/test/unit/phys/parameters/model_section_check/CMakeLists.txt @@ -0,0 +1,4 @@ +# Model section check unit test (issue #300) + +dca_add_gtest(model_section_check_test + GTEST_MAIN) diff --git a/test/unit/phys/parameters/model_section_check/model_section_check_test.cpp b/test/unit/phys/parameters/model_section_check/model_section_check_test.cpp new file mode 100644 index 000000000..deada5713 --- /dev/null +++ b/test/unit/phys/parameters/model_section_check/model_section_check_test.cpp @@ -0,0 +1,77 @@ +// 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. +// +// Author: Tyler Sax (tylersax@gmail.com) +// +// Unit tests for checkModelSections (issue #300 bugs #2 and #3). + +#include "dca/phys/parameters/model_section_check.hpp" + +#include +#include +#include +#include + +#include "dca/testing/gtest_h_w_warning_blocking.h" + +using dca::phys::params::checkModelSections; +using Groups = std::vector; + +// The built model's section was read and nothing else is present: no warning, no throw. +TEST(CheckModelSectionsTest, OnlyBuiltModelRead) { + const Groups groups{{"domains", true}, {"single-band-Hubbard-model", true}}; + std::ostringstream warnings; + EXPECT_NO_THROW(checkModelSections(groups, "input.json", warnings)); + EXPECT_TRUE(warnings.str().empty()); +} + +// The built model was read, but other model sections are present and unused -> warn only. +TEST(CheckModelSectionsTest, UnusedModelSectionWarns) { + const Groups groups{{"single-band-Hubbard-model", true}, {"bilayer-Hubbard-model", false}}; + std::ostringstream warnings; + EXPECT_NO_THROW(checkModelSections(groups, "input.json", warnings)); + const std::string out = warnings.str(); + EXPECT_NE(out.find("bilayer-Hubbard-model"), std::string::npos); + EXPECT_NE(out.find("ignored"), std::string::npos); + // The section that was actually used must not be warned about. + EXPECT_EQ(out.find("single-band-Hubbard-model"), std::string::npos); +} + +// Model section(s) present but none matches the built model (typo / wrong file) +// -> throw, even though a model section exists. +TEST(CheckModelSectionsTest, WrongModelSectionThrows) { + const Groups groups{{"bilayer-Hubbard-model", false}, {"domains", true}}; + std::ostringstream warnings; + try { + checkModelSections(groups, "input.json", warnings); + FAIL() << "expected std::invalid_argument"; + } + catch (const std::invalid_argument& e) { + const std::string msg = e.what(); + EXPECT_NE(msg.find("input.json"), std::string::npos); + // The unmatched section is surfaced as a typo hint. + EXPECT_NE(msg.find("bilayer-Hubbard-model"), std::string::npos); + } + // A throw must take precedence over emitting warnings. + EXPECT_TRUE(warnings.str().empty()); +} + +// No model section present at all -> warn (do not throw); model-agnostic uses keep working. +TEST(CheckModelSectionsTest, NoModelSectionPresentWarns) { + const Groups groups{{"domains", true}, {"output", false}}; + std::ostringstream warnings; + EXPECT_NO_THROW(checkModelSections(groups, "input.json", warnings)); + EXPECT_NE(warnings.str().find("no model section"), std::string::npos); +} + +// Sections that merely contain "model" but do not end in "-model" are not model sections. +TEST(CheckModelSectionsTest, SuffixMatchIsExact) { + const Groups groups{{"model-parameters", false}, {"single-band-Hubbard-model", true}}; + std::ostringstream warnings; + EXPECT_NO_THROW(checkModelSections(groups, "input.json", warnings)); + EXPECT_TRUE(warnings.str().empty()); // "model-parameters" is not treated as a model section +} From 121ff15837748bf5c1ec0fda74eb555b99c2e9ec Mon Sep 17 00:00:00 2001 From: Tyler Sax Date: Thu, 4 Jun 2026 15:06:35 -0500 Subject: [PATCH 2/2] test/tp: declare Rashba and FeAs model sections in shared inputs The model-section validation added for issue #300 throws when an input contains *-model sections but none matches the executable's own model. The shared GPU tp-accumulator fixtures input_4x4_complex.json and input_4x4_2_transfer.json carry a section per consuming model, but the Rashba and FeAs builds silently relied on default model parameters and had no section of their own, so the new check failed them in SetUp(). Add the missing sections with values equal to the model defaults so the executables read their own section (warn on the others) and the computed results are unchanged. Co-Authored-By: Claude Opus 4.8 --- .../accumulation/tp/input_4x4_2_transfer.json | 11 +++++++++++ .../accumulation/tp/input_4x4_complex.json | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_2_transfer.json b/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_2_transfer.json index c69c8c13b..607625e54 100644 --- a/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_2_transfer.json +++ b/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_2_transfer.json @@ -37,6 +37,17 @@ "U": 2 }, + "FeAs-model": { + "t1": 0, + "t2": 0, + "t3": 0, + "t4": 0, + "U": 0, + "V": 0, + "J": 0, + "Jp": 0 + }, + "domains": { "real-space-grids": { "cluster": [ diff --git a/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_complex.json b/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_complex.json index e5e5bf705..e1f3e26ef 100644 --- a/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_complex.json +++ b/test/unit/phys/dca_step/cluster_solver/shared_tools/accumulation/tp/input_4x4_complex.json @@ -30,6 +30,13 @@ "phi": 0 }, + "Rashba-Hubbard-model": { + "t": 1, + "h": 0, + "lambda": 0, + "U": 0 + }, + "single-band-Hubbard-model": { "t": 1, "U": 2