From 8c08292bc1bec02eb8748f7c72cd2ad6c43c51d2 Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 13:29:57 -0400 Subject: [PATCH 1/9] First pass at path resolution --- Code.v05-00/defaults/input.yaml | 17 +- Code.v05-00/include/Defaults/Input.hpp | 17 +- .../YamlInputReader/YamlInputReader.hpp | 18 +- .../include/YamlInputReader/YamlPathUtils.hpp | 54 ++++ .../src/YamlInputReader/CMakeLists.txt | 3 +- .../src/YamlInputReader/YamlInputReader.cpp | 55 ++-- .../src/YamlInputReader/YamlPathUtils.cpp | 194 +++++++++++++ Code.v05-00/tests/input-yamls/test1.yaml | 4 +- Code.v05-00/tests/input-yamls/test16.yaml | 4 + Code.v05-00/tests/input-yamls/test8.yaml | 3 + Code.v05-00/tests/test_yamlreader.cpp | 262 +++++++++++++++++- examples/default/input.yaml | 3 - examples/default/run_example.sh | 10 - 13 files changed, 593 insertions(+), 51 deletions(-) create mode 100644 Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp create mode 100644 Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp delete mode 100644 examples/default/input.yaml delete mode 100755 examples/default/run_example.sh diff --git a/Code.v05-00/defaults/input.yaml b/Code.v05-00/defaults/input.yaml index f6e60f499..915145ea3 100644 --- a/Code.v05-00/defaults/input.yaml +++ b/Code.v05-00/defaults/input.yaml @@ -1,13 +1,26 @@ +# Every file system path in this file and in your own input files follows one rule: +# a relative path resolves against the directory of the file that declares it, not +# against the working directory and not against the last file passed with -i. +# This file is compiled into APCEMM and has no directory of its own, so it must hold +# no relative path. The paths it does hold are the '=MISSING=' and '=DEFAULT=' +# sentinels, which mean "unset" and "use the compiled-in data". +# The keys treated as paths are listed in PATH_KEYS, in +# Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp. + SIMULATION MENU: OpenMP Num Threads (positive int): 8 OUTPUT SUBMENU: - Output folder (string): APCEMM_out/ + # Required: APCEMM has no default output folder. A relative path resolves + # against the directory of the file that sets it. + Output folder (string): =MISSING= Overwrite if folder exists (T/F): T # FFT options (for spectral solver) Use threaded FFT (T/F): F FFTW WISDOM SUBMENU: Use FFTW WISDOM (T/F): T - Dir w/ write permission (string): ./ + # Unset: the compiled-in defaults hold no relative path, and './' would have to + # resolve against a directory this file does not have. + Dir w/ write permission (string): =MISSING= # This mostly contains information on background aerosol concentration; Not too relevant for contrail behavior Input background condition (string): =DEFAULT= # All parameters here are overwritten in EMISSION INDICES SUBMENU diff --git a/Code.v05-00/include/Defaults/Input.hpp b/Code.v05-00/include/Defaults/Input.hpp index 583eea0e4..8bb0696c9 100644 --- a/Code.v05-00/include/Defaults/Input.hpp +++ b/Code.v05-00/include/Defaults/Input.hpp @@ -1,14 +1,27 @@ R"====( +# Every file system path in this file and in your own input files follows one rule: +# a relative path resolves against the directory of the file that declares it, not +# against the working directory and not against the last file passed with -i. +# This file is compiled into APCEMM and has no directory of its own, so it must hold +# no relative path. The paths it does hold are the '=MISSING=' and '=DEFAULT=' +# sentinels, which mean "unset" and "use the compiled-in data". +# The keys treated as paths are listed in PATH_KEYS, in +# Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp. + SIMULATION MENU: OpenMP Num Threads (positive int): 8 OUTPUT SUBMENU: - Output folder (string): APCEMM_out/ + # Required: APCEMM has no default output folder. A relative path resolves + # against the directory of the file that sets it. + Output folder (string): =MISSING= Overwrite if folder exists (T/F): T # FFT options (for spectral solver) Use threaded FFT (T/F): F FFTW WISDOM SUBMENU: Use FFTW WISDOM (T/F): T - Dir w/ write permission (string): ./ + # Unset: the compiled-in defaults hold no relative path, and './' would have to + # resolve against a directory this file does not have. + Dir w/ write permission (string): =MISSING= # This mostly contains information on background aerosol concentration; Not too relevant for contrail behavior Input background condition (string): =DEFAULT= # All parameters here are overwritten in EMISSION INDICES SUBMENU diff --git a/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp b/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp index 15a8ecc8a..971873920 100644 --- a/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp +++ b/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp @@ -2,7 +2,9 @@ #define YAMLINPUTREADER_H #include +#include #include +#include #include #include #include "Core/Input.hpp" @@ -20,7 +22,9 @@ namespace YamlInputReader{ "mode were removed, so every PARAMETER MENU entry takes exactly one value. To vary a " "parameter, write one input file per value and start one APCEMM process for each."; - static std::filesystem::path INPUT_FILE_PATH; + // Everything about file system paths in an input file lives in YamlPathUtils.hpp: + // the resolution rule, the set of keys that hold a path, and the readPath accessor + // the menu readers below use. // Merge default and user provided files, returns a merged YAML::Node YAML::Node mergeYamlInputFiles(const vector& filenames); @@ -46,6 +50,17 @@ namespace YamlInputReader{ Vector_1D parseVectorDoubleString(const string paramString, const string paramLocation = ""); vector split(const string str, const string delimiter); + // Case-insensitive compare, used for the option values APCEMM accepts in any case + // (e.g. the EPM type). Shared because both the menu readers and the path checks + // have to agree on what "external" means. + inline bool ichar_equals(char a, char b) { + return std::tolower(static_cast(a)) == + std::tolower(static_cast(b)); + } + inline bool iequals(std::string_view lhs, std::string_view rhs) { + return std::ranges::equal(lhs, rhs, ichar_equals); + } + inline string trim(const string str){ string s = str; //trim left @@ -122,6 +137,5 @@ namespace YamlInputReader{ } vector parseVectorIntString(const string paramString, const string paramLocation = ""); - std::string parseFileSystemPath(std::string str); } #endif diff --git a/Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp b/Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp new file mode 100644 index 000000000..4ef8eadfa --- /dev/null +++ b/Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp @@ -0,0 +1,54 @@ +#ifndef YAMLPATHUTILS_H_ +#define YAMLPATHUTILS_H_ + +#include +#include +#include +#include + +/* +Everything APCEMM knows about file system paths in an input file. + +ONE rule: a relative path resolves against the directory of the file that +declares it. Not the working directory, and not the last file passed on the +command line. The compiled-in defaults hold no relative path at all, so there is +no second rule for them. + +The keys whose value is a path are listed in PATH_KEYS, in YamlPathUtils.cpp. +*/ +namespace YamlInputReader { + // A path leaf as the user wrote it, and the file that declared it. Kept so an + // unresolvable path can be reported with the text the user typed, not only with + // the resolved path they never saw. + struct PathOrigin { + std::string asWritten; + std::string declaredIn; + }; + // Keyed by the full key path, e.g. "SIMULATION MENU -> OUTPUT SUBMENU -> Output folder (string)". + using PathOriginMap = std::map; + + // "unset" and "use the compiled-in data". Neither is a path, so neither is resolved + // nor checked, and a menu reader that needs a real path has to reject them itself. + bool isPathSentinel(const std::string& value); + + // Rewrite every path leaf of one input file to an absolute path, resolving a + // relative one against baseDir, the directory of the file that declares it. + // Records what each path looked like as written into origins. + // yaml-cpp nodes are reference types, so the node is edited in place. + void resolvePathsInPlace(YAML::Node node, const std::filesystem::path& baseDir, + const std::string& sourceFile, PathOriginMap& origins); + + // Report a path that names nothing on disk, once the merge is done. + void checkPathsExist(const YAML::Node& merged, const PathOriginMap& origins); + + // Reject a PATH_KEYS entry that names no scalar leaf of the default tree, and any + // default path that is relative. Neither can happen without a broken repository, + // which is why this runs once per read rather than being a test-only check. + void checkDefaultPaths(const YAML::Node& defaultData); + + // Read a path leaf that resolvePathsInPlace has already made absolute. + // Returns the "=MISSING=" and "=DEFAULT=" sentinels unchanged. + std::string readPath(const YAML::Node& node, const std::string& key); +} + +#endif // YAMLPATHUTILS_H_ diff --git a/Code.v05-00/src/YamlInputReader/CMakeLists.txt b/Code.v05-00/src/YamlInputReader/CMakeLists.txt index 50605471c..f4d10d33b 100644 --- a/Code.v05-00/src/YamlInputReader/CMakeLists.txt +++ b/Code.v05-00/src/YamlInputReader/CMakeLists.txt @@ -1,4 +1,5 @@ set(SRCS - YamlInputReader.cpp) + YamlInputReader.cpp + YamlPathUtils.cpp) add_library(YamlInputReader STATIC ${SRCS}) target_link_libraries(YamlInputReader yaml-cpp::yaml-cpp Core) diff --git a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp index e4036f3ed..82e1b822f 100644 --- a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp @@ -1,14 +1,12 @@ #include "YamlInputReader/YamlInputReader.hpp" +#include "YamlInputReader/YamlPathUtils.hpp" #include "APCEMM.h" #include "Util/ForwardDecl.hpp" #include "Util/YamlUtils.hpp" -#include // std::equal -#include // std::tolower #include #include #include #include -#include // std::string_view #include #include #include @@ -20,16 +18,6 @@ const std::string default_input = ; -bool ichar_equals(char a, char b) { - return std::tolower(static_cast(a)) == - std::tolower(static_cast(b)); -} - -bool iequals(std::string_view lhs, std::string_view rhs) { - return std::ranges::equal(lhs, rhs, ichar_equals); -} - - namespace YamlInputReader{ // Helper function to get all keys from a YAML Map node std::set getYamlKeys(const YAML::Node& node) { @@ -172,10 +160,16 @@ namespace YamlInputReader{ // errors in it, but this is cheap to verify. try { checkNoDuplicateKeysRecursive(defaultData, "the default input.yaml"); + checkDefaultPaths(defaultData); } catch (const std::runtime_error& e) { throw std::runtime_error(std::string(e.what()) + " This is not a problem with your input file: check the state of your APCEMM repository."); } + // What each path looked like before it was resolved, so an error can name the + // text the user wrote and the file they wrote it in. A later file overwrites an + // earlier one, which is what the merge does with the value itself. + PathOriginMap origins; + for (auto filename: filenames) { YAML::Node userData = YAML::LoadFile(filename); @@ -185,10 +179,15 @@ namespace YamlInputReader{ } catch (const std::runtime_error& e) { throw std::runtime_error("Invalid field in YAML input file '" + filename + "': " + e.what()); } - INPUT_FILE_PATH = std::filesystem::path(filename); + // Resolve before merging: this is the only point where the file a path was + // declared in is still known. Resolving after the merge would resolve every + // path against whichever file came last. + resolvePathsInPlace(userData, std::filesystem::path(filename).parent_path(), filename, origins); mergedData = mergeYamlNodes(mergedData, userData); } + checkPathsExist(mergedData, origins); + return mergedData; } @@ -284,7 +283,15 @@ namespace YamlInputReader{ } YAML::Node outputSubmenu = simNode["OUTPUT SUBMENU"]; - std::string outputFolder = parseFileSystemPath(outputSubmenu["Output folder (string)"].as()); + std::string outputFolder = readPath(outputSubmenu, "Output folder (string)"); + // Neither sentinel names a folder, and nothing downstream checks them: Main.cpp + // would create a directory called "=MISSING=". + if (isPathSentinel(outputFolder)) { + throw std::invalid_argument("No output folder set: 'Output folder (string)' under SIMULATION MENU -> " + "OUTPUT SUBMENU is '" + outputFolder + "'. APCEMM has no default output " + "folder, so set one in your input file. A relative path resolves against " + "the directory of the file that sets it."); + } // Ensure path to save directory is terminated by "/" if ( outputFolder.back() != '/' ) {outputFolder = outputFolder + "/";} input.SIMULATION_OUTPUT_FOLDER = outputFolder; @@ -293,9 +300,9 @@ namespace YamlInputReader{ YAML::Node fftwWisdomSubmenu = simNode["FFTW WISDOM SUBMENU"]; input.SIMULATION_USE_FFTW_WISDOM = parseBoolString(fftwWisdomSubmenu["Use FFTW WISDOM (T/F)"].as(), "Use FFTW WISDOM (T/F)"); - input.SIMULATION_DIRECTORY_W_WRITE_PERMISSION = parseFileSystemPath(fftwWisdomSubmenu["Dir w/ write permission (string)"].as()); - input.SIMULATION_INPUT_BACKG_COND = parseFileSystemPath(simNode["Input background condition (string)"].as()); - input.SIMULATION_INPUT_ENG_EI = parseFileSystemPath(simNode["Input engine emissions (string)"].as()); + input.SIMULATION_DIRECTORY_W_WRITE_PERMISSION = readPath(fftwWisdomSubmenu, "Dir w/ write permission (string)"); + input.SIMULATION_INPUT_BACKG_COND = readPath(simNode, "Input background condition (string)"); + input.SIMULATION_INPUT_ENG_EI = readPath(simNode, "Input engine emissions (string)"); YAML::Node saveForwardSubmenu = simNode["SAVE FORWARD RESULTS SUBMENU"]; input.SIMULATION_SAVE_FORWARD = parseBoolString(saveForwardSubmenu["Save forward results (T/F)"].as(), "Save forward results (T/F)"); @@ -328,7 +335,7 @@ namespace YamlInputReader{ throw std::invalid_argument("Invalid EPM type specified in SIMULATION MENU: " + epm); } - input.SIMULATION_EXTERNAL_EPM_NETCDF_FILENAME = simNode["External EPM NetCDF file"].as(); + input.SIMULATION_EXTERNAL_EPM_NETCDF_FILENAME = readPath(simNode, "External EPM NetCDF file"); } void readParamMenu(Input& scenario, const YAML::Node& paramNode){ @@ -386,7 +393,7 @@ namespace YamlInputReader{ void readChemMenu(OptInput& input, const YAML::Node& chemNode){ input.CHEMISTRY_CHEMISTRY = parseBoolString(chemNode["Turn on Chemistry (T/F)"].as(), "Turn on Chemistry (T/F)"); input.CHEMISTRY_HETCHEM = parseBoolString(chemNode["Perform hetero. chem. (T/F)"].as(), "Perform hetero. chem. (T/F)"); - input.CHEMISTRY_JRATE_FOLDER = parseFileSystemPath(chemNode["Photolysis rates folder (string)"].as()); + input.CHEMISTRY_JRATE_FOLDER = readPath(chemNode, "Photolysis rates folder (string)"); } void readAeroMenu(OptInput& input, const YAML::Node& aeroNode){ input.AEROSOL_GRAVSETTLING = parseBoolString(aeroNode["Turn on grav. settling (T/F)"].as(), "Turn on grav. settling (T/F)"); @@ -398,7 +405,7 @@ namespace YamlInputReader{ void readMetMenu(OptInput& input, const YAML::Node& metNode){ YAML::Node metInputSubmenu = metNode["METEOROLOGICAL INPUT SUBMENU"]; input.MET_LOADMET = parseBoolString(metInputSubmenu["Use met. input (T/F)"].as(), "Use met. input (T/F)"); - input.MET_FILENAME = parseFileSystemPath(metInputSubmenu["Met input file path (string)"].as()); + input.MET_FILENAME = readPath(metInputSubmenu, "Met input file path (string)"); input.MET_DT = parseDoubleString(metInputSubmenu["Time series data timestep [hr] (double)"].as(), "Time series data timestep [hr] (double)"); input.MET_LOADTEMP = parseBoolString(metInputSubmenu["Init temp. from met. (T/F)"].as(), "Init temp. from met. (T/F)"); input.MET_TEMPTIMESERIES = parseBoolString(metInputSubmenu["Temp. time series input (T/F)"].as(), "Temp. time series input (T/F)"); @@ -530,10 +537,4 @@ namespace YamlInputReader{ } return vecint; } - - std::string parseFileSystemPath(std::string str){ - if (str == "=MISSING=" || str == "=DEFAULT=") return str; - std::filesystem::path p(str); - return p.is_absolute() ? str : std::filesystem::weakly_canonical(INPUT_FILE_PATH.parent_path() / str).generic_string(); - } } diff --git a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp new file mode 100644 index 000000000..b6ec18f7c --- /dev/null +++ b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp @@ -0,0 +1,194 @@ +#include "YamlInputReader/YamlPathUtils.hpp" +#include "YamlInputReader/YamlInputReader.hpp" // split, parseBoolString, iequals +#include "Util/YamlUtils.hpp" // getScalarKey + +#include +#include +#include +#include +#include +#include + +namespace YamlInputReader{ + // The keys whose value is a file system path. A relative value under one of these + // keys resolves against the directory of the file that declares it. + // The entries are full key paths, built with the same " -> " separator + // validateYamlKeys uses, because a leaf key alone is not unique: "netCDF filename + // format (string)" appears four times, and neither it nor "Inst timeseries file + // (string)" is a path, they are filename patterns. + // Keep this in step with defaults/input.yaml. checkDefaultPaths() rejects an entry + // that names no leaf of the default tree, so a key rename cannot silently break + // resolution, and readPath() throws on a value left relative, so a path key missing + // from this set fails loudly on the first run instead of resolving against the wrong + // directory. + static const std::set PATH_KEYS = { + "SIMULATION MENU -> OUTPUT SUBMENU -> Output folder (string)", + "SIMULATION MENU -> FFTW WISDOM SUBMENU -> Dir w/ write permission (string)", // removed by TASK-56 + "SIMULATION MENU -> Input background condition (string)", + "SIMULATION MENU -> Input engine emissions (string)", + "SIMULATION MENU -> External EPM NetCDF file", + "CHEMISTRY MENU -> Photolysis rates folder (string)", // removed by TASK-56 + "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Met input file path (string)", + }; + + bool isPathSentinel(const std::string& value) { + return value == "=MISSING=" || value == "=DEFAULT="; + } + + // Internal to path handling. Everything the rest of the reader calls is declared in + // YamlPathUtils.hpp. + namespace { + // The resolution rule, in one place: a relative path resolves against baseDir, the + // directory of the file that declares it. Nothing else. + // currentPath is the full key path of the node being walked, which is what + // PATH_KEYS holds, so the recursion carries it and the public entry point does not + // have to expose it. + void resolvePaths(YAML::Node node, const std::filesystem::path& baseDir, + const std::string& sourceFile, PathOriginMap& origins, + const std::string& currentPath) { + if (!node.IsMap()) { + return; + } + for (const auto& it : node) { + const std::string key = getScalarKey(it.first); + const std::string keyPath = currentPath.empty() ? key : currentPath + " -> " + key; + YAML::Node value = it.second; + + if (value.IsMap()) { + resolvePaths(value, baseDir, sourceFile, origins, keyPath); + continue; + } + // Anything that is not a scalar is either a null value, which means "no + // override", or a type error that validateYamlKeys has already rejected. + if (!value.IsScalar() || !PATH_KEYS.contains(keyPath)) { + continue; + } + + const std::string asWritten = value.as(); + if (isPathSentinel(asWritten)) { + continue; + } + // Recorded whether or not the path is rewritten, so an error about an + // absolute path can still name the file that declared it. + origins[keyPath] = PathOrigin{asWritten, sourceFile}; + + const std::filesystem::path written(asWritten); + if (written.is_absolute()) { + continue; + } + // parent_path() is empty when the input file is named without a directory, + // and weakly_canonical() would then leave the result relative. + const std::filesystem::path base = baseDir.empty() ? std::filesystem::current_path() : baseDir; + node[key] = std::filesystem::weakly_canonical(base / written).generic_string(); + } + } + + // Name the path the way the user can act on it: the key, the text they wrote, + // where they wrote it, and what it resolved to. A path that no input file + // declares comes from the compiled-in defaults. + std::string describePath(const std::string& keyPath, const std::string& resolved, + const PathOriginMap& origins) { + const auto it = origins.find(keyPath); + const std::string declaredIn = it == origins.end() + ? "the compiled-in default input.yaml" + : "'" + it->second.declaredIn + "'"; + const std::string asWritten = it == origins.end() ? resolved : it->second.asWritten; + std::string description = "'" + keyPath + "' is set to '" + asWritten + "' in " + declaredIn; + if (asWritten != resolved) { + description += ", which resolves to '" + resolved + "'"; + } + return description; + } + + // Read a leaf by its full key path. Returns an empty string when the path names + // nothing or names a node that is not a scalar. + // The walk is recursive and the node is taken by const reference on purpose: + // "node = node[key]" would not rebind the local node, it would assign the child + // into the tree the node came from, and the non-const Node::operator[] inserts an + // undefined placeholder pair for a missing key while the const one does not. + std::string leafAt(const YAML::Node& node, const std::vector& keys, std::size_t depth) { + if (depth == keys.size()) { + return node.IsScalar() ? node.as() : ""; + } + if (!node.IsMap()) { + return ""; + } + const YAML::Node child = node[keys[depth]]; + if (!child) { + return ""; + } + return leafAt(child, keys, depth + 1); + } + + std::string leafAt(const YAML::Node& node, const std::string& keyPath) { + return leafAt(node, split(keyPath, " -> "), 0); + } + + void requirePathExists(const YAML::Node& merged, const std::string& keyPath, + const PathOriginMap& origins) { + const std::string resolved = leafAt(merged, keyPath); + if (isPathSentinel(resolved) || std::filesystem::exists(resolved)) { + return; + } + throw std::runtime_error("Input file not found: " + describePath(keyPath, resolved, origins) + + ". A relative path is resolved against the directory of the file that " + "sets it, not against the working directory."); + } + } + + // Runs on one input file at a time, before the merge, because the merge is what + // loses track of which file a value came from. + void resolvePathsInPlace(YAML::Node node, const std::filesystem::path& baseDir, + const std::string& sourceFile, PathOriginMap& origins) { + resolvePaths(node, baseDir, sourceFile, origins, ""); + } + + // One pass over the merged tree, so a path is reported while reading the input + // instead of much later, when a component fails to open it. + // A path is only checked when the run actually reads it. The output folder is never + // checked: Main.cpp creates it. + void checkPathsExist(const YAML::Node& merged, const PathOriginMap& origins) { + if (parseBoolString(leafAt(merged, "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Use met. input (T/F)"), + "Use met. input (T/F)")) { + requirePathExists(merged, "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Met input file path (string)", origins); + } + if (iequals(leafAt(merged, "SIMULATION MENU -> EPM type (original/external/new)"), "external")) { + requirePathExists(merged, "SIMULATION MENU -> External EPM NetCDF file", origins); + } + requirePathExists(merged, "SIMULATION MENU -> Input background condition (string)", origins); + requirePathExists(merged, "SIMULATION MENU -> Input engine emissions (string)", origins); + } + + // The defaults are compiled in, so this only fails when the repository is in a bad + // state. It is what keeps PATH_KEYS honest: an entry that names no leaf would stop + // resolving without any other sign, and a relative default would need a second + // resolution rule because the compiled-in defaults have no directory of their own. + void checkDefaultPaths(const YAML::Node& defaultData) { + for (const std::string& keyPath : PATH_KEYS) { + const std::string value = leafAt(defaultData, keyPath); + if (value.empty()) { + throw std::runtime_error("PATH_KEYS names '" + keyPath + "', which is not a scalar leaf of the " + "default input.yaml. Path resolution would silently skip it."); + } + if (!isPathSentinel(value) && !std::filesystem::path(value).is_absolute()) { + throw std::runtime_error("The default input.yaml sets '" + keyPath + "' to the relative path '" + + value + "'. The compiled-in defaults have no directory to resolve against, " + "so they must hold absolute paths or the '=MISSING=' sentinel."); + } + } + } + + // Every path reaches OptInput through this accessor. A value left relative means the + // key is not in PATH_KEYS, so it was never resolved and would be read against the + // working directory. Refusing it here turns that mistake into a clear error on the + // first run instead of a file that is silently looked for in the wrong place. + std::string readPath(const YAML::Node& node, const std::string& key){ + const std::string value = node[key].as(); + if (isPathSentinel(value) || std::filesystem::path(value).is_absolute()) { + return value; + } + throw std::runtime_error("Path '" + key + "' is still relative after reading the input: '" + value + + "'. Add its full key path to PATH_KEYS in YamlPathUtils.cpp so it is " + "resolved against the file that declares it."); + } +} diff --git a/Code.v05-00/tests/input-yamls/test1.yaml b/Code.v05-00/tests/input-yamls/test1.yaml index 2349c5d92..569e8668c 100644 --- a/Code.v05-00/tests/input-yamls/test1.yaml +++ b/Code.v05-00/tests/input-yamls/test1.yaml @@ -9,8 +9,8 @@ SIMULATION MENU: FFTW WISDOM SUBMENU: Use FFTW WISDOM (T/F): T Dir w/ write permission (string): ./ - Input background condition (string): ../../input_data/init.txt - Input engine emissions (string): ../../input_data/ENG_EI.txt + Input background condition (string): ../../../input_data/init.txt + Input engine emissions (string): ../../../input_data/ENG_EI.txt SAVE FORWARD RESULTS SUBMENU: Save forward results (T/F): T netCDF filename format (string): APCEMM_Case_* diff --git a/Code.v05-00/tests/input-yamls/test16.yaml b/Code.v05-00/tests/input-yamls/test16.yaml index f9b9efe45..e294fb59b 100644 --- a/Code.v05-00/tests/input-yamls/test16.yaml +++ b/Code.v05-00/tests/input-yamls/test16.yaml @@ -1,5 +1,9 @@ # Test that the removed multi-case runs correctly fail with # the migration message, each entry now takes exactly one value +SIMULATION MENU: + OUTPUT SUBMENU: + Output folder (string): ./ + PARAMETER MENU: METEOROLOGICAL PARAMETERS SUBMENU: Pressure [hPa] (double): 220 230 240 diff --git a/Code.v05-00/tests/input-yamls/test8.yaml b/Code.v05-00/tests/input-yamls/test8.yaml index bb9447ce8..4130a55bf 100644 --- a/Code.v05-00/tests/input-yamls/test8.yaml +++ b/Code.v05-00/tests/input-yamls/test8.yaml @@ -1,5 +1,8 @@ # Null values mean "no override". They must stay valid and leave the # compiled defaults untouched, at the top level and inside a submenu. +# The output folder is required, so it is the one value this file sets. SIMULATION MENU: + SAVE FORWARD RESULTS SUBMENU: OUTPUT SUBMENU: + Output folder (string): ./ PARAMETER MENU: diff --git a/Code.v05-00/tests/test_yamlreader.cpp b/Code.v05-00/tests/test_yamlreader.cpp index b0d7a10f7..da20e9450 100644 --- a/Code.v05-00/tests/test_yamlreader.cpp +++ b/Code.v05-00/tests/test_yamlreader.cpp @@ -1,7 +1,10 @@ #include #include +#include +#include #include #include +#include #include #include "Core/OutputFilenames.hpp" #include "APCEMM.h" @@ -186,6 +189,11 @@ TEST_CASE("Read Yaml File"){ string filename = string(APCEMM_TESTS_DIR) + YAML_DIR + "/test.yaml"; YAML::Node data = YAML::LoadFile(filename); + // The menu readers are called here without going through mergeYamlInputFiles, which + // is what normally resolves paths. readPath rejects a path left relative, so do that + // one step by hand. + PathOriginMap origins; + resolvePathsInPlace(data, std::filesystem::path(filename).parent_path(), filename, origins); SECTION("Read Simulation Menu"){ OptInput input; readSimMenu(input, data["SIMULATION MENU"]); @@ -613,10 +621,13 @@ TEST_CASE("Validate Input Files"){ } SECTION("Empty input file stays valid"){ + // The file stays empty on purpose, so it cannot set the required output folder + // and cannot reach populateInput. What it tests is that the merge accepts it and + // leaves every compiled default in place. YAML::Node merged; REQUIRE_NOTHROW(merged = YamlInputReader::mergeYamlInputFiles({filename7})); - REQUIRE_NOTHROW(YamlInputReader::populateInput(input, scenario, merged)); - REQUIRE(input.SIMULATION_FORWARD_FILENAME == "APCEMM_Case_*"); + REQUIRE(merged["SIMULATION MENU"]["SAVE FORWARD RESULTS SUBMENU"]["netCDF filename format (string)"].as() == "APCEMM_Case_*"); + REQUIRE(merged["SIMULATION MENU"]["OUTPUT SUBMENU"]["Output folder (string)"].as() == "=MISSING="); } SECTION("Document root is a bare value"){ @@ -672,3 +683,250 @@ TEST_CASE("Validate Input Files"){ ); } } + +namespace { + namespace fs = std::filesystem; + + // The rule under test is "a relative path resolves against the directory of the file + // that declares it", so the test needs input files in more than one directory. They + // are written under a temporary root instead of being checked in, because the point + // of every case is where a file sits, not what it says. + struct TwoDirectoryInputs { + fs::path root; + + explicit TwoDirectoryInputs(const string& name) + : root(fs::temp_directory_path() / name) { + fs::remove_all(root); + fs::create_directories(root / "caseA" / "data"); + fs::create_directories(root / "caseB"); + } + ~TwoDirectoryInputs() { fs::remove_all(root); } + + void write(const string& relativePath, const string& contents) const { + std::ofstream out(root / relativePath); + out << contents; + } + string path(const string& relativePath) const { + return (root / relativePath).string(); + } + // What the resolution rule must produce for a path written in a given file. + string resolved(const string& relativePath) const { + return fs::weakly_canonical(root / relativePath).generic_string(); + } + // Same, for a path the input file writes with a trailing separator. Resolution + // keeps it, which is what the reader wants for the output folder. + string resolvedDir(const string& relativePath) const { + return resolved(relativePath) + "/"; + } + }; + + string pathIn(const YAML::Node& merged, const string& submenu, const string& key) { + return merged["SIMULATION MENU"][submenu][key].as(); + } +} + +TEST_CASE("A relative path resolves against the file that declares it"){ + TwoDirectoryInputs inputs("apcemm-path-resolution-test"); + inputs.write("caseA/data/init.txt", "background\n"); + inputs.write("caseA/base.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): out/\n" + " Input background condition (string): data/init.txt\n"); + // A second file in another directory, which changes nothing about paths. + inputs.write("caseB/override.yaml", + "PARAMETER MENU:\n" + " Plume Process [hr] (double): 3\n"); + + const string base = inputs.path("caseA/base.yaml"); + const string overrideFile = inputs.path("caseB/override.yaml"); + + SECTION("One file: the path resolves next to that file"){ + YAML::Node merged = YamlInputReader::mergeYamlInputFiles({base}); + REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() + == inputs.resolved("caseA/data/init.txt")); + REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") + == inputs.resolvedDir("caseA/out")); + } + + SECTION("A second file in another directory does not move the first file's paths"){ + // This is the bug: the resolution used to use the directory of the LAST file. + YAML::Node merged = YamlInputReader::mergeYamlInputFiles({base, overrideFile}); + REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() + == inputs.resolved("caseA/data/init.txt")); + REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") + == inputs.resolvedDir("caseA/out")); + } + + SECTION("Each file's own paths resolve against its own directory"){ + inputs.write("caseB/override.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): out/\n"); + YAML::Node merged = YamlInputReader::mergeYamlInputFiles({base, overrideFile}); + // Declared by the first file, resolved under the first file's directory. + REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() + == inputs.resolved("caseA/data/init.txt")); + // Declared by both files, so the last one wins, under its own directory. + REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") + == inputs.resolvedDir("caseB/out")); + } + + SECTION("An absolute path and the sentinels pass through untouched"){ + const string absolute = inputs.resolved("caseA/data/init.txt"); + inputs.write("caseA/absolute.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): " + absolute + "\n" + " Input background condition (string): " + absolute + "\n" + " Input engine emissions (string): =DEFAULT=\n" + " External EPM NetCDF file: =MISSING=\n"); + YAML::Node merged = YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/absolute.yaml")}); + REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") == absolute); + REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() == absolute); + REQUIRE(merged["SIMULATION MENU"]["Input engine emissions (string)"].as() == "=DEFAULT="); + REQUIRE(merged["SIMULATION MENU"]["External EPM NetCDF file"].as() == "=MISSING="); + } +} + +TEST_CASE("A path that names nothing is reported while reading the input"){ + TwoDirectoryInputs inputs("apcemm-missing-path-test"); + inputs.write("caseA/base.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): out/\n" + " Input background condition (string): data/absent.txt\n"); + const string base = inputs.path("caseA/base.yaml"); + + // The message must name the text as written, what it resolved to, and the file that + // declared it. Naming only the resolved path shows the user something they never typed. + REQUIRE_THROWS_WITH( + YamlInputReader::mergeYamlInputFiles({base}), + Catch::Matchers::ContainsSubstring("Input file not found") && + Catch::Matchers::ContainsSubstring("SIMULATION MENU -> Input background condition (string)") && + Catch::Matchers::ContainsSubstring("data/absent.txt") && + Catch::Matchers::ContainsSubstring(inputs.resolved("caseA/data/absent.txt")) && + Catch::Matchers::ContainsSubstring(base) + ); +} + +TEST_CASE("A path is only checked when the run reads it"){ + TwoDirectoryInputs inputs("apcemm-conditional-path-test"); + inputs.write("caseA/data/init.txt", "background\n"); + + SECTION("The met input file is checked only when met input is on"){ + inputs.write("caseA/met_off.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): out/\n" + "METEOROLOGY MENU:\n" + " METEOROLOGICAL INPUT SUBMENU:\n" + " Use met. input (T/F): F\n" + " Met input file path (string): data/absent.nc\n"); + REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/met_off.yaml")})); + + inputs.write("caseA/met_on.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): out/\n" + "METEOROLOGY MENU:\n" + " METEOROLOGICAL INPUT SUBMENU:\n" + " Use met. input (T/F): T\n" + " Met input file path (string): data/absent.nc\n"); + REQUIRE_THROWS_WITH( + YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/met_on.yaml")}), + Catch::Matchers::ContainsSubstring("Met input file path (string)") && + Catch::Matchers::ContainsSubstring("data/absent.nc") + ); + } + + SECTION("The external EPM file is checked only for the external EPM"){ + inputs.write("caseA/epm_original.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): out/\n" + " EPM type (original/external/new): original\n" + " External EPM NetCDF file: data/absent.nc\n"); + REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/epm_original.yaml")})); + + inputs.write("caseA/epm_external.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): out/\n" + " EPM type (original/external/new): external\n" + " External EPM NetCDF file: data/absent.nc\n"); + REQUIRE_THROWS_WITH( + YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/epm_external.yaml")}), + Catch::Matchers::ContainsSubstring("External EPM NetCDF file") && + Catch::Matchers::ContainsSubstring("data/absent.nc") + ); + } + + SECTION("The output folder is never checked: Main.cpp creates it"){ + inputs.write("caseA/new_output.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): does/not/exist/yet/\n"); + REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/new_output.yaml")})); + } +} + +TEST_CASE("The output folder is required"){ + // The compiled-in default is the "=MISSING=" sentinel, so that nothing has to + // resolve a relative default against a directory the defaults do not have. + string emptyFile = string(APCEMM_TESTS_DIR) + YAML_DIR + "/test9.yaml"; + OptInput input; + Input scenario; + YAML::Node merged = YamlInputReader::mergeYamlInputFiles({emptyFile}); + REQUIRE_THROWS_WITH( + YamlInputReader::populateInput(input, scenario, merged), + Catch::Matchers::ContainsSubstring("No output folder set") && + Catch::Matchers::ContainsSubstring("Output folder (string)") + ); +} + +TEST_CASE("The output folder rejects the =DEFAULT= sentinel too"){ + // '=DEFAULT=' means "use the compiled-in data", which is meaningless for an output + // folder. Without this, Main.cpp would create a directory called '=DEFAULT='. + TwoDirectoryInputs inputs("apcemm-default-output-folder-test"); + inputs.write("caseA/base.yaml", + "SIMULATION MENU:\n" + " OUTPUT SUBMENU:\n" + " Output folder (string): =DEFAULT=\n"); + OptInput input; + Input scenario; + YAML::Node merged = YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/base.yaml")}); + REQUIRE_THROWS_WITH( + YamlInputReader::populateInput(input, scenario, merged), + Catch::Matchers::ContainsSubstring("No output folder set") && + Catch::Matchers::ContainsSubstring("=DEFAULT=") + ); +} + +TEST_CASE("readPath rejects a path that was never resolved"){ + // This is what makes the hand-maintained PATH_KEYS set safe. A path key that is + // missing from it is never resolved, and readPath turns that into a clear error on + // the first run instead of a file quietly looked for in the working directory. + YAML::Node node = YAML::Load( + "absolute: /tmp/somewhere\n" + "relative: data/init.txt\n" + "missing: =MISSING=\n" + "default: =DEFAULT=\n"); + REQUIRE(readPath(node, "absolute") == "/tmp/somewhere"); + REQUIRE(readPath(node, "missing") == "=MISSING="); + REQUIRE(readPath(node, "default") == "=DEFAULT="); + REQUIRE_THROWS_WITH( + readPath(node, "relative"), + Catch::Matchers::ContainsSubstring("still relative") && + Catch::Matchers::ContainsSubstring("PATH_KEYS") + ); +} + +TEST_CASE("The compiled-in defaults hold no relative path"){ + // checkDefaultPaths runs on every merge, so it also asserts that every PATH_KEYS + // entry still names a real scalar leaf of the default tree. A key renamed in + // defaults/input.yaml without renaming its PATH_KEYS entry fails here, instead of + // silently no longer being resolved. + string emptyFile = string(APCEMM_TESTS_DIR) + YAML_DIR + "/test9.yaml"; + REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({emptyFile})); +} diff --git a/examples/default/input.yaml b/examples/default/input.yaml deleted file mode 100644 index b0da3e285..000000000 --- a/examples/default/input.yaml +++ /dev/null @@ -1,3 +0,0 @@ -METEOROLOGY MENU: - METEOROLOGICAL INPUT SUBMENU: - Met input file path (string): example_met_file.nc diff --git a/examples/default/run_example.sh b/examples/default/run_example.sh deleted file mode 100755 index 96410c156..000000000 --- a/examples/default/run_example.sh +++ /dev/null @@ -1,10 +0,0 @@ -# # Uncomment the following lines to compile APCEMM -# git submodule update --init --recursive -# mkdir ../../build -# cd ../../build/ -# cmake ../Code.v05-00 && cmake --build . || exit 1 -# cd ../examples/Example3_met_input - -# Run Example 3 -export APCEMM_runDir="." -./../../build/APCEMM input.yaml From 91c536f541be3b68ddad8e0adc4b31c3680433da Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 13:59:31 -0400 Subject: [PATCH 2/9] Better error messages for missing files --- Code.v05-00/src/Core/Engine.cpp | 10 ++++++---- Code.v05-00/src/EPM/EPM.cpp | 9 +++++++++ Code.v05-00/src/EPM/Solution.cpp | 10 +++++++--- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/Code.v05-00/src/Core/Engine.cpp b/Code.v05-00/src/Core/Engine.cpp index b72444418..cb99d9e51 100644 --- a/Code.v05-00/src/Core/Engine.cpp +++ b/Code.v05-00/src/Core/Engine.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include "Util/PhysConstant.hpp" #include "Core/Engine.hpp" @@ -47,10 +48,11 @@ Engine::Engine(std::string engineName, std::string engineFileName, } else { std::ifstream engineFile(engineFileName); if (!engineFile) { - std::string const currFunc("Engine::Engine"); - std::cout << "ERROR: In " << currFunc << ": Cannot read (" - << engineFileName << ")" << std::endl; - return; + throw std::runtime_error( + "Cannot open the engine emissions file '" + engineFileName + + "'. It is set by 'Input engine emissions (string)' under SIMULATION MENU. " + "A relative path is resolved against the directory of the input file that " + "sets it. Use '=DEFAULT=' for the emissions data compiled into APCEMM."); } foundEngine = diff --git a/Code.v05-00/src/EPM/EPM.cpp b/Code.v05-00/src/EPM/EPM.cpp index 1a3fa9843..684d6483a 100644 --- a/Code.v05-00/src/EPM/EPM.cpp +++ b/Code.v05-00/src/EPM/EPM.cpp @@ -1,5 +1,7 @@ #include +#include #include +#include #include "Core/Input_Mod.hpp" #include "EPM/EPM.hpp" @@ -68,6 +70,13 @@ void Output::write(std::string filename) const { } void Output::read(std::string filename) { + if (!std::filesystem::exists(filename)) { + throw std::runtime_error( + "External EPM NetCDF file does not exist: '" + filename + + "'. It is set by 'External EPM NetCDF file' under SIMULATION MENU. A " + "relative path is resolved against the directory of the input file that " + "sets it."); + } NcFile nc(filename, NcFile::read); // Scalar variables. diff --git a/Code.v05-00/src/EPM/Solution.cpp b/Code.v05-00/src/EPM/Solution.cpp index b0cde6ecc..709a19f86 100644 --- a/Code.v05-00/src/EPM/Solution.cpp +++ b/Code.v05-00/src/EPM/Solution.cpp @@ -12,6 +12,7 @@ /* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ #include +#include #include "KPP/KPP.hpp" #include "KPP/KPP_Parameters.h" @@ -209,9 +210,12 @@ void Solution::readInputBackgroundConditions(const Input& input, Vector_1D& amb_ } else { std::ifstream file(fileName); if (!file) { - std::cout << "ERROR: In Structure::Initialize: Can't read (" << fileName - << ")" << std::endl; - exit(-1); + throw std::runtime_error( + "Cannot open the background condition file '" + fileName + + "'. It is set by 'Input background condition (string)' under SIMULATION " + "MENU. A relative path is resolved against the directory of the input " + "file that sets it. Use '=DEFAULT=' for the background data compiled " + "into APCEMM."); } processInputBackgroundLine(file, amb_Value, aer_Value); From 002a9cfed5b1db114f958b4a814b8187bafcbe80 Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 14:56:29 -0400 Subject: [PATCH 3/9] Cleanup --- Code.v05-00/include/Util/YamlUtils.hpp | 4 + .../include/YamlInputReader/YamlPathUtils.hpp | 37 +--- .../src/YamlInputReader/YamlInputReader.cpp | 24 +-- .../src/YamlInputReader/YamlPathUtils.cpp | 203 +++++++----------- Code.v05-00/tests/test_yamlreader.cpp | 167 ++------------ 5 files changed, 113 insertions(+), 322 deletions(-) diff --git a/Code.v05-00/include/Util/YamlUtils.hpp b/Code.v05-00/include/Util/YamlUtils.hpp index dc59a2e9c..116b1489d 100644 --- a/Code.v05-00/include/Util/YamlUtils.hpp +++ b/Code.v05-00/include/Util/YamlUtils.hpp @@ -17,4 +17,8 @@ const std::string getScalarKey(const YAML::Node& key); const YAML::Node mergeYamlNodes(const YAML::Node& defaults, const YAML::Node& overrides); +inline const std::string buildKeyPath(const std::string& currentPath, const std::string& key){ + return currentPath.empty() ? key : currentPath + " -> " + key; +} + #endif // YAMLUTILS_H_ diff --git a/Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp b/Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp index 4ef8eadfa..e05bfb125 100644 --- a/Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp +++ b/Code.v05-00/include/YamlInputReader/YamlPathUtils.hpp @@ -2,52 +2,19 @@ #define YAMLPATHUTILS_H_ #include -#include #include #include /* -Everything APCEMM knows about file system paths in an input file. - -ONE rule: a relative path resolves against the directory of the file that -declares it. Not the working directory, and not the last file passed on the -command line. The compiled-in defaults hold no relative path at all, so there is -no second rule for them. - -The keys whose value is a path are listed in PATH_KEYS, in YamlPathUtils.cpp. +See description in YamlPathUtils.cpp */ namespace YamlInputReader { - // A path leaf as the user wrote it, and the file that declared it. Kept so an - // unresolvable path can be reported with the text the user typed, not only with - // the resolved path they never saw. - struct PathOrigin { - std::string asWritten; - std::string declaredIn; - }; - // Keyed by the full key path, e.g. "SIMULATION MENU -> OUTPUT SUBMENU -> Output folder (string)". - using PathOriginMap = std::map; - - // "unset" and "use the compiled-in data". Neither is a path, so neither is resolved - // nor checked, and a menu reader that needs a real path has to reject them itself. bool isPathSentinel(const std::string& value); - // Rewrite every path leaf of one input file to an absolute path, resolving a - // relative one against baseDir, the directory of the file that declares it. - // Records what each path looked like as written into origins. - // yaml-cpp nodes are reference types, so the node is edited in place. - void resolvePathsInPlace(YAML::Node node, const std::filesystem::path& baseDir, - const std::string& sourceFile, PathOriginMap& origins); - - // Report a path that names nothing on disk, once the merge is done. - void checkPathsExist(const YAML::Node& merged, const PathOriginMap& origins); + void resolvePathsInPlace(YAML::Node node, const std::filesystem::path& baseDir); - // Reject a PATH_KEYS entry that names no scalar leaf of the default tree, and any - // default path that is relative. Neither can happen without a broken repository, - // which is why this runs once per read rather than being a test-only check. void checkDefaultPaths(const YAML::Node& defaultData); - // Read a path leaf that resolvePathsInPlace has already made absolute. - // Returns the "=MISSING=" and "=DEFAULT=" sentinels unchanged. std::string readPath(const YAML::Node& node, const std::string& key); } diff --git a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp index 82e1b822f..edc38782d 100644 --- a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp @@ -45,7 +45,7 @@ namespace YamlInputReader{ const std::string key = getScalarKey(it.first); const bool isNewKey = seenKeys.insert(key).second; if (!isNewKey) { - std::string errorPath = currentPath.empty() ? key : currentPath + " -> " + key; + std::string errorPath = buildKeyPath(currentPath, key); throw std::runtime_error("Duplicate key found in " + source + ": '" + errorPath + "'. Each key must appear only once. Group all the entries of a menu under a single heading."); } } @@ -62,7 +62,7 @@ namespace YamlInputReader{ checkNoDuplicateKeys(node, source, currentPath); for (const auto& it : node) { const std::string key = getScalarKey(it.first); - const std::string nextPath = currentPath.empty() ? key : currentPath + " -> " + key; + const std::string nextPath = buildKeyPath(currentPath, key); checkNoDuplicateKeysRecursive(it.second, source, nextPath); } } @@ -133,7 +133,7 @@ namespace YamlInputReader{ for (const auto& key : userKeys) { if (!defaultKeys.contains(key)) { // The key from the user's YAML does not exist in the default YAML. - std::string errorPath = currentPath.empty() ? key : currentPath + " -> " + key; + std::string errorPath = buildKeyPath(currentPath, key); if (checkDeprecatedKey(key, errorPath)) { continue; } @@ -144,7 +144,7 @@ namespace YamlInputReader{ // Recurse into every map/value to check their validity const YAML::Node nextUserNode = userNode[key]; const YAML::Node nextDefaultNode = defaultNode[key]; - std::string nextPath = currentPath.empty() ? key : currentPath + " -> " + key; + std::string nextPath = buildKeyPath(currentPath, key); validateYamlKeys(nextDefaultNode, nextUserNode, nextPath); } @@ -165,11 +165,6 @@ namespace YamlInputReader{ throw std::runtime_error(std::string(e.what()) + " This is not a problem with your input file: check the state of your APCEMM repository."); } - // What each path looked like before it was resolved, so an error can name the - // text the user wrote and the file they wrote it in. A later file overwrites an - // earlier one, which is what the merge does with the value itself. - PathOriginMap origins; - for (auto filename: filenames) { YAML::Node userData = YAML::LoadFile(filename); @@ -179,15 +174,11 @@ namespace YamlInputReader{ } catch (const std::runtime_error& e) { throw std::runtime_error("Invalid field in YAML input file '" + filename + "': " + e.what()); } - // Resolve before merging: this is the only point where the file a path was - // declared in is still known. Resolving after the merge would resolve every - // path against whichever file came last. - resolvePathsInPlace(userData, std::filesystem::path(filename).parent_path(), filename, origins); + // Resolve relative paths against the location of the current input file before merging + resolvePathsInPlace(userData, std::filesystem::path(filename).parent_path()); mergedData = mergeYamlNodes(mergedData, userData); } - checkPathsExist(mergedData, origins); - return mergedData; } @@ -284,8 +275,7 @@ namespace YamlInputReader{ YAML::Node outputSubmenu = simNode["OUTPUT SUBMENU"]; std::string outputFolder = readPath(outputSubmenu, "Output folder (string)"); - // Neither sentinel names a folder, and nothing downstream checks them: Main.cpp - // would create a directory called "=MISSING=". + // Ensure that outputFolder has a real value set by the user if (isPathSentinel(outputFolder)) { throw std::invalid_argument("No output folder set: 'Output folder (string)' under SIMULATION MENU -> " "OUTPUT SUBMENU is '" + outputFolder + "'. APCEMM has no default output " diff --git a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp index b6ec18f7c..3dd6b2a3f 100644 --- a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp @@ -1,6 +1,6 @@ #include "YamlInputReader/YamlPathUtils.hpp" -#include "YamlInputReader/YamlInputReader.hpp" // split, parseBoolString, iequals -#include "Util/YamlUtils.hpp" // getScalarKey +#include "YamlInputReader/YamlInputReader.hpp" // split +#include "Util/YamlUtils.hpp" // getScalarKey, buildKeyPath #include #include @@ -9,17 +9,29 @@ #include #include +/* +This module exists to resolve relative paths in APCEMM input files. +APCEMM resolves a relative path relative to the directory of the input file +that specifies the path. This is fine for a single input file, but when they +are multiple we need to ensure that each path is resolved against the dir of +the input file it came from. During the parsing of a user input file, all +relative paths are resolved to absolute paths before they get merged. + +To do so we: +1. Keep track of which keys in the input yaml are paths +2. Check that set of path keys against the default yaml at runtime +3. Check that there are no relative paths left after path resolution +*/ namespace YamlInputReader{ - // The keys whose value is a file system path. A relative value under one of these - // keys resolves against the directory of the file that declares it. - // The entries are full key paths, built with the same " -> " separator - // validateYamlKeys uses, because a leaf key alone is not unique: "netCDF filename - // format (string)" appears four times, and neither it nor "Inst timeseries file - // (string)" is a path, they are filename patterns. - // Keep this in step with defaults/input.yaml. checkDefaultPaths() rejects an entry - // that names no leaf of the default tree, so a key rename cannot silently break - // resolution, and readPath() throws on a value left relative, so a path key missing - // from this set fails loudly on the first run instead of resolving against the wrong + // The input.yaml keys whose value is a file system path. + // The entries are a 'keyPath', built with the same " -> " separator buildKeyPath uses. + // This set needs to be kept up to date with defaults/input.yaml, every path that + // appears in the default path must also appear here to be checked. + // To ensure this set is up to date: + // checkDefaultPaths() rejects an key in the default/input.yaml + // is not in this set so a key rename cannot bypass the check. + // readPath() throws on a value left relative, so a path key missing + // from this set returns an error instead of potentially resolving against the wrong // directory. static const std::set PATH_KEYS = { "SIMULATION MENU -> OUTPUT SUBMENU -> Output folder (string)", @@ -31,82 +43,52 @@ namespace YamlInputReader{ "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Met input file path (string)", }; + // Sentinal values used in the default/input.yaml bool isPathSentinel(const std::string& value) { return value == "=MISSING=" || value == "=DEFAULT="; } - // Internal to path handling. Everything the rest of the reader calls is declared in - // YamlPathUtils.hpp. - namespace { - // The resolution rule, in one place: a relative path resolves against baseDir, the - // directory of the file that declares it. Nothing else. - // currentPath is the full key path of the node being walked, which is what - // PATH_KEYS holds, so the recursion carries it and the public entry point does not - // have to expose it. - void resolvePaths(YAML::Node node, const std::filesystem::path& baseDir, - const std::string& sourceFile, PathOriginMap& origins, - const std::string& currentPath) { - if (!node.IsMap()) { - return; - } - for (const auto& it : node) { - const std::string key = getScalarKey(it.first); - const std::string keyPath = currentPath.empty() ? key : currentPath + " -> " + key; - YAML::Node value = it.second; - - if (value.IsMap()) { - resolvePaths(value, baseDir, sourceFile, origins, keyPath); - continue; - } - // Anything that is not a scalar is either a null value, which means "no - // override", or a type error that validateYamlKeys has already rejected. - if (!value.IsScalar() || !PATH_KEYS.contains(keyPath)) { - continue; - } - - const std::string asWritten = value.as(); - if (isPathSentinel(asWritten)) { - continue; - } - // Recorded whether or not the path is rewritten, so an error about an - // absolute path can still name the file that declared it. - origins[keyPath] = PathOrigin{asWritten, sourceFile}; + // Traverse all nodes of a YAML file, and resolve all keys contained + // in path keys to absolute paths + void resolvePaths(YAML::Node node, const std::filesystem::path& baseDir, + const std::string& currentPath) { + if (!node.IsMap()) { + return; + } + for (const auto& it : node) { + const std::string key = getScalarKey(it.first); + const std::string keyPath = buildKeyPath(currentPath, key); + YAML::Node value = it.second; - const std::filesystem::path written(asWritten); - if (written.is_absolute()) { - continue; - } - // parent_path() is empty when the input file is named without a directory, - // and weakly_canonical() would then leave the result relative. - const std::filesystem::path base = baseDir.empty() ? std::filesystem::current_path() : baseDir; - node[key] = std::filesystem::weakly_canonical(base / written).generic_string(); + if (value.IsMap()) { + resolvePaths(value, baseDir, keyPath); + continue; + } + // Anything that is not a scalar is either a null value, which means "no + // override", or a type error that validateYamlKeys has already rejected. + if (!value.IsScalar() || !PATH_KEYS.contains(keyPath)) { + continue; } - } - // Name the path the way the user can act on it: the key, the text they wrote, - // where they wrote it, and what it resolved to. A path that no input file - // declares comes from the compiled-in defaults. - std::string describePath(const std::string& keyPath, const std::string& resolved, - const PathOriginMap& origins) { - const auto it = origins.find(keyPath); - const std::string declaredIn = it == origins.end() - ? "the compiled-in default input.yaml" - : "'" + it->second.declaredIn + "'"; - const std::string asWritten = it == origins.end() ? resolved : it->second.asWritten; - std::string description = "'" + keyPath + "' is set to '" + asWritten + "' in " + declaredIn; - if (asWritten != resolved) { - description += ", which resolves to '" + resolved + "'"; + const std::string pathAsWritten = value.as(); + if (isPathSentinel(pathAsWritten)) { + continue; + } + const std::filesystem::path written(pathAsWritten); + if (written.is_absolute()) { + continue; } - return description; + // parent_path() is empty when the input file is named without a directory, + // and weakly_canonical() would then leave the result relative. + const std::filesystem::path base = baseDir.empty() ? std::filesystem::current_path() : baseDir; + // Update the node value to be the resolved absolute path + node[key] = std::filesystem::weakly_canonical(base / written).generic_string(); + } } - // Read a leaf by its full key path. Returns an empty string when the path names - // nothing or names a node that is not a scalar. - // The walk is recursive and the node is taken by const reference on purpose: - // "node = node[key]" would not rebind the local node, it would assign the child - // into the tree the node came from, and the non-const Node::operator[] inserts an - // undefined placeholder pair for a missing key while the const one does not. - std::string leafAt(const YAML::Node& node, const std::vector& keys, std::size_t depth) { + // Get a value from the YAML node tree from a key path but as a vector of keys e.g. + // ['METEOROLOGY MENU', 'METEOROLOGICAL INPUT SUBMENU' 'Met input file path (string)' + std::string getValueFromKeyPath(const YAML::Node& node, const std::vector& keys, std::size_t depth) { if (depth == keys.size()) { return node.IsScalar() ? node.as() : ""; } @@ -117,74 +99,45 @@ namespace YamlInputReader{ if (!child) { return ""; } - return leafAt(child, keys, depth + 1); + return getValueFromKeyPath(child, keys, depth + 1); } - std::string leafAt(const YAML::Node& node, const std::string& keyPath) { - return leafAt(node, split(keyPath, " -> "), 0); + // Get a value from the YAML node tree from a key path e.g. (splits onto ' -> ') + // 'METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Met input file path (string)' + std::string getValueFromKeyPath(const YAML::Node& node, const std::string& keyPath) { + return getValueFromKeyPath(node, split(keyPath, " -> "), 0); } - void requirePathExists(const YAML::Node& merged, const std::string& keyPath, - const PathOriginMap& origins) { - const std::string resolved = leafAt(merged, keyPath); - if (isPathSentinel(resolved) || std::filesystem::exists(resolved)) { - return; - } - throw std::runtime_error("Input file not found: " + describePath(keyPath, resolved, origins) + - ". A relative path is resolved against the directory of the file that " - "sets it, not against the working directory."); - } - } - - // Runs on one input file at a time, before the merge, because the merge is what - // loses track of which file a value came from. - void resolvePathsInPlace(YAML::Node node, const std::filesystem::path& baseDir, - const std::string& sourceFile, PathOriginMap& origins) { - resolvePaths(node, baseDir, sourceFile, origins, ""); - } - - // One pass over the merged tree, so a path is reported while reading the input - // instead of much later, when a component fails to open it. - // A path is only checked when the run actually reads it. The output folder is never - // checked: Main.cpp creates it. - void checkPathsExist(const YAML::Node& merged, const PathOriginMap& origins) { - if (parseBoolString(leafAt(merged, "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Use met. input (T/F)"), - "Use met. input (T/F)")) { - requirePathExists(merged, "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Met input file path (string)", origins); - } - if (iequals(leafAt(merged, "SIMULATION MENU -> EPM type (original/external/new)"), "external")) { - requirePathExists(merged, "SIMULATION MENU -> External EPM NetCDF file", origins); - } - requirePathExists(merged, "SIMULATION MENU -> Input background condition (string)", origins); - requirePathExists(merged, "SIMULATION MENU -> Input engine emissions (string)", origins); + // Runs on one input file at a time, before the merge + void resolvePathsInPlace(YAML::Node node, const std::filesystem::path& baseDir) { + resolvePaths(node, baseDir, ""); } // The defaults are compiled in, so this only fails when the repository is in a bad - // state. It is what keeps PATH_KEYS honest: an entry that names no leaf would stop - // resolving without any other sign, and a relative default would need a second - // resolution rule because the compiled-in defaults have no directory of their own. + // state. It checks that PATH_KEYS is current with default/input.yaml void checkDefaultPaths(const YAML::Node& defaultData) { for (const std::string& keyPath : PATH_KEYS) { - const std::string value = leafAt(defaultData, keyPath); + const std::string value = getValueFromKeyPath(defaultData, keyPath); + // Here the default/input.yaml has been updated but the set PATH_KEYS has not if (value.empty()) { throw std::runtime_error("PATH_KEYS names '" + keyPath + "', which is not a scalar leaf of the " - "default input.yaml. Path resolution would silently skip it."); + "default input.yaml. Path resolution would silently skip it, update PATH_KEYS."); } + // Here the default/input.yaml has been updated to contain unsupported values if (!isPathSentinel(value) && !std::filesystem::path(value).is_absolute()) { throw std::runtime_error("The default input.yaml sets '" + keyPath + "' to the relative path '" + value + "'. The compiled-in defaults have no directory to resolve against, " - "so they must hold absolute paths or the '=MISSING=' sentinel."); + "so they must hold absolute paths or the '=MISSING='/'=DEFAULT=' sentinels."); } } } - // Every path reaches OptInput through this accessor. A value left relative means the - // key is not in PATH_KEYS, so it was never resolved and would be read against the - // working directory. Refusing it here turns that mistake into a clear error on the - // first run instead of a file that is silently looked for in the wrong place. + // Final check before passing a key that is supposed to be a path to OptInput, + // if the path is somehow still relative, it means it was not added to PATH_KEYS + // to be resolved -> throw because otherwise we might resolve the relative path incorrectly std::string readPath(const YAML::Node& node, const std::string& key){ const std::string value = node[key].as(); - if (isPathSentinel(value) || std::filesystem::path(value).is_absolute()) { + if (isPathSentinel(value)|| std::filesystem::path(value).is_absolute()) { return value; } throw std::runtime_error("Path '" + key + "' is still relative after reading the input: '" + value + diff --git a/Code.v05-00/tests/test_yamlreader.cpp b/Code.v05-00/tests/test_yamlreader.cpp index da20e9450..f7448aece 100644 --- a/Code.v05-00/tests/test_yamlreader.cpp +++ b/Code.v05-00/tests/test_yamlreader.cpp @@ -192,8 +192,7 @@ TEST_CASE("Read Yaml File"){ // The menu readers are called here without going through mergeYamlInputFiles, which // is what normally resolves paths. readPath rejects a path left relative, so do that // one step by hand. - PathOriginMap origins; - resolvePathsInPlace(data, std::filesystem::path(filename).parent_path(), filename, origins); + resolvePathsInPlace(data, std::filesystem::path(filename).parent_path()); SECTION("Read Simulation Menu"){ OptInput input; readSimMenu(input, data["SIMULATION MENU"]); @@ -687,10 +686,9 @@ TEST_CASE("Validate Input Files"){ namespace { namespace fs = std::filesystem; - // The rule under test is "a relative path resolves against the directory of the file - // that declares it", so the test needs input files in more than one directory. They - // are written under a temporary root instead of being checked in, because the point - // of every case is where a file sits, not what it says. + // Test that "a relative path resolves against the directory of the file + // that declares it", so the test needs input files in more than one directory. + // Write these test file in /tmp/ struct TwoDirectoryInputs { fs::path root; @@ -700,6 +698,7 @@ namespace { fs::create_directories(root / "caseA" / "data"); fs::create_directories(root / "caseB"); } + // Ensure clean up test files in /tmp/ ~TwoDirectoryInputs() { fs::remove_all(root); } void write(const string& relativePath, const string& contents) const { @@ -713,44 +712,41 @@ namespace { string resolved(const string& relativePath) const { return fs::weakly_canonical(root / relativePath).generic_string(); } - // Same, for a path the input file writes with a trailing separator. Resolution - // keeps it, which is what the reader wants for the output folder. + // Same, for a path the input file writes with a trailing '/' because path + // resolution keeps it string resolvedDir(const string& relativePath) const { return resolved(relativePath) + "/"; } }; + // Get a node value from menu/submenu/key keys string pathIn(const YAML::Node& merged, const string& submenu, const string& key) { return merged["SIMULATION MENU"][submenu][key].as(); } } -TEST_CASE("A relative path resolves against the file that declares it"){ +TEST_CASE("Relative path resolution"){ + // Setup test dir TwoDirectoryInputs inputs("apcemm-path-resolution-test"); + // Temp data inputs.write("caseA/data/init.txt", "background\n"); + // First input file in directory caseA/ inputs.write("caseA/base.yaml", "SIMULATION MENU:\n" " OUTPUT SUBMENU:\n" " Output folder (string): out/\n" " Input background condition (string): data/init.txt\n"); - // A second file in another directory, which changes nothing about paths. + // Second input file is in directory caseB/ inputs.write("caseB/override.yaml", "PARAMETER MENU:\n" " Plume Process [hr] (double): 3\n"); + // The input files are in different directories to test relative path resolution const string base = inputs.path("caseA/base.yaml"); const string overrideFile = inputs.path("caseB/override.yaml"); - SECTION("One file: the path resolves next to that file"){ - YAML::Node merged = YamlInputReader::mergeYamlInputFiles({base}); - REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() - == inputs.resolved("caseA/data/init.txt")); - REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") - == inputs.resolvedDir("caseA/out")); - } - - SECTION("A second file in another directory does not move the first file's paths"){ - // This is the bug: the resolution used to use the directory of the LAST file. + SECTION("Path resolution works"){ + // Before the resolution used to resolve with the directory of the last file YAML::Node merged = YamlInputReader::mergeYamlInputFiles({base, overrideFile}); REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() == inputs.resolved("caseA/data/init.txt")); @@ -767,12 +763,13 @@ TEST_CASE("A relative path resolves against the file that declares it"){ // Declared by the first file, resolved under the first file's directory. REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() == inputs.resolved("caseA/data/init.txt")); - // Declared by both files, so the last one wins, under its own directory. + // Declared by both files, so the last one overrides the first (normal override behavior) + // and resolves against the second input file dir REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") == inputs.resolvedDir("caseB/out")); } - SECTION("An absolute path and the sentinels pass through untouched"){ + SECTION("Absolute paths and the sentinels are not changed"){ const string absolute = inputs.resolved("caseA/data/init.txt"); inputs.write("caseA/absolute.yaml", "SIMULATION MENU:\n" @@ -789,131 +786,14 @@ TEST_CASE("A relative path resolves against the file that declares it"){ } } -TEST_CASE("A path that names nothing is reported while reading the input"){ - TwoDirectoryInputs inputs("apcemm-missing-path-test"); - inputs.write("caseA/base.yaml", - "SIMULATION MENU:\n" - " OUTPUT SUBMENU:\n" - " Output folder (string): out/\n" - " Input background condition (string): data/absent.txt\n"); - const string base = inputs.path("caseA/base.yaml"); - - // The message must name the text as written, what it resolved to, and the file that - // declared it. Naming only the resolved path shows the user something they never typed. - REQUIRE_THROWS_WITH( - YamlInputReader::mergeYamlInputFiles({base}), - Catch::Matchers::ContainsSubstring("Input file not found") && - Catch::Matchers::ContainsSubstring("SIMULATION MENU -> Input background condition (string)") && - Catch::Matchers::ContainsSubstring("data/absent.txt") && - Catch::Matchers::ContainsSubstring(inputs.resolved("caseA/data/absent.txt")) && - Catch::Matchers::ContainsSubstring(base) - ); -} - -TEST_CASE("A path is only checked when the run reads it"){ - TwoDirectoryInputs inputs("apcemm-conditional-path-test"); - inputs.write("caseA/data/init.txt", "background\n"); - - SECTION("The met input file is checked only when met input is on"){ - inputs.write("caseA/met_off.yaml", - "SIMULATION MENU:\n" - " OUTPUT SUBMENU:\n" - " Output folder (string): out/\n" - "METEOROLOGY MENU:\n" - " METEOROLOGICAL INPUT SUBMENU:\n" - " Use met. input (T/F): F\n" - " Met input file path (string): data/absent.nc\n"); - REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/met_off.yaml")})); - - inputs.write("caseA/met_on.yaml", - "SIMULATION MENU:\n" - " OUTPUT SUBMENU:\n" - " Output folder (string): out/\n" - "METEOROLOGY MENU:\n" - " METEOROLOGICAL INPUT SUBMENU:\n" - " Use met. input (T/F): T\n" - " Met input file path (string): data/absent.nc\n"); - REQUIRE_THROWS_WITH( - YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/met_on.yaml")}), - Catch::Matchers::ContainsSubstring("Met input file path (string)") && - Catch::Matchers::ContainsSubstring("data/absent.nc") - ); - } - - SECTION("The external EPM file is checked only for the external EPM"){ - inputs.write("caseA/epm_original.yaml", - "SIMULATION MENU:\n" - " OUTPUT SUBMENU:\n" - " Output folder (string): out/\n" - " EPM type (original/external/new): original\n" - " External EPM NetCDF file: data/absent.nc\n"); - REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/epm_original.yaml")})); - - inputs.write("caseA/epm_external.yaml", - "SIMULATION MENU:\n" - " OUTPUT SUBMENU:\n" - " Output folder (string): out/\n" - " EPM type (original/external/new): external\n" - " External EPM NetCDF file: data/absent.nc\n"); - REQUIRE_THROWS_WITH( - YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/epm_external.yaml")}), - Catch::Matchers::ContainsSubstring("External EPM NetCDF file") && - Catch::Matchers::ContainsSubstring("data/absent.nc") - ); - } - - SECTION("The output folder is never checked: Main.cpp creates it"){ - inputs.write("caseA/new_output.yaml", - "SIMULATION MENU:\n" - " OUTPUT SUBMENU:\n" - " Output folder (string): does/not/exist/yet/\n"); - REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/new_output.yaml")})); - } -} - -TEST_CASE("The output folder is required"){ - // The compiled-in default is the "=MISSING=" sentinel, so that nothing has to - // resolve a relative default against a directory the defaults do not have. - string emptyFile = string(APCEMM_TESTS_DIR) + YAML_DIR + "/test9.yaml"; - OptInput input; - Input scenario; - YAML::Node merged = YamlInputReader::mergeYamlInputFiles({emptyFile}); - REQUIRE_THROWS_WITH( - YamlInputReader::populateInput(input, scenario, merged), - Catch::Matchers::ContainsSubstring("No output folder set") && - Catch::Matchers::ContainsSubstring("Output folder (string)") - ); -} - -TEST_CASE("The output folder rejects the =DEFAULT= sentinel too"){ - // '=DEFAULT=' means "use the compiled-in data", which is meaningless for an output - // folder. Without this, Main.cpp would create a directory called '=DEFAULT='. - TwoDirectoryInputs inputs("apcemm-default-output-folder-test"); - inputs.write("caseA/base.yaml", - "SIMULATION MENU:\n" - " OUTPUT SUBMENU:\n" - " Output folder (string): =DEFAULT=\n"); - OptInput input; - Input scenario; - YAML::Node merged = YamlInputReader::mergeYamlInputFiles({inputs.path("caseA/base.yaml")}); - REQUIRE_THROWS_WITH( - YamlInputReader::populateInput(input, scenario, merged), - Catch::Matchers::ContainsSubstring("No output folder set") && - Catch::Matchers::ContainsSubstring("=DEFAULT=") - ); -} - TEST_CASE("readPath rejects a path that was never resolved"){ - // This is what makes the hand-maintained PATH_KEYS set safe. A path key that is - // missing from it is never resolved, and readPath turns that into a clear error on - // the first run instead of a file quietly looked for in the working directory. + // Check that readPath correctly guards against left over relative paths YAML::Node node = YAML::Load( "absolute: /tmp/somewhere\n" "relative: data/init.txt\n" "missing: =MISSING=\n" "default: =DEFAULT=\n"); REQUIRE(readPath(node, "absolute") == "/tmp/somewhere"); - REQUIRE(readPath(node, "missing") == "=MISSING="); REQUIRE(readPath(node, "default") == "=DEFAULT="); REQUIRE_THROWS_WITH( readPath(node, "relative"), @@ -922,11 +802,8 @@ TEST_CASE("readPath rejects a path that was never resolved"){ ); } -TEST_CASE("The compiled-in defaults hold no relative path"){ - // checkDefaultPaths runs on every merge, so it also asserts that every PATH_KEYS - // entry still names a real scalar leaf of the default tree. A key renamed in - // defaults/input.yaml without renaming its PATH_KEYS entry fails here, instead of - // silently no longer being resolved. +TEST_CASE("Defaults has no relative path"){ + // Test against empty yaml string emptyFile = string(APCEMM_TESTS_DIR) + YAML_DIR + "/test9.yaml"; REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({emptyFile})); } From 42c72de379d26b8c81aa027b2dea96f449f2d0a8 Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 15:39:21 -0400 Subject: [PATCH 4/9] More cleanup --- .../include/YamlInputReader/YamlInputReader.hpp | 11 ----------- .../src/YamlInputReader/YamlInputReader.cpp | 9 +++++++++ Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp | 14 ++++++++++---- Code.v05-00/tests/test_yamlreader.cpp | 10 +++------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp b/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp index 971873920..13990fbc1 100644 --- a/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp +++ b/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp @@ -50,17 +50,6 @@ namespace YamlInputReader{ Vector_1D parseVectorDoubleString(const string paramString, const string paramLocation = ""); vector split(const string str, const string delimiter); - // Case-insensitive compare, used for the option values APCEMM accepts in any case - // (e.g. the EPM type). Shared because both the menu readers and the path checks - // have to agree on what "external" means. - inline bool ichar_equals(char a, char b) { - return std::tolower(static_cast(a)) == - std::tolower(static_cast(b)); - } - inline bool iequals(std::string_view lhs, std::string_view rhs) { - return std::ranges::equal(lhs, rhs, ichar_equals); - } - inline string trim(const string str){ string s = str; //trim left diff --git a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp index edc38782d..2ea667eb6 100644 --- a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp @@ -19,6 +19,15 @@ const std::string default_input = namespace YamlInputReader{ + // Case insensitive string comparison + inline bool ichar_equals(char a, char b) { + return std::tolower(static_cast(a)) == + std::tolower(static_cast(b)); + } + inline bool iequals(std::string_view lhs, std::string_view rhs) { + return std::ranges::equal(lhs, rhs, ichar_equals); + } + // Helper function to get all keys from a YAML Map node std::set getYamlKeys(const YAML::Node& node) { std::set keys; diff --git a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp index 3dd6b2a3f..775ed9f92 100644 --- a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp @@ -28,8 +28,8 @@ namespace YamlInputReader{ // This set needs to be kept up to date with defaults/input.yaml, every path that // appears in the default path must also appear here to be checked. // To ensure this set is up to date: - // checkDefaultPaths() rejects an key in the default/input.yaml - // is not in this set so a key rename cannot bypass the check. + // checkDefaultPaths() checks that all keys in the PATH_KEYS set + // are also in default/input.yaml so a key rename cannot bypass the check. // readPath() throws on a value left relative, so a path key missing // from this set returns an error instead of potentially resolving against the wrong // directory. @@ -43,7 +43,7 @@ namespace YamlInputReader{ "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Met input file path (string)", }; - // Sentinal values used in the default/input.yaml + // Sentinel values used in the default/input.yaml bool isPathSentinel(const std::string& value) { return value == "=MISSING=" || value == "=DEFAULT="; } @@ -82,7 +82,13 @@ namespace YamlInputReader{ // and weakly_canonical() would then leave the result relative. const std::filesystem::path base = baseDir.empty() ? std::filesystem::current_path() : baseDir; // Update the node value to be the resolved absolute path - node[key] = std::filesystem::weakly_canonical(base / written).generic_string(); + // Direct assignment updates the node inplace + std::filesystem::path resolved = std::filesystem::weakly_canonical(base / written); + // Normalize trailing / at the end of a directory path + if (!resolved.has_filename()) { + resolved = resolved.parent_path(); + } + value = resolved.generic_string(); } } diff --git a/Code.v05-00/tests/test_yamlreader.cpp b/Code.v05-00/tests/test_yamlreader.cpp index f7448aece..8ede6c696 100644 --- a/Code.v05-00/tests/test_yamlreader.cpp +++ b/Code.v05-00/tests/test_yamlreader.cpp @@ -712,11 +712,6 @@ namespace { string resolved(const string& relativePath) const { return fs::weakly_canonical(root / relativePath).generic_string(); } - // Same, for a path the input file writes with a trailing '/' because path - // resolution keeps it - string resolvedDir(const string& relativePath) const { - return resolved(relativePath) + "/"; - } }; // Get a node value from menu/submenu/key keys @@ -750,8 +745,9 @@ TEST_CASE("Relative path resolution"){ YAML::Node merged = YamlInputReader::mergeYamlInputFiles({base, overrideFile}); REQUIRE(merged["SIMULATION MENU"]["Input background condition (string)"].as() == inputs.resolved("caseA/data/init.txt")); + // 'out/' is written with a trailing '/', resolution drops it. REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") - == inputs.resolvedDir("caseA/out")); + == inputs.resolved("caseA/out")); } SECTION("Each file's own paths resolve against its own directory"){ @@ -766,7 +762,7 @@ TEST_CASE("Relative path resolution"){ // Declared by both files, so the last one overrides the first (normal override behavior) // and resolves against the second input file dir REQUIRE(pathIn(merged, "OUTPUT SUBMENU", "Output folder (string)") - == inputs.resolvedDir("caseB/out")); + == inputs.resolved("caseB/out")); } SECTION("Absolute paths and the sentinels are not changed"){ From 1db77792e316e0f47b32747c1c306592903698d9 Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 16:09:47 -0400 Subject: [PATCH 5/9] Added tests to checkDefaultPaths --- Code.v05-00/tests/test_yamlreader.cpp | 66 +++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/Code.v05-00/tests/test_yamlreader.cpp b/Code.v05-00/tests/test_yamlreader.cpp index 8ede6c696..0926f0eaf 100644 --- a/Code.v05-00/tests/test_yamlreader.cpp +++ b/Code.v05-00/tests/test_yamlreader.cpp @@ -798,8 +798,66 @@ TEST_CASE("readPath rejects a path that was never resolved"){ ); } -TEST_CASE("Defaults has no relative path"){ - // Test against empty yaml - string emptyFile = string(APCEMM_TESTS_DIR) + YAML_DIR + "/test9.yaml"; - REQUIRE_NOTHROW(YamlInputReader::mergeYamlInputFiles({emptyFile})); +namespace { + // Load in the defaults via loading an empty file. This is so that we can mutate + // the default yaml for tests here + YAML::Node compiledDefaults() { + const string emptyFile = string(APCEMM_TESTS_DIR) + YAML_DIR + "/test9.yaml"; + return YamlInputReader::mergeYamlInputFiles({emptyFile}); + } +} + +TEST_CASE("checkDefaultPaths guards the compiled-in defaults"){ + const string pathKey = "Input background condition (string)"; + + SECTION("Real defaults pass"){ + REQUIRE_NOTHROW(checkDefaultPaths(compiledDefaults())); + } + + SECTION("A PATH_KEYS entry that is no longer a leaf is rejected"){ + // Renaming a key in defaults/input.yaml means one of the elements in + // PATH_KEYS is not being used, so the path resolution would not occur + // for the renamed key. + // This test ensures that checkDefaultPaths throws if one of the PATH_KEYS + // is not present in the default file. Most likely, this means that a key + // was renamed in default/input.yaml but not updated in PATH_KEYS + YAML::Node defaults = compiledDefaults(); + YAML::Node simMenu = defaults["SIMULATION MENU"]; + REQUIRE(simMenu.remove(pathKey)); + REQUIRE_THROWS_WITH( + checkDefaultPaths(defaults), + Catch::Matchers::ContainsSubstring("not a scalar leaf") && + Catch::Matchers::ContainsSubstring("SIMULATION MENU -> " + pathKey) + ); + } + + SECTION("A relative path in the defaults is rejected"){ + // Paths in the defaults cannot be relative, instead they should be + // =MISSING= or =DEFAULT=. Ensure checkDefaultPaths throws if it + // finds a relative path + YAML::Node defaults = compiledDefaults(); + defaults["SIMULATION MENU"][pathKey] = "relative/init.txt"; + REQUIRE_THROWS_WITH( + checkDefaultPaths(defaults), + Catch::Matchers::ContainsSubstring("relative path") && + Catch::Matchers::ContainsSubstring("relative/init.txt") && + Catch::Matchers::ContainsSubstring("SIMULATION MENU -> " + pathKey) + ); + } +} + +TEST_CASE("An unset output folder is reported while reading the input"){ + // 'Output folder (string)' is required and the defaults is set to '=MISSING='. + // Check that populateInput throws if the output dir is unspecified. + // Without this check APCEMM creates a directory named '=MISSING=' in the + // working directory. + OptInput input; + Input scenario; + YAML::Node merged = compiledDefaults(); + REQUIRE_THROWS_WITH( + YamlInputReader::populateInput(input, scenario, merged), + Catch::Matchers::ContainsSubstring("No output folder set") && + Catch::Matchers::ContainsSubstring("Output folder (string)") && + Catch::Matchers::ContainsSubstring("=MISSING=") + ); } From a36fe448dc30b637275741c1edc74ba463e5e7ea Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 16:59:47 -0400 Subject: [PATCH 6/9] Cleanup of input.yaml --- Code.v05-00/defaults/input.yaml | 13 ------------- .../YamlInputReader/YamlInputReader.hpp | 3 --- .../src/YamlInputReader/YamlInputReader.cpp | 18 +++++++++--------- .../src/YamlInputReader/YamlPathUtils.cpp | 6 +++--- 4 files changed, 12 insertions(+), 28 deletions(-) diff --git a/Code.v05-00/defaults/input.yaml b/Code.v05-00/defaults/input.yaml index 915145ea3..d8dd54954 100644 --- a/Code.v05-00/defaults/input.yaml +++ b/Code.v05-00/defaults/input.yaml @@ -1,25 +1,12 @@ -# Every file system path in this file and in your own input files follows one rule: -# a relative path resolves against the directory of the file that declares it, not -# against the working directory and not against the last file passed with -i. -# This file is compiled into APCEMM and has no directory of its own, so it must hold -# no relative path. The paths it does hold are the '=MISSING=' and '=DEFAULT=' -# sentinels, which mean "unset" and "use the compiled-in data". -# The keys treated as paths are listed in PATH_KEYS, in -# Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp. - SIMULATION MENU: OpenMP Num Threads (positive int): 8 OUTPUT SUBMENU: - # Required: APCEMM has no default output folder. A relative path resolves - # against the directory of the file that sets it. Output folder (string): =MISSING= Overwrite if folder exists (T/F): T # FFT options (for spectral solver) Use threaded FFT (T/F): F FFTW WISDOM SUBMENU: Use FFTW WISDOM (T/F): T - # Unset: the compiled-in defaults hold no relative path, and './' would have to - # resolve against a directory this file does not have. Dir w/ write permission (string): =MISSING= # This mostly contains information on background aerosol concentration; Not too relevant for contrail behavior Input background condition (string): =DEFAULT= diff --git a/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp b/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp index 13990fbc1..81ea22bff 100644 --- a/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp +++ b/Code.v05-00/include/YamlInputReader/YamlInputReader.hpp @@ -22,9 +22,6 @@ namespace YamlInputReader{ "mode were removed, so every PARAMETER MENU entry takes exactly one value. To vary a " "parameter, write one input file per value and start one APCEMM process for each."; - // Everything about file system paths in an input file lives in YamlPathUtils.hpp: - // the resolution rule, the set of keys that hold a path, and the readPath accessor - // the menu readers below use. // Merge default and user provided files, returns a merged YAML::Node YAML::Node mergeYamlInputFiles(const vector& filenames); diff --git a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp index 2ea667eb6..8d15ab87d 100644 --- a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp @@ -18,16 +18,16 @@ const std::string default_input = ; -namespace YamlInputReader{ - // Case insensitive string comparison - inline bool ichar_equals(char a, char b) { - return std::tolower(static_cast(a)) == - std::tolower(static_cast(b)); - } - inline bool iequals(std::string_view lhs, std::string_view rhs) { - return std::ranges::equal(lhs, rhs, ichar_equals); - } +inline bool ichar_equals(char a, char b) { + return std::tolower(static_cast(a)) == + std::tolower(static_cast(b)); +} +inline bool iequals(std::string_view lhs, std::string_view rhs) { + return std::ranges::equal(lhs, rhs, ichar_equals); +} + +namespace YamlInputReader{ // Helper function to get all keys from a YAML Map node std::set getYamlKeys(const YAML::Node& node) { std::set keys; diff --git a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp index 775ed9f92..f6206c4e0 100644 --- a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp @@ -35,11 +35,11 @@ namespace YamlInputReader{ // directory. static const std::set PATH_KEYS = { "SIMULATION MENU -> OUTPUT SUBMENU -> Output folder (string)", - "SIMULATION MENU -> FFTW WISDOM SUBMENU -> Dir w/ write permission (string)", // removed by TASK-56 + "SIMULATION MENU -> FFTW WISDOM SUBMENU -> Dir w/ write permission (string)", "SIMULATION MENU -> Input background condition (string)", "SIMULATION MENU -> Input engine emissions (string)", "SIMULATION MENU -> External EPM NetCDF file", - "CHEMISTRY MENU -> Photolysis rates folder (string)", // removed by TASK-56 + "CHEMISTRY MENU -> Photolysis rates folder (string)", "METEOROLOGY MENU -> METEOROLOGICAL INPUT SUBMENU -> Met input file path (string)", }; @@ -93,7 +93,7 @@ namespace YamlInputReader{ } // Get a value from the YAML node tree from a key path but as a vector of keys e.g. - // ['METEOROLOGY MENU', 'METEOROLOGICAL INPUT SUBMENU' 'Met input file path (string)' + // ['METEOROLOGY MENU', 'METEOROLOGICAL INPUT SUBMENU' 'Met input file path (string)'] std::string getValueFromKeyPath(const YAML::Node& node, const std::vector& keys, std::size_t depth) { if (depth == keys.size()) { return node.IsScalar() ? node.as() : ""; From 04726cee8a2024bdec17fbf520ea01fad79f83e0 Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 17:01:06 -0400 Subject: [PATCH 7/9] Update Input.hpp --- Code.v05-00/include/Defaults/Input.hpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Code.v05-00/include/Defaults/Input.hpp b/Code.v05-00/include/Defaults/Input.hpp index 8bb0696c9..19bbe03d1 100644 --- a/Code.v05-00/include/Defaults/Input.hpp +++ b/Code.v05-00/include/Defaults/Input.hpp @@ -1,26 +1,13 @@ R"====( -# Every file system path in this file and in your own input files follows one rule: -# a relative path resolves against the directory of the file that declares it, not -# against the working directory and not against the last file passed with -i. -# This file is compiled into APCEMM and has no directory of its own, so it must hold -# no relative path. The paths it does hold are the '=MISSING=' and '=DEFAULT=' -# sentinels, which mean "unset" and "use the compiled-in data". -# The keys treated as paths are listed in PATH_KEYS, in -# Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp. - SIMULATION MENU: OpenMP Num Threads (positive int): 8 OUTPUT SUBMENU: - # Required: APCEMM has no default output folder. A relative path resolves - # against the directory of the file that sets it. Output folder (string): =MISSING= Overwrite if folder exists (T/F): T # FFT options (for spectral solver) Use threaded FFT (T/F): F FFTW WISDOM SUBMENU: Use FFTW WISDOM (T/F): T - # Unset: the compiled-in defaults hold no relative path, and './' would have to - # resolve against a directory this file does not have. Dir w/ write permission (string): =MISSING= # This mostly contains information on background aerosol concentration; Not too relevant for contrail behavior Input background condition (string): =DEFAULT= From 7b42562e916169f5309a37abed71776193caa403 Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Thu, 20 Aug 2026 17:04:44 -0400 Subject: [PATCH 8/9] Remove unnecessary diff --- Code.v05-00/src/YamlInputReader/YamlInputReader.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp index 8d15ab87d..a7d5692e1 100644 --- a/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlInputReader.cpp @@ -18,12 +18,13 @@ const std::string default_input = ; -inline bool ichar_equals(char a, char b) { - return std::tolower(static_cast(a)) == - std::tolower(static_cast(b)); +bool ichar_equals(char a, char b) { + return std::tolower(static_cast(a)) == + std::tolower(static_cast(b)); } -inline bool iequals(std::string_view lhs, std::string_view rhs) { - return std::ranges::equal(lhs, rhs, ichar_equals); + +bool iequals(std::string_view lhs, std::string_view rhs) { + return std::ranges::equal(lhs, rhs, ichar_equals); } From ea13f0fa00992aae899da3507e78e0287dd44a39 Mon Sep 17 00:00:00 2001 From: Louis Robion Date: Fri, 21 Aug 2026 11:34:20 -0400 Subject: [PATCH 9/9] Document edge case in path resolution --- Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp index f6206c4e0..92f88faa6 100644 --- a/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp +++ b/Code.v05-00/src/YamlInputReader/YamlPathUtils.cpp @@ -78,8 +78,16 @@ namespace YamlInputReader{ if (written.is_absolute()) { continue; } - // parent_path() is empty when the input file is named without a directory, - // and weakly_canonical() would then leave the result relative. + // baseDir is computed using std::filesystem::parent_path(input_file_path). + // It is empty when the input file was named without a directory (e.g. passed + // to APCEMM as './path/to/APCEMM input.yaml'). This means that when baseDir is empty, + // the input file is in the same dir as the current working directory (assuming no + // part of APCEMM changes the CWD which is true). + // weakly_canonical() only makes absolute the leading portion of the path that + // already exists. A value whose first element does not exist yet, such as an + // output folder APCEMM creates later (APCEMM_out/ in input.yaml in CWD), stays + // relative and readPath() then rejects it. + // Set the base explicitly so the result is absolute either way. const std::filesystem::path base = baseDir.empty() ? std::filesystem::current_path() : baseDir; // Update the node value to be the resolved absolute path // Direct assignment updates the node inplace