diff --git a/CHANGELOG.md b/CHANGELOG.md index f10e8526e04..ac3877aa269 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,18 @@ ### Added (new features/APIs/variables/...) - [[PR658]](https://github.com/lanl/singularity-eos/pull/XXX) Add `MinimumInternalEnergy`/`MaximumInternalEnergy` to the EOS introspection API, so energy bounds are reachable through modifiers and the `singularity::EOS` variant +- Added `sesame2spiner::saveAllMaterials` overloads taking a list of matids instead of a list of input files, with optional per-material `Params` overrides, so host codes can generate an sp5 file without writing input decks to disk. Also added an overload taking an already-open `hid_t`, plus `writeSP5RootAttributes`, so a single sp5 file can be built up one material at a time. +- [[PR653]] (https://github.com/lanl/singularity-eos/pull/659) Added `sesame2spiner::saveAllMaterials` overloads taking a list of matids instead of a list of input files, with optional per-material `Params` overrides, so host codes can generate an sp5 file without writing input decks to disk. Also added an overload taking an already-open `hid_t`, plus `writeSP5RootAttributes`, so a single sp5 file can be built up one material at a time. ### Fixed (Repair bugs, etc) +- [[PR653]] (https://github.com/lanl/singularity-eos/pull/659) `sesame2spiner` now validates the metadata returned by `eosGetMetadata` before using it. A matid absent from the sesame file previously produced +all-zero bounds with no error, which reached `log()` and generated NaN grids; it is now reported and skipped. It also no longer reports an HDF5 failure against every material following the first failed one, +and now names the offending matid. Material save status is no longer accumulated by summing `herr_t` values, which could cancel out and report success. ### Changed (changing behavior/API/variables/...) - [[PR658]](https://github.com/lanl/singularity-eos/pull/XXX) `ScaledEOS::CheckParams` now requires a strictly positive scale factor, where it previously accepted any nonzero value. +- `sesame2spiner` now honors the requested verbosity when reading material metadata, rather than always using `Verbosity::Debug`. Default command line output is correspondingly quieter. +- `sesame2spiner::getMatBounds` no longer takes a leading index argument, which was unused. ### Infrastructure (changes irrelevant to downstream codes) - [[PR653]](https://github.com/lanl/singularity-eos/pull/653) Move pybind11 to a submodule rather than fetching it via cmake fetchcontent diff --git a/plan_histories/MR659-2026-09-23-sesame2spiner-matid-api.md b/plan_histories/MR659-2026-09-23-sesame2spiner-matid-api.md new file mode 100644 index 00000000000..3fc5aec0120 --- /dev/null +++ b/plan_histories/MR659-2026-09-23-sesame2spiner-matid-api.md @@ -0,0 +1,277 @@ +# Add matid-based `saveAllMaterials` overloads to sesame2spiner-lib + +## Context + +A host code needs to hand sesame2spiner a list of sesame matids (idnos) and get an sp5 +file back, without writing `.dat` parameter decks to disk first. Today the only entry +point is `saveAllMaterials(savename, filenames, ...)` +(`sesame2spiner/sesame2spiner/generate_files.cpp:173`), which parses config files as its +first step. + +A colleague proposed adding two `vector` overloads by copying the existing function +body. The signatures are right, but the copy duplicates ~90 lines that will drift from the +file-based path, and it promotes three latent defects into a public API. This plan keeps +the proposed signatures, makes the `(matids, params)` form the *single* implementation that +everything else delegates to, and splits out a core taking an already-open `hid_t` so the +host can also build one file up one material at a time. + +Two decisions confirmed with the user: +- **Bad matid** → warn naming the matid, skip it, keep going, return nonzero. The good + materials still land in the file. +- **Append** → expose the `hid_t` core plus root-attribute helpers. No new RAII class. + +### Why a bad matid needs explicit handling + +`eosGetMetadata` **discards** the `eosSafeLoad` return code +(`eospac-wrapper/eospac_wrapper.cpp:53`), and its `infoVals` buffers are zero-initialized +just above. A matid absent from the sesame file therefore returns metadata with +`rhoMin == rhoMax == 0` and no error of any kind, which flows into `getMatBounds` → +`log(0)` → NaN bounds. Tolerable when a human wrote the deck and read the spew; +a landmine when a host passes a programmatic list. + +### Why the `log_type` root attribute needs validating + +`SpinerEOS` hard-checks the **file-level** `log_type` attribute at load +(`singularity-eos/eos/eos_spiner_rho_temp.hpp:398-402`, `PORTABLE_ALWAYS_REQUIRE`). That +check cannot catch a file whose materials were written by builds with *different* +`log_type` settings: one material's grids will disagree with the attribute and be silently +mis-interpolated. Only matters once appending is possible, which is what this change +enables. + +## Changes + +### 1. `sesame2spiner/sesame2spiner/generate_files.hpp` + +Replace the single `saveAllMaterials` declaration (line 59) with: + +```cpp +// Root attributes (singularity_version, log_type). Call once on a newly created +// file before adding materials. Exposed for callers that own the hid_t themselves. +herr_t writeSP5RootAttributes(hid_t file); +// Verify an existing file's log_type matches this build. Called by the hid_t core. +herr_t checkSP5RootAttributes(hid_t file); + +// Core. Adds materials to an already-open sp5 file. Safe to call repeatedly on the +// same file to build it up incrementally; duplicate matid and name detection read +// the file, so they stay correct across calls. +herr_t saveAllMaterials(hid_t file, const std::vector &matids, + const std::vector ¶ms, bool printMetadata, + Verbosity eospacWarn); + +// Per-material parameter overrides, into a freshly created file. +herr_t saveAllMaterials(const std::string &savename, const std::vector &matids, + const std::vector ¶ms, bool printMetadata, + Verbosity eospacWarn); + +// Standard sesame2spiner defaults for every material. +herr_t saveAllMaterials(const std::string &savename, const std::vector &matids, + bool printMetadata, Verbosity eospacWarn); + +// Existing file-based interface. Signature unchanged. +herr_t saveAllMaterials(const std::string &savename, + const std::vector &filenames, bool printMetadata, + Verbosity eospacWarn); +``` + +Also: declare `bool checkMetadataValid(int matid, const SesameMetadata &metadata);` next to +the existing `checkValInMatBounds` (line 76), and drop the unused leading `int i` parameter +from `getMatBounds` (line 68). + +### 2. `sesame2spiner/sesame2spiner/generate_files.cpp` + +**The `hid_t` core is the only implementation.** It is the current loop body (lines +196-253) with these changes: + +- Guard `matids.size() != params.size()` and `file < 0`; call `checkSP5RootAttributes` + and bail on mismatch. +- **Duplicate detection via the file, not local state.** Drop `used_matids` / + `used_names`; use `H5Lexists(file, ..., H5P_DEFAULT) > 0`. Both the `/` group and + the `/` soft link live at root (`saveMaterial` creates them at `loc`, lines 66-67), + so one call covers each. Name collisions resolve by incrementing a suffix from `_2` + until free — same numbering the current `used_names` map produces. +- **Validate metadata** with `checkMetadataValid` before use; on failure log the matid, + increment a failure count, `continue`. +- **Per-material status.** Replace `status += saveMaterial(...)` plus the sticky + `if (status != H5_SUCCESS)` (lines 248-252) with a local `herr_t` per material. Today, + once any material fails, every *subsequent* material also prints "problem with HDF5" + even on success, and the message never names a matid. +- **Count failures instead of summing `herr_t`.** `status += ...` lets a `-1` and a `+1` + cancel to `0`, which the caller reads as success. Return `H5_SUCCESS` or `-1` based on a + failure counter, preserving `main.cpp:59`'s `(status == H5_SUCCESS) ? 0 : 1`. +- **Pass `eospacWarn` to `eosGetMetadata`** instead of the hardcoded `Verbosity::Debug` + (line 219). *This quiets the CLI's default output* — intended, but it is a visible + change. +- Default name uses the matid, not the loop index: `"material_" + std::to_string(matid)` + replaces `"material_" + std::to_string(i)` (line 209). The index is meaningless to a + caller who passed matids. + +**The three `savename` overloads become thin wrappers.** The file-based one keeps its +current parse loop (lines 176-182) and then delegates — `AddMaterials` +(`parser.hpp:91`) already produces exactly the `(params, matids)` pair the core wants: + +```cpp +herr_t saveAllMaterials(const std::string &savename, + const std::vector &filenames, bool printMetadata, + Verbosity eospacWarn) { + std::vector params; + std::vector matids; + for (auto const &filename : filenames) AddMaterials(params, matids, filename); + return saveAllMaterials(savename, matids, params, printMetadata, eospacWarn); +} + +herr_t saveAllMaterials(const std::string &savename, const std::vector &matids, + bool printMetadata, Verbosity eospacWarn) { + return saveAllMaterials(savename, matids, std::vector(matids.size()), + printMetadata, eospacWarn); +} +``` + +`Params` default-constructs (`parser.hpp:71`) and every `Get` call in the material loop +supplies a default, so an empty `Params` is exactly "standard defaults". The +`savename` + `(matids, params)` overload owns `H5Fcreate(H5F_ACC_TRUNC)` → +`writeSP5RootAttributes` → core → `H5Fclose`. + +Finally, drop the `` / `` includes, now unused. + +### 3. Docs + +- `sesame2spiner/README.md` — currently a pure CLI transcript with no library section at + all. Add "Using sesame2spiner as a library": the one-shot `vector` call and the + incremental `hid_t` loop, which is the host-facing documentation this whole change is + for. +- `INTEGRATION_WITH_SESAME2SPINER_REFACTOR.md` — the de facto API doc. Update its + `saveAllMaterials`/`getMatBounds` references (lines ~27, 34, 41, 125, 251, 268, 334-338). +- `CHANGELOG.md` — one entry. + +`doc/sphinx/src/models.rst` needs no change: its sesame2spiner section (~line 1965) covers +the CLI and input-deck keys, neither of which changes. + +## Deliberately out of scope + +- **No `SP5File` RAII class.** Per the decision above; `writeSP5RootAttributes` plus the + core's validation covers the hazard without a new public type. +- **`cmake/Format.cmake:56` has a typo** — `sesame2smpiner` instead of `sesame2spiner` — + so `make format` silently skips all sesame2spiner `.cpp` files. I will *not* fix it here: + it would reformat three files that have never been formatted and bury this diff. I'll run + clang-format on just the edited regions and flag the typo separately. +- **Wiring up `sesame2spiner/test/test.cpp`.** It is registered by no CMake target + anywhere, and is stale twice over: Catch2 v2 (`#define CATCH_CONFIG_MAIN` + + `"catch.hpp"`) against the repo's Catch2 v3.7.1, and unqualified `eosGetMetadata` / + `saveMaterial` / `eosDataOfRhoT` calls predating the `sesame2spiner` namespace. Porting + it is real, unrelated work. **Caveat: this means the change lands with no automated + coverage** — verification below is manual. `SINGULARITY_TEST_SESAME` (top-level + `CMakeLists.txt:111-113`) is the correct gate if you want me to do it as a follow-up. + +## Verification + +Build with your usual kessel/spack flow; `SINGULARITY_BUILD_SESAME2SPINER` requires +`SINGULARITY_USE_SPINER`, `SINGULARITY_USE_SPINER_WITH_HDF5`, and +`SINGULARITY_USE_EOSPAC`. + +1. **CLI regression — the critical one**, since the file-based path now delegates through + new code. Capture `h5ls -r` output from the current binary *before* the change, then + after: `./sesame2spiner -p examples/air.dat examples/steel.dat` must produce an + identical file structure. Expect only stdout differences from the `eosGetMetadata` + verbosity fix. +2. **Dedupe regression.** `./sesame2spiner examples/duplicate-test/*.dat` — `air.dat` and + `air2.dat` are byte-identical, so this fixture exercises duplicate-matid skipping. + Confirm the `H5Lexists` rewrite still skips it and yields the same structure as before. +3. **New one-shot API.** Short driver linking `sesame2spiner::sesame2spiner`: + `saveAllMaterials("mats.sp5", {5030, 4272}, false, Verbosity::Quiet)`. `h5ls -r` must + match what deck-driven run #1 produced. +4. **Bad matid.** `saveAllMaterials("mats.sp5", {5030, 99999, 4272}, ...)` — expect a + stderr line naming 99999, a nonzero return, and an sp5 containing 5030 and 4272 with + no NaN-bounded garbage group. +5. **Incremental path.** `H5Fcreate` + `writeSP5RootAttributes`, then + `saveAllMaterials(file, {matid}, ...)` per matid, then `H5Fclose`. Structure must match + the one-shot file from #3. Also call it twice with the same matid to confirm + cross-call duplicate detection fires. +6. **Round-trip.** Load the incrementally built file through `SpinerEOSDependsRhoT` and + evaluate a point or two, confirming it is a valid sp5 and the `log_type` check passes. + +--- + +# Implementation Outcome (2026-09-19) + +Implemented as planned, with one addition the plan did not anticipate. + +## Files changed + +- `sesame2spiner/sesame2spiner/generate_files.hpp` — new declarations +- `sesame2spiner/sesame2spiner/generate_files.cpp` — core + wrappers, fixes +- `sesame2spiner/README.md` — new "Using sesame2spiner as a library" section +- `CHANGELOG.md` — Added / Fixed / Changed entries +- `INTEGRATION_WITH_SESAME2SPINER_REFACTOR.md` — new "Programmatic saveAllMaterials + API" section; annotated the older `SpinerTableGridParams` proposal as partially + delivered; removed stale line-number references + +No CMake changes were needed — no new translation units. + +## Unplanned addition: overload ambiguity on braced string-literal lists + +The plan did not catch this; a compile check of all call forms did. + +```cpp +saveAllMaterials(savename, {"air.dat", "steel.dat"}, false, warn); // ambiguous! +``` + +`std::vector` has an iterator-pair constructor `vector(InputIt, InputIt)`, and +two `const char *` satisfy it: `iterator_traits::value_type` is `char`, +which converts to `int`. So the braced list is viable as both a +`std::vector` and a (nonsensical) `std::vector`, and the call does +not compile. + +Fixed by adding an inline overload taking `std::initializer_list`, +which is an exact match for a braced list of string literals and therefore outranks +both `vector` candidates. It forwards to the `std::vector` overload. + +Note this only ever affected braced-literal call sites. `src/main.cpp` passes a named +`std::vector`, so the CLI was never affected. The failure mode was a +compile error, not silent misbehavior. + +## Behavior changes to be aware of + +1. **CLI output is quieter.** `eosGetMetadata` now receives the caller's verbosity + instead of a hardcoded `Verbosity::Debug`. +2. **Bad matids no longer produce garbage.** Previously an absent matid yielded + all-zero metadata bounds, reaching `log()` and generating NaN grids with no + diagnostic. Now reported by matid and skipped; other materials still save and the + return is non-zero. +3. **Duplicate/name detection reads the file** (`H5Lexists`) rather than in-memory + sets. Equivalent for a single batch call, and correct across incremental calls. +4. **`getMatBounds` lost its leading `int i`.** Nothing outside sesame2spiner called + it. +5. **Default material names use the matid**, not the loop index: `material_`. + +## Verification status + +**Compiled, not run.** `build/`'s dependencies resolved into +`/tmp/buechler-ci-envs/spack`, which has since been cleaned, so `cmake --build` +cannot reconfigure. Compilation was verified by taking the exact flags from +`build/compile_commands.json` and substituting the in-tree `utils/ports-of-call` and +`utils/spiner` submodules for the missing spack prefixes. Clean under +`-Wall -Wextra` for `generate_files.cpp`, `src/main.cpp`, and a scratch driver +exercising all six call forms. Both edited files are clang-format clean (the +pre-existing code in them already was). + +**Outstanding — needs the kessel/spack env and EOSPAC tables.** All six steps in the +Verification section above, most importantly: +- before/after `h5ls -r` comparison for + `./sesame2spiner -p examples/air.dat examples/steel.dat`, since the file-based path + now routes through new code +- `examples/duplicate-test/*.dat` to confirm the `H5Lexists` rewrite still skips the + duplicate air matid +- a bad matid mixed into a real list +- incremental `hid_t` build compared against the equivalent one-shot file + +## Known gaps + +- **No automated coverage.** `sesame2spiner/test/test.cpp` is registered by no CMake + target and would not compile if it were: Catch2 v2 idioms against the repo's v3.7.1, + and unqualified calls predating the `sesame2spiner` namespace. Porting it under + `SINGULARITY_TEST_SESAME` would give the bad-matid and cross-call dedupe paths real + tests. +- **`cmake/Format.cmake:56` typo** — `sesame2smpiner` instead of `sesame2spiner` — so + `make format` silently skips every sesame2spiner `.cpp`. Left alone deliberately; + fixing it reformats files unrelated to this change. clang-format was run directly on + the two edited files instead. diff --git a/sesame2spiner/README.md b/sesame2spiner/README.md index 666d193a02a..e60657f1b48 100644 --- a/sesame2spiner/README.md +++ b/sesame2spiner/README.md @@ -53,9 +53,83 @@ air Soft Link {5030} stainless\ steel\ 347 Soft Link {4272} ``` +## Using sesame2spiner as a library + +The conversion machinery is also available as a library, `sesame2spiner-lib` +(aliased `sesame2spiner::sesame2spiner`), for host codes that want to generate an +sp5 file directly from a list of sesame material ids without writing input decks +to disk first. The declarations live in ``. + +### Generating a file from a list of matids + +```cpp +#include + +using namespace sesame2spiner; +using EospacWrapper::Verbosity; + +const std::vector matids = {5030, 4272}; + +herr_t status = saveAllMaterials("materials.sp5", matids, + /*printMetadata=*/false, Verbosity::Quiet); +``` + +This uses the same default grids the command line tool would use for a deck that +names only `matid`. A material whose metadata cannot be read -- most commonly +because the matid is not present in the sesame file -- is reported on `stderr` +and skipped; the remaining materials are still written and the return value is +non-zero. Duplicate matids are likewise skipped with a warning. + +### Overriding parameters per material + +Every key accepted in an input deck can also be set programmatically on a +`Params` object, one per matid: + +```cpp +std::vector params(matids.size()); +params[0].Set("name", "air"); +params[0].Set("numrho/decade", "40"); +params[1].Set("ionization", "true"); + +herr_t status = saveAllMaterials("materials.sp5", matids, params, + /*printMetadata=*/false, Verbosity::Quiet); +``` + +### Adding materials one at a time + +To build a single file up incrementally -- for instance as a host code discovers +which materials it needs -- open the file yourself and call the `hid_t` overload +once per material. Duplicate matid and name detection query the file, so this +produces the same result as saving everything in one call: + +```cpp +hid_t file = H5Fcreate("materials.sp5", H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); +herr_t status = writeSP5RootAttributes(file); + +for (int matid : matids) { + if (saveAllMaterials(file, {matid}, {Params{}}, + /*printMetadata=*/false, Verbosity::Quiet) != H5_SUCCESS) { + status = -1; // note which materials failed here if you need that detail + } +} + +if (H5Fclose(file) != H5_SUCCESS) status = -1; +``` + +`writeSP5RootAttributes` records the singularity version and the log type the +file was generated with, and must be called once on a newly created file before +any materials are added. To add materials to a file from a previous run, reopen +it with `H5Fopen(..., H5F_ACC_RDWR, ...)` and skip `writeSP5RootAttributes` -- +the attributes are already there. Every call to the `hid_t` overload checks that +the file's log type matches the current build and refuses to proceed otherwise, +since the resulting file would be silently misinterpreted at read time. + +Note that the file must be closed for the resulting sp5 to be valid, so a host +code that may exit early should ensure `H5Fclose` still runs. + ## Copyright -© 2021-2023. Triad National Security, LLC. All rights reserved. This +© 2021-2026. Triad National Security, LLC. All rights reserved. This program was produced under U.S. Government contract 89233218CNA000001 for Los Alamos National Laboratory (LANL), which is operated by Triad National Security, LLC for the U.S. Department of Energy/National diff --git a/sesame2spiner/sesame2spiner/generate_files.cpp b/sesame2spiner/sesame2spiner/generate_files.cpp index 809f8c15155..5f4d13f841a 100644 --- a/sesame2spiner/sesame2spiner/generate_files.cpp +++ b/sesame2spiner/sesame2spiner/generate_files.cpp @@ -18,8 +18,6 @@ #include #include #include -#include -#include #include #ifdef SPINER_USE_HDF @@ -170,64 +168,111 @@ herr_t saveMaterial(hid_t loc, const SesameMetadata &metadata, const Bounds &lRh return status; } -herr_t saveAllMaterials(const std::string &savename, - const std::vector &filenames, bool printMetadata, - Verbosity eospacWarn) { - std::vector params; - std::vector matids; - std::unordered_map used_names; - std::unordered_set used_matids; - SesameMetadata metadata; - hid_t file; +herr_t writeSP5RootAttributes(hid_t file) { herr_t status = H5_SUCCESS; - for (auto const &filename : filenames) { - AddMaterials(params, matids, filename); - } - - std::cout << "Saving to file " << savename << std::endl; - file = H5Fcreate(savename.c_str(), H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); - // singularity version - H5LTset_attribute_string(file, "/", "singularity_version", SESAME2SPINER_VERSION); + if (H5LTset_attribute_string(file, "/", "singularity_version", SESAME2SPINER_VERSION) != + H5_SUCCESS) { + status = -1; + } // log type. 0 for true, 1 for NQT1, 2 for NQT2, -1 for single precision true int log_type = singularity::FastMath::Settings::log_type; - H5LTset_attribute_int(file, "/", SP5::logType, &log_type, 1); + if (H5LTset_attribute_int(file, "/", SP5::logType, &log_type, 1) != H5_SUCCESS) { + status = -1; + } + + if (status != H5_SUCCESS) { + std::cerr << "ERROR: unable to write sp5 root attributes." << std::endl; + } + return status; +} + +herr_t checkSP5RootAttributes(hid_t file) { + const int log_type = singularity::FastMath::Settings::log_type; + + int file_log_type = 0; + if (H5LTget_attribute_int(file, "/", SP5::logType, &file_log_type) != H5_SUCCESS) { + std::cerr << "ERROR: sp5 file has no \"" << SP5::logType << "\" root attribute. " + << "Call writeSP5RootAttributes() on a newly created file before " + << "adding materials to it." << std::endl; + return -1; + } + + if (file_log_type != log_type) { + std::cerr << "ERROR: sp5 file was written with log type " << file_log_type + << " but this build of singularity-eos uses log type " << log_type << ". " + << "Adding materials would produce a file whose materials disagree " + << "with its \"" << SP5::logType << "\" attribute. Refusing." << std::endl; + return -1; + } + + return H5_SUCCESS; +} + +herr_t saveAllMaterials(hid_t file, const std::vector &matids, + const std::vector ¶ms, bool printMetadata, + Verbosity eospacWarn) { + if (file < 0) { + std::cerr << "ERROR: invalid sp5 file handle." << std::endl; + return -1; + } + if (matids.size() != params.size()) { + std::cerr << "ERROR: matids and params must be the same length. Got " << matids.size() + << " and " << params.size() << "." << std::endl; + return -1; + } + if (checkSP5RootAttributes(file) != H5_SUCCESS) { + return -1; + } + + SesameMetadata metadata; + int num_failed = 0; std::cout << "Processing " << matids.size() << " materials..." << std::endl; - for (size_t i = 0; i < matids.size(); i++) { - int matid = matids[i]; - if (used_matids.count(matid) > 0) { + for (std::size_t i = 0; i < matids.size(); i++) { + const int matid = matids[i]; + const std::string sMatid = std::to_string(matid); + + // Duplicate detection queries the file rather than tracking local state, so + // that repeated calls building a file up incrementally behave the same as a + // single call that saves everything at once. + if (H5Lexists(file, sMatid.c_str(), H5P_DEFAULT) > 0) { std::cerr << "...Duplicate matid " << matid << " detected. Skipping." << std::endl; continue; } - used_matids.insert(matid); std::cout << "..." << matid << std::endl; - eosGetMetadata(matid, metadata, Verbosity::Debug); + eosGetMetadata(matid, metadata, eospacWarn); if (printMetadata) std::cout << metadata << std::endl; + if (!checkMetadataValid(matid, metadata)) { + num_failed += 1; + continue; + } + std::string name = params[i].Get("name", metadata.name); if (name == "-1" || name == "") { - std::string new_name = "material_" + std::to_string(i); + std::string new_name = "material_" + sMatid; std::cerr << "...WARNING: no reasonable name found. " << "Using a default name: " << new_name << std::endl; name = new_name; } - if (used_names.count(name) > 0) { - used_names[name] += 1; - std::string new_name = name + "_" + std::to_string(used_names[name]); + if (H5Lexists(file, name.c_str(), H5P_DEFAULT) > 0) { + std::string new_name; + int suffix = 2; + do { + new_name = name + "_" + std::to_string(suffix++); + } while (H5Lexists(file, new_name.c_str(), H5P_DEFAULT) > 0); std::cerr << "...WARNING: Name " << name << " already used. " << "Using name: " << new_name << std::endl; name = new_name; - } else { - used_names[name] = 1; } Bounds lRhoBounds, lTBounds, leBounds; - getMatBounds(i, matid, metadata, params[i], lRhoBounds, lTBounds, leBounds); + getMatBounds(matid, metadata, params[i], lRhoBounds, lTBounds, leBounds); if (eospacWarn == Verbosity::Debug) { std::cout << "bounds for log(rho), log(T), log(sie) are:\n" @@ -240,21 +285,76 @@ herr_t saveAllMaterials(const std::string &savename, << std::endl; } - status += saveMaterial(file, metadata, lRhoBounds, lTBounds, leBounds, name, - add_subtables, eospacWarn); - if (status != H5_SUCCESS) { - std::cerr << "WARNING: problem with HDf5" << std::endl; + // Track status per material so that one failure does not get reported against + // every material that follows it. + const herr_t mat_status = saveMaterial(file, metadata, lRhoBounds, lTBounds, leBounds, + name, add_subtables, eospacWarn); + if (mat_status != H5_SUCCESS) { + std::cerr << "ERROR [" << matid << "]: problem with HDF5 while saving material." + << std::endl; + num_failed += 1; } } + if (num_failed > 0) { + std::cerr << "WARNING: " << num_failed << " of " << matids.size() + << " materials could not be saved." << std::endl; + } + // Count failures rather than summing herr_t values, which can cancel out and + // report success. + return (num_failed == 0) ? H5_SUCCESS : -1; +} + +herr_t saveAllMaterials(const std::string &savename, const std::vector &matids, + const std::vector ¶ms, bool printMetadata, + Verbosity eospacWarn) { + if (matids.size() != params.size()) { + std::cerr << "ERROR: matids and params must be the same length. Got " << matids.size() + << " and " << params.size() << "." << std::endl; + return -1; + } + + std::cout << "Saving to file " << savename << std::endl; + hid_t file = H5Fcreate(savename.c_str(), H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + if (file < 0) { + std::cerr << "ERROR: unable to create file " << savename << std::endl; + return -1; + } + + herr_t status = writeSP5RootAttributes(file); + if (saveAllMaterials(file, matids, params, printMetadata, eospacWarn) != H5_SUCCESS) { + status = -1; + } + std::cout << "Cleaning up." << std::endl; - status += H5Fclose(file); - if (status != H5_SUCCESS) { - std::cerr << "WARNING: problem with HDf5" << std::endl; + if (H5Fclose(file) != H5_SUCCESS) { + std::cerr << "WARNING: problem with HDF5 while closing " << savename << std::endl; + status = -1; } return status; } +herr_t saveAllMaterials(const std::string &savename, const std::vector &matids, + bool printMetadata, Verbosity eospacWarn) { + // A default-constructed Params supplies no overrides, and every lookup in the + // material loop falls back to a default, so this is exactly "standard defaults". + return saveAllMaterials(savename, matids, std::vector(matids.size()), + printMetadata, eospacWarn); +} + +herr_t saveAllMaterials(const std::string &savename, + const std::vector &filenames, bool printMetadata, + Verbosity eospacWarn) { + std::vector params; + std::vector matids; + + for (auto const &filename : filenames) { + AddMaterials(params, matids, filename); + } + + return saveAllMaterials(savename, matids, params, printMetadata, eospacWarn); +} + herr_t saveTablesRhoSie(hid_t loc, int matid, TableSplit split, const Bounds &lRhoBounds, const Bounds &leBounds, Verbosity eospacWarn) { herr_t status = 0; @@ -401,7 +501,7 @@ SpinerTableGridParams paramsToGridParams(int matid, const SesameMetadata &metada return gridParams; } -void getMatBounds(int i, int matid, const SesameMetadata &metadata, const Params ¶ms, +void getMatBounds(int matid, const SesameMetadata &metadata, const Params ¶ms, Bounds &lRhoBounds, Bounds &lTBounds, Bounds &leBounds) { // Convert string-based Params to structured grid parameters @@ -467,6 +567,27 @@ void getMatBounds(int i, int matid, const SesameMetadata &metadata, const Params return; } +bool checkMetadataValid(int matid, const SesameMetadata &metadata) { + auto rangeBad = [](Real vmin, Real vmax) { + return !std::isfinite(vmin) || !std::isfinite(vmax) || !(vmax > vmin); + }; + + if (rangeBad(metadata.rhoMin, metadata.rhoMax) || + rangeBad(metadata.TMin, metadata.TMax) || + rangeBad(metadata.sieMin, metadata.sieMax) || metadata.numRho <= 0 || + metadata.numT <= 0) { + std::cerr << "ERROR [" << matid << "]: metadata is not usable. Skipping.\n" + << "\trho, T, sie bounds = [" << metadata.rhoMin << ", " << metadata.rhoMax + << "], [" << metadata.TMin << ", " << metadata.TMax << "], [" + << metadata.sieMin << ", " << metadata.sieMax << "]\n" + << "\tnumRho, numT = " << metadata.numRho << ", " << metadata.numT << "\n" + << "\tThis usually means matid " << matid + << " is not present in the sesame file." << std::endl; + return false; + } + return true; +} + bool checkValInMatBounds(int matid, const std::string &name, Real val, Real vmin, Real vmax) { if (val < vmin || val > vmax) { diff --git a/sesame2spiner/sesame2spiner/generate_files.hpp b/sesame2spiner/sesame2spiner/generate_files.hpp index 5f5b0b04e85..f6d64f448c7 100644 --- a/sesame2spiner/sesame2spiner/generate_files.hpp +++ b/sesame2spiner/sesame2spiner/generate_files.hpp @@ -17,6 +17,7 @@ #ifndef _SESAME2SPINER_GENERATE_FILES_HPP_ #define _SESAME2SPINER_GENERATE_FILES_HPP_ +#include #include #include @@ -56,16 +57,59 @@ inline herr_t saveMaterial(hid_t loc, const SesameMetadata &metadata, eospacWarn); } +// Write the sp5 root attributes (singularity version and log type). Call this once +// on a newly created file before adding any materials to it. Exposed for callers +// that manage the hid_t themselves; the savename-based overloads below do it for you. +herr_t writeSP5RootAttributes(hid_t file); + +// Check that an existing sp5 file's log type matches the one this build was +// compiled with. Appending materials to a file written with a different log type +// would produce a file whose materials disagree with its log_type attribute, which +// the SpinerEOS constructors cannot detect. Called by the hid_t overload below. +herr_t checkSP5RootAttributes(hid_t file); + +// Add materials to an already-open sp5 file. This is the core implementation; the +// overloads below all funnel into it. Safe to call repeatedly on the same file to +// build it up one material at a time -- duplicate matid and name detection query +// the file itself, so they remain correct across calls. +herr_t saveAllMaterials(hid_t file, const std::vector &matids, + const std::vector ¶ms, bool printMetadata, + Verbosity eospacWarn); + +// Save a list of materials, with per-material parameter overrides, to a new file. +herr_t saveAllMaterials(const std::string &savename, const std::vector &matids, + const std::vector ¶ms, bool printMetadata, + Verbosity eospacWarn); + +// Save a list of materials to a new file using standard sesame2spiner defaults. +herr_t saveAllMaterials(const std::string &savename, const std::vector &matids, + bool printMetadata, Verbosity eospacWarn); + +// Save all materials described by a list of input files to a new file. herr_t saveAllMaterials(const std::string &savename, const std::vector &filenames, bool printMetadata, Verbosity eospacWarn); +// Disambiguates a braced list of string literals, e.g. +// saveAllMaterials(savename, {"air.dat", "steel.dat"}, false, warn); +// Without this, such a call is ambiguous against the std::vector overload +// above: std::vector has an iterator-pair constructor, and a pair of +// const char* satisfies it (as a range of char, which converts to int). An +// exact-match std::initializer_list parameter outranks both vector candidates. +inline herr_t saveAllMaterials(const std::string &savename, + std::initializer_list filenames, + bool printMetadata, Verbosity eospacWarn) { + return saveAllMaterials(savename, + std::vector(filenames.begin(), filenames.end()), + printMetadata, eospacWarn); +} + herr_t saveTablesRhoSie(hid_t loc, int matid, TableSplit split, const Bounds &lRhoBounds, const Bounds &leBounds, Verbosity eospacWarn = Verbosity::Quiet); herr_t saveTablesRhoT(hid_t loc, int matid, TableSplit split, const Bounds &lRhoBounds, const Bounds &lTBounds, Verbosity eospacWarn = Verbosity::Quiet); -void getMatBounds(int i, int matid, const SesameMetadata &metadata, const Params ¶ms, +void getMatBounds(int matid, const SesameMetadata &metadata, const Params ¶ms, Bounds &lRhoBounds, Bounds &lTBounds, Bounds &leBounds); // Convert string-based Params to structured SpinerTableGridParams @@ -77,6 +121,12 @@ SpinerTableGridParams paramsToGridParams(int matid, const SesameMetadata &metada bool checkValInMatBounds(int matid, const std::string &name, Real val, Real vmin, Real vmax); +// Sanity check metadata returned by eosGetMetadata. eosGetMetadata discards the +// eosSafeLoad error code and leaves its buffers zero-initialized on failure, so a +// matid that is absent from the sesame file comes back with all-zero bounds rather +// than an error. Left unchecked, those bounds reach log() and produce NaN grids. +bool checkMetadataValid(int matid, const SesameMetadata &metadata); + int getNumPointsFromPPD(Real min, Real max, int ppd); } // namespace sesame2spiner