From e0bf450b40c6e7e893845a9db37b1a375a890e93 Mon Sep 17 00:00:00 2001 From: petlenz Date: Sun, 16 Aug 2026 09:11:00 +0200 Subject: [PATCH 1/9] umat: compare the deck's constants by value, not just by count The per-thread cache keyed a built context on the material name and checked only NPROPS on later calls. Two calls for one name with the same count but different numbers therefore passed the check and were served the FIRST call's graph, whose constants are baked into its parameters. The analysis converges and reports nothing; the moduli are simply wrong from the second call on. Store the constants and compare them. NPROPS doubles is nothing against an evaluation -- 22x an update just to rebuild a graph, so the comparison is not the cost worth saving here. Found while writing tests for the JSON model layer, but the defect is in the registry and independent of it. --- .../numsim-materials/umat/umat_interface.h | 31 +++++++++-------- tests/test_umat_interface.cpp | 33 +++++++++++++++++++ 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/include/numsim-materials/umat/umat_interface.h b/include/numsim-materials/umat/umat_interface.h index b10ce03..b908a83 100644 --- a/include/numsim-materials/umat/umat_interface.h +++ b/include/numsim-materials/umat/umat_interface.h @@ -207,10 +207,10 @@ class umat_registry { std::unique_ptr ctx; std::unique_ptr solid; std::unique_ptr ps; - /// How many constants the context was built from. The graph is built once - /// and reused, so a later call arriving with a different count would mean - /// the cached parameters no longer describe this material. - std::size_t nprops{0}; + /// The constants the context was built from. The graph is built once and + /// reused, so a later call arriving with different ones would mean the + /// cached parameters no longer describe this material. + std::vector props; }; static std::unordered_mapsecond.nprops != props.size()) + // have distinct names — so anything different means the deck contradicts + // the cached graph, and the constants baked into it would be silently + // wrong for every subsequent call. Comparing the VALUES, not just the + // count: a same-length array with different numbers is the case that + // actually reaches a material, and NPROPS doubles is nothing next to an + // evaluation. + if (!std::equal(it->second.props.begin(), it->second.props.end(), + props.begin(), props.end())) throw fatal_error( "numsim UMAT: material '" + std::string(key) + - "' was built from " + std::to_string(it->second.nprops) + - " constants but this call supplies " + - std::to_string(props.size()) + - " — PROPS must be constant for a given material name"); + "' was built from a different set of " + + std::to_string(it->second.props.size()) + + " constants than this call supplies — PROPS must be constant for a " + "given material name; use distinct *MATERIAL names for distinct " + "constants"); return it->second; } @@ -269,7 +274,7 @@ class umat_registry { thread_state ts; ts.ctx = std::make_unique(); m.build(*ts.ctx, props); - ts.nprops = props.size(); + ts.props.assign(props.begin(), props.end()); if (!ts.ctx->is_finalized()) throw fatal_error( "the builder returned without calling finalize() on the context"); diff --git a/tests/test_umat_interface.cpp b/tests/test_umat_interface.cpp index 72dafc2..17c7813 100644 --- a/tests/test_umat_interface.cpp +++ b/tests/test_umat_interface.cpp @@ -166,6 +166,10 @@ struct Registration { // against an already-built name the NPROPS-consistency check fires first // and require_props is never reached. registry::instance().register_model("COLDNAME", build_deck_elastic, de); + // Likewise used by exactly one test: it has to warm the cache itself with a + // known set of constants, so any other test touching it would decide the + // outcome. + registry::instance().register_model("VALUEPROBE", build_deck_elastic, de); // Deliberately lower-case, to prove the registry folds case on both sides. registry::config lc; @@ -769,4 +773,33 @@ TEST(UmatInterface, ChangingNpropsForTheSameNameIsFatal) { << FatalProbe::last; } +/// The same contradiction with the count held fixed — two constants either way, +/// different numbers. This is the case that actually reaches a material: a +/// count check accepts it and every subsequent call silently returns the FIRST +/// call's stiffness, giving a converged analysis with the wrong moduli. +TEST(UmatInterface, ChangingPropsValuesForTheSameNameIsFatal) { + FatalProbe probe; + std::vector statev(1, 0.0); + const T stran[6] = {0, 0, 0, 0, 0, 0}; + const T dstran[6] = {0.001, 0, 0, 0, 0, 0}; + T stress[6] = {0}, ddsdde[36] = {0}, pnewdt = 1.0; + const T soft[2] = {100.0, 40.0}; + const T stiff[2] = {300.0, 140.0}; + + // Builds the context, and shows which constants it was built from. + call_umat("VALUEPROBE", stress, statev.data(), ddsdde, stran, dstran, 0.0, 0.1, + 3, 3, 6, 0, &pnewdt, nullptr, nullptr, nullptr, nullptr, soft, 2); + ASSERT_EQ(FatalProbe::count, 0); + ASSERT_NEAR(ddsdde[0], 100.0 + 4.0 * 40.0 / 3.0, 1e-9); + + // Same name, same count, different values. + call_umat("VALUEPROBE", stress, statev.data(), ddsdde, stran, dstran, 0.0, 0.1, + 3, 3, 6, 0, &pnewdt, nullptr, nullptr, nullptr, nullptr, stiff, 2); + EXPECT_EQ(FatalProbe::count, 1) + << "a same-length PROPS with different numbers must be reported, not " + "silently served from the cached graph"; + EXPECT_NE(FatalProbe::last.find("constant"), std::string::npos) + << FatalProbe::last; +} + } // namespace From e0f78220f51fbc03960182e37feb943b980a9102 Mon Sep 17 00:00:00 2001 From: petlenz Date: Sun, 16 Aug 2026 12:14:04 +0200 Subject: [PATCH 2/9] materials: props_scalar -- deck constants read per call instead of baked in constant_scalar copies its number into the graph at construction, which is right for Abaqus: PROPS is fixed for a material name, and the registry enforces it. It is wrong for CalculiX, which interpolates the *USER MATERIAL constants by temperature so they can differ from one call to the next. props_scalar records only WHICH slot it owns and takes the number from the host each call. Rebuilding the graph per call would also be correct -- nothing is retained between calls -- but measures 22x an evaluation even from a hand-written builder (6.7 us against 302 ns), where reading a slot is free. bind() dereferences and keeps nothing. A host constants array may be a per-call temporary, so an implementation that stored `const double*` would read a dead stack slot on the next call, and usually return the right number anyway because the slot is commonly reused. There is a test that clobbers the caller's buffer between bind and update; it fails against the stored-pointer version. The property is plain and has no update callback, exactly as constant_scalar: nothing reaches statev_map (so a modulus costs no STATEV slot) and the value is in place before ctx.update(), so graph ordering cannot affect it. In a JSON document the position in "constants" is the slot, so the binding is written exactly as before and only the material type changes: - {"type": "constant_scalar", "name": "K", "value": 0}, + {"type": "props_scalar", "name": "K"}, "constants": ["K::value", "G::value"] The evaluator collects the readers at construction with the dynamic_cast probe statev_map already uses, rather than taking them from config -- a hand-listed set is a second thing to keep in step with the graph, and a missed entry is a silently stale modulus. It range-checks them once per call and reports a short array as the setup fault it is. The registry's PROPS-consistency check stands down for these models: a changed constants array is the point rather than a contradiction, since nothing was baked into the graph for it to contradict. That check is the merged #27, which this needs -- without value comparison the guard has nothing to guard. std::size_t joins the JSON reader registry so "index" survives the document round trip. 9 tests. --- include/numsim-materials/default_materials.h | 2 + .../io/json_parameter_converter.h | 1 + .../numsim-materials/materials/props_scalar.h | 88 +++++ include/numsim-materials/umat/json_model.h | 23 +- .../umat/material_point_evaluator.h | 40 ++ .../umat/plane_stress_evaluator.h | 11 + .../numsim-materials/umat/umat_interface.h | 15 +- tests/CMakeLists.txt | 1 + tests/test_props_scalar.cpp | 352 ++++++++++++++++++ 9 files changed, 527 insertions(+), 6 deletions(-) create mode 100644 include/numsim-materials/materials/props_scalar.h create mode 100644 tests/test_props_scalar.cpp diff --git a/include/numsim-materials/default_materials.h b/include/numsim-materials/default_materials.h index 1ba4672..d433970 100644 --- a/include/numsim-materials/default_materials.h +++ b/include/numsim-materials/default_materials.h @@ -7,6 +7,7 @@ #include "numsim-materials/solvers/vector_newton.h" #include "numsim-materials/materials/scalar_stepper.h" #include "numsim-materials/materials/constant_scalar.h" +#include "numsim-materials/materials/props_scalar.h" #include "numsim-materials/materials/isotropic_tangent.h" #include "numsim-materials/materials/linear_elasticity.h" #include "numsim-materials/materials/linear_stress.h" @@ -73,6 +74,7 @@ void register_default_materials() { factory.template register_type>("scalar_stepper"); factory.template register_type>("linear_elasticity"); factory.template register_type>("constant_scalar"); + factory.template register_type>("props_scalar"); factory.template register_type>("isotropic_tangent"); factory.template register_type>("linear_stress"); factory.template register_type>("autocatalytic_reaction"); diff --git a/include/numsim-materials/io/json_parameter_converter.h b/include/numsim-materials/io/json_parameter_converter.h index a6aff2f..f6d60f5 100644 --- a/include/numsim-materials/io/json_parameter_converter.h +++ b/include/numsim-materials/io/json_parameter_converter.h @@ -93,6 +93,7 @@ json_reader_registry make_default_json_registry() { reg.template add(); reg.template add(); reg.template add(); + reg.template add(); reg.template add(); reg.template add(); diff --git a/include/numsim-materials/materials/props_scalar.h b/include/numsim-materials/materials/props_scalar.h new file mode 100644 index 0000000..642463d --- /dev/null +++ b/include/numsim-materials/materials/props_scalar.h @@ -0,0 +1,88 @@ +#ifndef NUMSIM_MATERIALS_PROPS_SCALAR_H +#define NUMSIM_MATERIALS_PROPS_SCALAR_H + +#include +#include +#include +#include + +#include "numsim-materials/core/material_base.h" + +namespace numsim::materials { + +/// A scalar taken from the host's material-constants array on every call. +/// +/// constant_scalar bakes its number into the graph at construction, which is +/// right when the constants are fixed for a material name — Abaqus PROPS, +/// where two *MATERIAL blocks must have distinct names. This one records only +/// WHICH slot it owns and reads the number from the host each call, for hosts +/// whose constants genuinely vary: CalculiX interpolates the *USER MATERIAL +/// constants by temperature, so they can differ from one call to the next. +/// +/// Rebuilding the graph per call would also be correct — nothing is retained +/// between calls — but it measures 22x an evaluation even from a hand-written +/// builder (6.7 us against 302 ns), against nothing at all for reading a slot. +/// +/// bind() DEREFERENCES. It never stores the pointer, and that is the whole +/// safety argument: a host constants array may be a per-call temporary, so a +/// kept pointer reads a dead stack slot on the next call — and usually returns +/// the right number anyway, because the slot is commonly reused. That failure +/// survives every test and breaks somewhere else. +/// +/// The property is PLAIN and there is NO update callback, exactly as in +/// constant_scalar. Two consequences, both load-bearing: statev_map never sees +/// it, so it costs no STATEV slot; and the value is in place before +/// ctx.update() runs, so where the sort happens to put it cannot matter. +/// +/// Parameters: +/// "name": material name +/// "index": which host constant this publishes, 0-based +template +class props_scalar final : public material_base, Traits> { +public: + using base = material_base, Traits>; + using value_type = typename base::value_type; + using input_parameter_controller = typename base::input_parameter_controller; + + template + explicit props_scalar(Args&&... args) + : base(std::forward(args)...), + m_value(base::template add_output("value")), + // By value, not by reference into the parameter store: a later insert() + // can relocate anything past the handler's small buffer. + m_index(base::template get_parameter("index")) { + // Until the first bind(). A model that never binds would otherwise publish + // whatever the property storage happened to hold. + m_value = value_type{}; + } + + static input_parameter_controller parameters() { + input_parameter_controller para{base::parameters()}; + para.template insert("index").template add(); + return para; + } + + /// Copy this material's constant out of the host's array. + /// + /// Callers under the UMAT layer go through material_point_evaluator, which + /// range-checks every reader once per call and reports a short array as the + /// setup fault it is. The check here is the backstop for direct C++ use. + void bind(std::span props) { + if (m_index >= props.size()) + throw std::out_of_range( + "props_scalar '" + base::name() + "': wants constant " + + std::to_string(m_index) + " but only " + std::to_string(props.size()) + + " were supplied"); + m_value = props[m_index]; + } + + [[nodiscard]] std::size_t index() const noexcept { return m_index; } + +private: + value_type& m_value; + const std::size_t m_index; +}; + +} // namespace numsim::materials + +#endif // NUMSIM_MATERIALS_PROPS_SCALAR_H diff --git a/include/numsim-materials/umat/json_model.h b/include/numsim-materials/umat/json_model.h index 8d1c578..0936ff2 100644 --- a/include/numsim-materials/umat/json_model.h +++ b/include/numsim-materials/umat/json_model.h @@ -52,6 +52,13 @@ /// Pairing this with constant_scalar means a deck constant enters as a graph /// property, so consumers are ordered after it and follow it — see /// materials/isotropic_tangent.h. +/// +/// An entry naming a props_scalar binds the SLOT rather than the value: that +/// material is told which constant it owns and reads the number from the host +/// on every call, instead of having it baked into the graph. The document is +/// otherwise unchanged — swap constant_scalar for props_scalar and nothing +/// downstream notices — which is what a host with genuinely varying constants +/// needs, CalculiX interpolating them by temperature being the case in hand. namespace numsim::materials::umat { /// Register the host-driven source materials with the runtime factory. @@ -162,10 +169,20 @@ typename umat_registry::builder make_json_builder( // a second thread building the same model is unaffected. nlohmann::json doc = parsed; for (std::size_t i = 0; i < bindings.size(); ++i) - for (auto& material : doc["materials"]) - if (material.contains("name") && - material["name"].get() == bindings[i].material) + for (auto& material : doc["materials"]) { + if (!material.contains("name") || + material["name"].get() != bindings[i].material) + continue; + // Same binding, two binding TIMES. A props_scalar is told which slot it + // owns and reads the number on every call; anything else has the number + // copied in now and keeps it for the life of the graph. The position in + // "constants" is the slot either way, so the document says the same + // thing and only the material type decides when it is read. + if (material.value("type", std::string{}) == "props_scalar") + material["index"] = i; + else material[bindings[i].property] = props[i]; + } for (const auto& material : doc["materials"]) create_from_json(ctx, material); diff --git a/include/numsim-materials/umat/material_point_evaluator.h b/include/numsim-materials/umat/material_point_evaluator.h index 614ea26..c27bdc2 100644 --- a/include/numsim-materials/umat/material_point_evaluator.h +++ b/include/numsim-materials/umat/material_point_evaluator.h @@ -1,6 +1,7 @@ #ifndef NUMSIM_MATERIALS_UMAT_MATERIAL_POINT_EVALUATOR_H #define NUMSIM_MATERIALS_UMAT_MATERIAL_POINT_EVALUATOR_H +#include #include #include #include @@ -11,6 +12,7 @@ #include #include "numsim-materials/core/material_context.h" +#include "numsim-materials/materials/props_scalar.h" #include "numsim-materials/umat/errors.h" #include "numsim-materials/umat/external_state_source.h" #include "numsim-materials/umat/statev_map.h" @@ -130,6 +132,15 @@ class material_point_evaluator { } m_statev = std::make_unique>(m_ctx, exclusions); + + // Materials that read the host's constants on every call. Collected rather + // than configured: listing them would be a second place to keep in step + // with the graph, and a missed entry is a silently stale modulus. + for (auto* material : m_ctx.materials()) + if (auto* reader = dynamic_cast*>(material)) { + m_props_readers.push_back(reader); + m_props_needed = std::max(m_props_needed, reader->index() + 1); + } } /// Doubles this material needs in STATEV — what *DEPVAR must be at least. @@ -210,6 +221,33 @@ class material_point_evaluator { tangent_to_buffer(*m_tangent, tangent36); } + /// Copy the host's material constants into the graph. + /// + /// Call once per host call, BEFORE evaluate()/evaluate_canonical(). Separate + /// from `call` on purpose: the plane-stress solve runs the graph repeatedly + /// for one host call, and the constants are the same for every iterate, so + /// they bind once outside that loop. + /// + /// A no-op unless the model contains props_scalar materials — a model whose + /// constants were baked in at build time never reaches the loop. + void bind_props(std::span props) { + if (m_props_readers.empty()) return; + if (props.size() < m_props_needed) + throw fatal_error( + "material_point_evaluator: the model reads " + + std::to_string(m_props_needed) + + " host constants but this call supplied " + + std::to_string(props.size()) + " — check *USER MATERIAL, CONSTANTS="); + for (auto* reader : m_props_readers) reader->bind(props); + } + + /// True when the constants are read per call rather than baked into the + /// graph. The registry uses this to decide whether a changed PROPS array + /// contradicts the cached context or is simply the model working as intended. + [[nodiscard]] bool has_live_props() const noexcept { + return !m_props_readers.empty(); + } + /// Write the updated history back. No commit(): the host owns the timestep. void store_statev(value_type* statev) const { m_statev->pack(statev); } @@ -347,6 +385,8 @@ class material_point_evaluator { const numsim_core::history_property* m_plastic_strain{nullptr}; std::unique_ptr> m_statev; + std::vector*> m_props_readers; + std::size_t m_props_needed{0}; }; } // namespace numsim::materials::umat diff --git a/include/numsim-materials/umat/plane_stress_evaluator.h b/include/numsim-materials/umat/plane_stress_evaluator.h index a39b665..8d5de81 100644 --- a/include/numsim-materials/umat/plane_stress_evaluator.h +++ b/include/numsim-materials/umat/plane_stress_evaluator.h @@ -66,6 +66,17 @@ class plane_stress_evaluator { /// Iterations the last evaluate() needed, for diagnostics. [[nodiscard]] int last_iterations() const noexcept { return m_last_iters; } + /// Bind the host's constants once for the whole out-of-plane solve. They do + /// not depend on the iterate, so binding inside the loop would be repeated + /// work with no effect. + void bind_props(std::span props) { + m_inner->bind_props(props); + } + + [[nodiscard]] bool has_live_props() const noexcept { + return m_inner->has_live_props(); + } + void evaluate(const call& c) { if (c.statev.size() < nstatv()) throw fatal_error( diff --git a/include/numsim-materials/umat/umat_interface.h b/include/numsim-materials/umat/umat_interface.h index b908a83..fd72485 100644 --- a/include/numsim-materials/umat/umat_interface.h +++ b/include/numsim-materials/umat/umat_interface.h @@ -179,10 +179,13 @@ class umat_registry { void evaluate(std::string_view cmname, std::span props, const call& c) { auto& ts = thread_state_for(cmname, props); - if (c.ec == element_case::plane_stress) + if (c.ec == element_case::plane_stress) { + ts.ps->bind_props(props); ts.ps->evaluate(c); - else + } else { + ts.solid->bind_props(props); ts.solid->evaluate(c); + } } /// Drop this thread's cached contexts. Only needed if models are @@ -236,7 +239,13 @@ class umat_registry { // count: a same-length array with different numbers is the case that // actually reaches a material, and NPROPS doubles is nothing next to an // evaluation. - if (!std::equal(it->second.props.begin(), it->second.props.end(), + // + // Unless the model reads its constants per call, in which case a changed + // array is the point rather than a contradiction: nothing was baked into + // the graph for it to contradict. material_point_evaluator::bind_props + // does the range check those models actually need. + if (!it->second.solid->has_live_props() && + !std::equal(it->second.props.begin(), it->second.props.end(), props.begin(), props.end())) throw fatal_error( "numsim UMAT: material '" + std::string(key) + diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f14a377..4425a79 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -25,6 +25,7 @@ add_numsim_test(test_plane_stress_evaluator test_plane_stress_evaluator.cpp) add_numsim_test(test_umat_interface test_umat_interface.cpp) add_numsim_test(test_tangent_generator test_tangent_generator.cpp) add_numsim_test(test_json_model test_json_model.cpp) +add_numsim_test(test_props_scalar test_props_scalar.cpp) target_link_libraries(test_umat_interface PRIVATE Threads::Threads) # Data dumper for plotting (not a test — standalone executable) diff --git a/tests/test_props_scalar.cpp b/tests/test_props_scalar.cpp new file mode 100644 index 0000000..1706167 --- /dev/null +++ b/tests/test_props_scalar.cpp @@ -0,0 +1,352 @@ +#include +#include +#include +#include +#include +#include +#include +#include "numsim-materials/core/material_context.h" +#include "numsim-materials/materials/constant_scalar.h" +#include "numsim-materials/materials/isotropic_tangent.h" +#include "numsim-materials/materials/linear_stress.h" +#include "numsim-materials/materials/props_scalar.h" +#include "numsim-materials/umat/json_model.h" + +// The Fortran-callable symbol, so live constants are exercised through the real +// ABI and not only through the C++ evaluator. +NUMSIM_MATERIALS_DEFINE_UMAT(numsim::materials::material_policy_default) + +namespace { + +namespace nm = numsim::materials; +namespace u = numsim::materials::umat; + +using policy = nm::material_policy_default; +using T = policy::value_type; +using ctx_type = nm::material_context; +using param_type = policy::ParameterHandler; +using registry = u::umat_registry; +using tensor2 = tmech::tensor; +using tensor4 = tmech::tensor; + +/// C1111 for an isotropic tangent, the quantity every test here reads back. +constexpr T c1111(T K, T G) { return K + 4.0 * G / 3.0; } + +// --------------------------------------------------------------------------- +// The material on its own +// --------------------------------------------------------------------------- + +TEST(PropsScalar, PublishesTheConstantAtItsIndex) { + ctx_type ctx; + param_type p; + p.insert("name", "K"); + p.insert("index", 1); + auto& k = ctx.create>(p); + ctx.finalize(); + + const T props[3] = {11.0, 22.0, 33.0}; + k.bind(std::span(props, 3)); + EXPECT_DOUBLE_EQ(ctx.get("K", "value"), 22.0); +} + +/// Before the first bind the property must be a definite value rather than +/// whatever the property storage happened to hold. +TEST(PropsScalar, IsZeroBeforeTheFirstBind) { + ctx_type ctx; + param_type p; + p.insert("name", "K"); + p.insert("index", 0); + ctx.create>(p); + ctx.finalize(); + EXPECT_DOUBLE_EQ(ctx.get("K", "value"), 0.0); +} + +TEST(PropsScalar, RejectsAnIndexPastTheSuppliedConstants) { + ctx_type ctx; + param_type p; + p.insert("name", "K"); + p.insert("index", 5); + auto& k = ctx.create>(p); + ctx.finalize(); + + const T props[2] = {1.0, 2.0}; + EXPECT_THROW(k.bind(std::span(props, 2)), std::out_of_range); +} + +/// Plain, not history — so it costs no STATEV slot. This is the property that +/// makes it usable for a modulus at all; external_scalar_source would consume +/// one slot per constant. +TEST(PropsScalar, CostsNoStatevSlot) { + ctx_type ctx; + param_type p; + p.insert("name", "strain_in"); + ctx.create>(p); + p.clear(); + p.insert("name", "K"); + p.insert("index", 0); + ctx.create>(p); + p.clear(); + p.insert("name", "G"); + p.insert("index", 1); + ctx.create>(p); + p.clear(); + p.insert("name", "stiffness"); + p.insert("K_source", "K"); + p.insert("G_source", "G"); + ctx.create>(p); + p.clear(); + p.insert("name", "elastic"); + p.insert("tangent_source", "stiffness"); + p.insert("strain_source", "strain_in"); + ctx.create>(p); + ctx.finalize(); + + u::material_point_evaluator::config cfg; + cfg.strain_source = "strain_in"; + cfg.stress_source = "elastic"; + cfg.tangent_source = "stiffness"; + u::material_point_evaluator eval(ctx, cfg); + + EXPECT_EQ(eval.nstatv(), 0u); + EXPECT_TRUE(eval.has_live_props()); +} + +// --------------------------------------------------------------------------- +// Through the evaluator +// --------------------------------------------------------------------------- + +/// A model whose two moduli are live deck constants. +void build_live(ctx_type& ctx) { + param_type p; + p.insert("name", "strain_in"); + ctx.create>(p); + p.clear(); + p.insert("name", "K"); + p.insert("index", 0); + ctx.create>(p); + p.clear(); + p.insert("name", "G"); + p.insert("index", 1); + ctx.create>(p); + p.clear(); + p.insert("name", "stiffness"); + p.insert("K_source", "K"); + p.insert("G_source", "G"); + ctx.create>(p); + p.clear(); + p.insert("name", "elastic"); + p.insert("tangent_source", "stiffness"); + p.insert("strain_source", "strain_in"); + ctx.create>(p); + ctx.finalize(); +} + +u::material_point_evaluator::config live_config() { + u::material_point_evaluator::config cfg; + cfg.strain_source = "strain_in"; + cfg.stress_source = "elastic"; + cfg.tangent_source = "stiffness"; + return cfg; +} + +/// One uniaxial call, returning DDSDDE(1,1). +T call_once(u::material_point_evaluator& eval, + std::span props) { + const T stran[6] = {0, 0, 0, 0, 0, 0}; + const T dstran[6] = {0.001, 0, 0, 0, 0, 0}; + std::vector stress(6, 0.0), ddsdde(36, 0.0), statev; + u::material_point_evaluator::call c; + c.stran = stran; + c.dstran = dstran; + c.stress = stress; + c.ddsdde = ddsdde; + c.statev = statev; + eval.bind_props(props); + eval.evaluate(c); + return ddsdde[0]; +} + +/// The whole point: the same graph, two different constant sets, two different +/// stiffnesses — with no rebuild in between. +TEST(PropsScalar, ChangedConstantsChangeTheTangentWithoutARebuild) { + ctx_type ctx; + build_live(ctx); + u::material_point_evaluator eval(ctx, live_config()); + + const T soft[2] = {100.0, 40.0}; + const T stiff[2] = {300.0, 140.0}; + + EXPECT_NEAR(call_once(eval, soft), c1111(100.0, 40.0), 1e-9); + EXPECT_NEAR(call_once(eval, stiff), c1111(300.0, 140.0), 1e-9); + // and back, so the second answer is not simply "the last one wins forever" + EXPECT_NEAR(call_once(eval, soft), c1111(100.0, 40.0), 1e-9); +} + +/// The stored-pointer trap. A host constants array may be a per-call temporary; +/// an implementation that kept `const double*` and dereferenced it during +/// update() would read whatever now occupies that memory. +/// +/// Here the buffer is deliberately overwritten AFTER bind_props and before the +/// graph runs. Dereferencing at bind time makes that irrelevant; keeping the +/// pointer makes the tangent follow the clobbered values. +TEST(PropsScalar, ReadsTheConstantsAtBindTimeNotAtUpdateTime) { + ctx_type ctx; + build_live(ctx); + u::material_point_evaluator eval(ctx, live_config()); + + std::vector buffer{100.0, 40.0}; + eval.bind_props(buffer); + + // Everything the host promised is gone by the time the graph runs. + buffer[0] = -1.0e9; + buffer[1] = -1.0e9; + + const T stran[6] = {0, 0, 0, 0, 0, 0}; + const T dstran[6] = {0.001, 0, 0, 0, 0, 0}; + std::vector stress(6, 0.0), ddsdde(36, 0.0), statev; + u::material_point_evaluator::call c; + c.stran = stran; + c.dstran = dstran; + c.stress = stress; + c.ddsdde = ddsdde; + c.statev = statev; + eval.evaluate(c); + + EXPECT_NEAR(ddsdde[0], c1111(100.0, 40.0), 1e-9) + << "the constants must be copied at bind(), not read through a kept " + "pointer during update()"; +} + +/// Too few constants is a *USER MATERIAL setup error, so it must be fatal +/// rather than something a smaller increment could fix. +TEST(PropsScalar, TooFewConstantsIsFatalAtTheEvaluator) { + ctx_type ctx; + build_live(ctx); + u::material_point_evaluator eval(ctx, live_config()); + + const T only_one[1] = {100.0}; + EXPECT_THROW(eval.bind_props(std::span(only_one, 1)), u::fatal_error); +} + +/// A model with baked constants must not pay for machinery it does not use, +/// and must still report itself as such to the registry. +TEST(PropsScalar, BakedConstantModelsHaveNoLiveProps) { + ctx_type ctx; + param_type p; + p.insert("name", "strain_in"); + ctx.create>(p); + p.clear(); + p.insert("name", "K"); + p.insert("value", 100.0); + ctx.create>(p); + p.clear(); + p.insert("name", "G"); + p.insert("value", 40.0); + ctx.create>(p); + p.clear(); + p.insert("name", "stiffness"); + p.insert("K_source", "K"); + p.insert("G_source", "G"); + ctx.create>(p); + p.clear(); + p.insert("name", "elastic"); + p.insert("tangent_source", "stiffness"); + p.insert("strain_source", "strain_in"); + ctx.create>(p); + ctx.finalize(); + + u::material_point_evaluator eval(ctx, live_config()); + EXPECT_FALSE(eval.has_live_props()); + // bind_props is then a no-op, including for an empty array. + EXPECT_NO_THROW(eval.bind_props({})); +} + +// --------------------------------------------------------------------------- +// Through JSON and the real umat_ entry point +// --------------------------------------------------------------------------- + +/// Identical to the baked document except for the material type — which is the +/// claim the JSON layer makes, so it is worth asserting rather than describing. +const char* kLive = R"({ + "materials": [ + {"type": "external_strain_source", "name": "strain_in"}, + {"type": "props_scalar", "name": "K"}, + {"type": "props_scalar", "name": "G"}, + {"type": "isotropic_tangent", "name": "stiffness", + "K_source": "K", "G_source": "G"}, + {"type": "linear_stress", "name": "elastic", + "tangent_source": "stiffness", "strain_source": "strain_in"} + ], + "constants": ["K::value", "G::value"] +})"; + +registry::config json_config() { + registry::config cfg; + cfg.strain_source = "strain_in"; + cfg.stress_source = "elastic"; + cfg.tangent_source = "stiffness"; + return cfg; +} + +struct Registration { + Registration() { + u::register_json_model("LIVEELASTIC", kLive, json_config()); + } +}; +const Registration registration_{}; + +struct fortran_name { + char buf[80]; + explicit fortran_name(const std::string& s) { + for (auto& c : buf) c = ' '; + for (std::size_t i = 0; i < s.size() && i < 80; ++i) buf[i] = s[i]; + } +}; + +T uniaxial_tangent(const std::string& name, const T* props, int nprops) { + const fortran_name cm(name); + T statev[1] = {0}; + const T stran[6] = {0, 0, 0, 0, 0, 0}; + const T dstran[6] = {0.001, 0, 0, 0, 0, 0}; + T stress[6] = {0}, ddsdde[36] = {0}, pnewdt = 1.0; + T sse = 0, spd = 0, scd = 0, rpl = 0, ddsddt[6] = {0}, drplde[6] = {0}; + T drpldt = 0; + const T time[2] = {0, 0}; + T dtime = 0.1; + const T temp = 0, dtemp = 0, predef = 0, dpred = 0, celent = 1; + const T coords[3] = {0}, drot[9] = {1, 0, 0, 0, 1, 0, 0, 0, 1}; + const T dfg[9] = {1, 0, 0, 0, 1, 0, 0, 0, 1}; + int noel = 1, npt = 1, layer = 1, kspt = 1, jstep = 1, kinc = 1; + int ndi = 3, nshr = 3, ntens = 6, nstatv = 0; + + umat_(stress, statev, ddsdde, &sse, &spd, &scd, &rpl, ddsddt, drplde, &drpldt, + stran, dstran, time, &dtime, &temp, &dtemp, &predef, &dpred, cm.buf, + &ndi, &nshr, &ntens, &nstatv, props, &nprops, coords, drot, &pnewdt, + &celent, dfg, dfg, &noel, &npt, &layer, &kspt, &jstep, &kinc, 80); + + EXPECT_DOUBLE_EQ(pnewdt, 1.0); + return ddsdde[0]; +} + +/// The constants array position is the slot: no "index" appears in the +/// document, yet K takes PROPS[0] and G takes PROPS[1]. +TEST(PropsScalarJson, ConstantsPositionBindsTheSlot) { + const T props[2] = {250.0, 90.0}; + EXPECT_NEAR(uniaxial_tangent("LIVEELASTIC", props, 2), c1111(250.0, 90.0), + 1e-9); +} + +/// Same material NAME, changed constants, no rebuild and no fatal error — the +/// registry's PROPS-consistency check must stand down for a model that reads +/// its constants per call, because there is nothing baked in to contradict. +TEST(PropsScalarJson, ChangedDeckConstantsAreHonouredNotRejected) { + const T first[2] = {100.0, 40.0}; + const T second[2] = {300.0, 140.0}; + + EXPECT_NEAR(uniaxial_tangent("LIVEELASTIC", first, 2), c1111(100.0, 40.0), + 1e-9); + EXPECT_NEAR(uniaxial_tangent("LIVEELASTIC", second, 2), c1111(300.0, 140.0), + 1e-9); +} + +} // namespace From 751f6f562240ad76b1d0b4b49aafeecdd3cd8f74 Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 17:18:47 +0200 Subject: [PATCH 3/9] umat: answer the PROPS-consistency check per slot, and catch an unbound model Two fixes from reviewing the props_scalar PR, both reproduced before fixing. 1. The guard disabled the consistency check for the whole model as soon as ANY constant was live. A document mixing the two binding times therefore let a changed BAKED constant through: through the real umat_ entry point, with K baked and G live and both changed from {100,40} to {300,140}, the tangent came back 286.667 = 100 + 4(140)/3. G tracked the deck, K silently kept its first value, no diagnostic. That is the defect #27 exists to catch, reintroduced for mixed documents. The check is now per slot: the count still has to match, and each value is compared unless a props_scalar owns that slot. Mixing is a reasonable thing to want -- a temperature-dependent modulus beside a fixed yield stress -- so rejecting mixed documents outright was the worse fix. 2. Nothing distinguished "no live constants" from "live constants, never bound". An unbound model published zero for every modulus, giving an all-zero DDSDDE and a host that fails to converge with nothing naming the cause. evaluate_canonical now throws if the model has readers and bind_props has never run. It catches never-bound, not stale: the plane-stress solve runs the graph repeatedly for one host call and must not re-bind per iterate, so "deliberately the same" and "forgot to re-bind" are indistinguishable. Under the registry the gap does not exist -- it binds on every call. Four tests, each verified to fail against the code it guards: a mixed document rejecting a changed baked constant, the same document honouring a changed live one, evaluating before binding, and the plane-stress bind_props forward -- which had no coverage at all and is the one place binding meets an iterative evaluator. --- .../umat/material_point_evaluator.h | 34 ++++- .../numsim-materials/umat/umat_interface.h | 35 +++-- tests/test_props_scalar.cpp | 124 ++++++++++++++++++ 3 files changed, 178 insertions(+), 15 deletions(-) diff --git a/include/numsim-materials/umat/material_point_evaluator.h b/include/numsim-materials/umat/material_point_evaluator.h index c27bdc2..e559304 100644 --- a/include/numsim-materials/umat/material_point_evaluator.h +++ b/include/numsim-materials/umat/material_point_evaluator.h @@ -141,6 +141,8 @@ class material_point_evaluator { m_props_readers.push_back(reader); m_props_needed = std::max(m_props_needed, reader->index() + 1); } + m_live_slots.assign(m_props_needed, false); + for (const auto* reader : m_props_readers) m_live_slots[reader->index()] = true; } /// Doubles this material needs in STATEV — what *DEPVAR must be at least. @@ -201,6 +203,20 @@ class material_point_evaluator { value_type dtime, value_type* stress6, value_type* tangent36, std::span drot = {}) { + // A model that reads its constants per call publishes zero for every one of + // them until bind_props() runs. Left to itself that means moduli of zero, an + // all-zero DDSDDE and a host that fails to converge with nothing naming the + // cause — a plausible number for a modulus, and a degenerate one. + // + // Catches never-bound, not stale: the plane-stress solve runs this repeatedly + // for one host call and must not have to re-bind per iterate, so there is no + // way to tell "deliberately the same constants" from "forgot to re-bind". + // Under the registry that gap does not exist — it binds on every call. + if (!m_props_readers.empty() && !m_props_bound) + throw fatal_error( + "material_point_evaluator: this model reads its material constants " + "per call — call bind_props() before evaluating"); + // STATEV is the only state store: reload it every call, so a repeated call // on an unconverged iterate starts from t_n, not from the previous trial. m_statev->unpack(statev); @@ -239,15 +255,25 @@ class material_point_evaluator { " host constants but this call supplied " + std::to_string(props.size()) + " — check *USER MATERIAL, CONSTANTS="); for (auto* reader : m_props_readers) reader->bind(props); + m_props_bound = true; } - /// True when the constants are read per call rather than baked into the - /// graph. The registry uses this to decide whether a changed PROPS array - /// contradicts the cached context or is simply the model working as intended. + /// True when any constant is read per call rather than baked into the graph. [[nodiscard]] bool has_live_props() const noexcept { return !m_props_readers.empty(); } + /// True when host constant @p slot is read per call, so a changed value there + /// is the model working as intended rather than a contradiction. + /// + /// Per slot, not per model: a document may mix the two binding times — a + /// temperature-dependent modulus beside a genuinely fixed yield stress — and + /// answering this for the whole model would wave the baked constants through + /// as well, leaving them silently at their first value. + [[nodiscard]] bool is_live_prop(std::size_t slot) const noexcept { + return slot < m_live_slots.size() && m_live_slots[slot]; + } + /// Write the updated history back. No commit(): the host owns the timestep. void store_statev(value_type* statev) const { m_statev->pack(statev); } @@ -386,7 +412,9 @@ class material_point_evaluator { m_plastic_strain{nullptr}; std::unique_ptr> m_statev; std::vector*> m_props_readers; + std::vector m_live_slots; std::size_t m_props_needed{0}; + bool m_props_bound{false}; }; } // namespace numsim::materials::umat diff --git a/include/numsim-materials/umat/umat_interface.h b/include/numsim-materials/umat/umat_interface.h index fd72485..58c1ec3 100644 --- a/include/numsim-materials/umat/umat_interface.h +++ b/include/numsim-materials/umat/umat_interface.h @@ -240,20 +240,31 @@ class umat_registry { // actually reaches a material, and NPROPS doubles is nothing next to an // evaluation. // - // Unless the model reads its constants per call, in which case a changed - // array is the point rather than a contradiction: nothing was baked into - // the graph for it to contradict. material_point_evaluator::bind_props - // does the range check those models actually need. - if (!it->second.solid->has_live_props() && - !std::equal(it->second.props.begin(), it->second.props.end(), - props.begin(), props.end())) + // A changed COUNT is a dispatch mistake either way: one material name + // carries one *USER MATERIAL block, whatever its constants mean. + if (it->second.props.size() != props.size()) throw fatal_error( "numsim UMAT: material '" + std::string(key) + - "' was built from a different set of " + - std::to_string(it->second.props.size()) + - " constants than this call supplies — PROPS must be constant for a " - "given material name; use distinct *MATERIAL names for distinct " - "constants"); + "' was built from " + std::to_string(it->second.props.size()) + + " constants but this call supplies " + std::to_string(props.size()) + + " — NPROPS cannot vary for a given material name"); + + // Then per SLOT, skipping the ones the model reads live. Asking + // has_live_props() for the whole model instead would wave a mixed + // document's BAKED constants through as well: they would keep the first + // call's values while the live ones tracked the deck, silently, which is + // the defect this check exists to catch. + for (std::size_t i = 0; i < props.size(); ++i) + if (!it->second.solid->is_live_prop(i) && + it->second.props[i] != props[i]) + throw fatal_error( + "numsim UMAT: material '" + std::string(key) + "' constant " + + std::to_string(i + 1) + " was baked into the graph as " + + std::to_string(it->second.props[i]) + " but this call supplies " + + std::to_string(props[i]) + + " — PROPS must be constant for a given material name; use " + "distinct *MATERIAL names for distinct constants, or a " + "props_scalar for a constant that genuinely varies per call"); return it->second; } diff --git a/tests/test_props_scalar.cpp b/tests/test_props_scalar.cpp index 1706167..cc06adc 100644 --- a/tests/test_props_scalar.cpp +++ b/tests/test_props_scalar.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -217,6 +218,65 @@ TEST(PropsScalar, ReadsTheConstantsAtBindTimeNotAtUpdateTime) { "pointer during update()"; } +/// Every reader publishes zero until bind_props runs, so an unbound model would +/// otherwise evaluate with moduli of zero: an all-zero DDSDDE, a host that fails +/// to converge, and nothing naming the cause. +TEST(PropsScalar, EvaluatingBeforeBindingIsFatal) { + ctx_type ctx; + build_live(ctx); + u::material_point_evaluator eval(ctx, live_config()); + + const T stran[6] = {0, 0, 0, 0, 0, 0}; + const T dstran[6] = {0.001, 0, 0, 0, 0, 0}; + std::vector stress(6, 0.0), ddsdde(36, 0.0), statev; + u::material_point_evaluator::call c; + c.stran = stran; + c.dstran = dstran; + c.stress = stress; + c.ddsdde = ddsdde; + c.statev = statev; + + EXPECT_THROW(eval.evaluate(c), u::fatal_error); + + // Bound, it evaluates normally — the guard must not be a one-way latch. + const T props[2] = {100.0, 40.0}; + eval.bind_props(props); + EXPECT_NO_THROW(eval.evaluate(c)); + EXPECT_NEAR(ddsdde[0], c1111(100.0, 40.0), 1e-9); +} + +/// The plane-stress path binds once for the whole out-of-plane solve. Nothing +/// else here exercises it, and it is the one place binding meets an ITERATIVE +/// evaluator — if a change moved the bind inside the loop, or dropped the +/// forward, only this notices. +TEST(PropsScalar, PlaneStressUsesTheLiveConstants) { + ctx_type ctx; + build_live(ctx); + u::plane_stress_evaluator ps(ctx, live_config(), {}); + + constexpr T K = 100.0, G = 40.0; + const T props[2] = {K, G}; + const T stran[3] = {0, 0, 0}; + const T dstran[3] = {0.001, 0, 0}; + std::vector stress(3, 0.0), ddsdde(9, 0.0), statev(ps.nstatv(), 0.0); + u::material_point_evaluator::call c; + c.stran = stran; + c.dstran = dstran; + c.stress = stress; + c.ddsdde = ddsdde; + c.statev = statev; + c.ec = u::element_case::plane_stress; + + ps.bind_props(props); + ps.evaluate(c); + + // Condensed plane-stress modulus, from the constants the host supplied. + const T E = 9 * K * G / (3 * K + G); + const T nu = (3 * K - 2 * G) / (2 * (3 * K + G)); + EXPECT_NEAR(ddsdde[0], E / (1 - nu * nu), 1e-9); + EXPECT_NEAR(stress[2], 0.0, 1e-10) << "sigma_33 must be driven to zero"; +} + /// Too few constants is a *USER MATERIAL setup error, so it must be fatal /// rather than something a smaller increment could fix. TEST(PropsScalar, TooFewConstantsIsFatalAtTheEvaluator) { @@ -288,9 +348,29 @@ registry::config json_config() { return cfg; } +/// K baked, G live, both listed in "constants". A document is free to mix the +/// two binding times — a temperature-dependent modulus beside a genuinely fixed +/// one — and the consistency check has to be answered per slot, not per model. +const char* kMixed = R"({ + "materials": [ + {"type": "external_strain_source", "name": "strain_in"}, + {"type": "constant_scalar", "name": "K", "value": 0}, + {"type": "props_scalar", "name": "G"}, + {"type": "isotropic_tangent", "name": "stiffness", + "K_source": "K", "G_source": "G"}, + {"type": "linear_stress", "name": "elastic", + "tangent_source": "stiffness", "strain_source": "strain_in"} + ], + "constants": ["K::value", "G::value"] +})"; + struct Registration { Registration() { u::register_json_model("LIVEELASTIC", kLive, json_config()); + // One name per mixed test: each has to warm the cache itself with known + // constants, so sharing a name would let test order decide the outcome. + u::register_json_model("MIXEDBAKED", kMixed, json_config()); + u::register_json_model("MIXEDLIVE", kMixed, json_config()); } }; const Registration registration_{}; @@ -349,4 +429,48 @@ TEST(PropsScalarJson, ChangedDeckConstantsAreHonouredNotRejected) { 1e-9); } +// --------------------------------------------------------------------------- +// Mixed documents: baked and live constants side by side +// --------------------------------------------------------------------------- + +/// Changing a BAKED constant must still be fatal even though the same document +/// has a live one. Answering has_live_props() for the whole model instead let +/// this through: G tracked the deck while K silently kept its first value, and +/// the returned tangent was wrong-but-plausible with no diagnostic — exactly +/// the failure the check exists to catch. +TEST(PropsScalarJson, ChangingABakedConstantIsFatalEvenBesideALiveOne) { + const T first[2] = {100.0, 40.0}; + const T changed_both[2] = {300.0, 140.0}; // K baked, G live + + ASSERT_NEAR(uniaxial_tangent("MIXEDBAKED", first, 2), c1111(100.0, 40.0), + 1e-9); + + int fatal_count = 0; + static int* counter = &fatal_count; + u::set_fatal_handler([](const char*) { ++*counter; }); + const T got = uniaxial_tangent("MIXEDBAKED", changed_both, 2); + u::set_fatal_handler(nullptr); + + EXPECT_EQ(fatal_count, 1) + << "a changed baked constant must be reported, not served from the " + "cached graph because some OTHER constant is live"; + // The wrong-but-plausible answer this used to return, named so a regression + // cannot pass by returning it. + EXPECT_FALSE(std::abs(got - c1111(100.0, 140.0)) < 1e-9) + << "K kept its first value while G followed the deck"; +} + +/// The other half: with the baked constant left alone, changing only the live +/// one is honoured. Without this, the fix above could simply reject every mixed +/// document and still pass. +TEST(PropsScalarJson, ChangingOnlyTheLiveConstantIsHonouredInAMixedDocument) { + const T first[2] = {100.0, 40.0}; + const T live_changed[2] = {100.0, 140.0}; // K unchanged, G changed + + EXPECT_NEAR(uniaxial_tangent("MIXEDLIVE", first, 2), c1111(100.0, 40.0), + 1e-9); + EXPECT_NEAR(uniaxial_tangent("MIXEDLIVE", live_changed, 2), + c1111(100.0, 140.0), 1e-9); +} + } // namespace From 4420ded79670f145c68503d465ca07f44f8de26a Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 22:50:01 +0200 Subject: [PATCH 4/9] umat: split the PROPS-consistency message; name the constant that disagrees Folding both faults into one message lost information the old count check carried. It read "was built from a different set of 2 constants than this call supplies" -- the caller's count gone, so a user reading NPROPS=3 has to work out which number is theirs. They are different faults and want different text: a wrong CONSTANTS= count is a deck error, while equal counts with different numbers is a dispatch error. The value case now names the offending slot and both values: material 'STIFF' constant 1 was baked into the graph as 100.000000 but this call supplies 300.000000 This check never changes a result -- it only ever explains one -- so the message is the entire feature. The tests now assert the contents rather than that the word "constant" appears somewhere. --- .../numsim-materials/umat/umat_interface.h | 26 ++++++++++++++----- tests/test_umat_interface.cpp | 13 ++++++++-- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/include/numsim-materials/umat/umat_interface.h b/include/numsim-materials/umat/umat_interface.h index b908a83..27719cc 100644 --- a/include/numsim-materials/umat/umat_interface.h +++ b/include/numsim-materials/umat/umat_interface.h @@ -236,15 +236,27 @@ class umat_registry { // count: a same-length array with different numbers is the case that // actually reaches a material, and NPROPS doubles is nothing next to an // evaluation. - if (!std::equal(it->second.props.begin(), it->second.props.end(), - props.begin(), props.end())) + // + // Two different faults, so two messages. A wrong count is a deck error; + // equal counts with different numbers is a dispatch error, and there the + // useful thing is WHICH constant disagrees — a check that only ever + // explains is worth the words. + if (it->second.props.size() != props.size()) throw fatal_error( - "numsim UMAT: material '" + std::string(key) + - "' was built from a different set of " + + "numsim UMAT: material '" + std::string(key) + "' was built from " + std::to_string(it->second.props.size()) + - " constants than this call supplies — PROPS must be constant for a " - "given material name; use distinct *MATERIAL names for distinct " - "constants"); + " constants but this call supplies " + std::to_string(props.size()) + + " — NPROPS cannot vary for a given material name"); + + for (std::size_t i = 0; i < props.size(); ++i) + if (it->second.props[i] != props[i]) + throw fatal_error( + "numsim UMAT: material '" + std::string(key) + "' constant " + + std::to_string(i + 1) + " was baked into the graph as " + + std::to_string(it->second.props[i]) + " but this call supplies " + + std::to_string(props[i]) + + " — PROPS must be constant for a given material name; use " + "distinct *MATERIAL names for distinct constants"); return it->second; } diff --git a/tests/test_umat_interface.cpp b/tests/test_umat_interface.cpp index 17c7813..5285802 100644 --- a/tests/test_umat_interface.cpp +++ b/tests/test_umat_interface.cpp @@ -769,7 +769,10 @@ TEST(UmatInterface, ChangingNpropsForTheSameNameIsFatal) { call_umat("STIFF", stress, statev.data(), ddsdde, stran, dstran, 0.0, 0.1, 3, 3, 6, 0, &pnewdt, nullptr, nullptr, nullptr, nullptr, three, 3); EXPECT_EQ(FatalProbe::count, 1); - EXPECT_NE(FatalProbe::last.find("constant"), std::string::npos) + // Both counts, so a user reading NPROPS=3 is not left matching one number. + EXPECT_NE(FatalProbe::last.find("2 constants"), std::string::npos) + << FatalProbe::last; + EXPECT_NE(FatalProbe::last.find("supplies 3"), std::string::npos) << FatalProbe::last; } @@ -798,7 +801,13 @@ TEST(UmatInterface, ChangingPropsValuesForTheSameNameIsFatal) { EXPECT_EQ(FatalProbe::count, 1) << "a same-length PROPS with different numbers must be reported, not " "silently served from the cached graph"; - EXPECT_NE(FatalProbe::last.find("constant"), std::string::npos) + // The message has to name WHICH constant disagrees and both values: this + // check never changes a result, it only ever explains one. + EXPECT_NE(FatalProbe::last.find("constant 1"), std::string::npos) + << FatalProbe::last; + EXPECT_NE(FatalProbe::last.find("100.0"), std::string::npos) + << FatalProbe::last; + EXPECT_NE(FatalProbe::last.find("300.0"), std::string::npos) << FatalProbe::last; } From c26bbbe8da0ede2eceab390135026d02447639d5 Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 22:53:23 +0200 Subject: [PATCH 5/9] umat: resolve a constants target through one function, for both binding times Merging the json_model fixes exposed the duplication they warned about. The declared-parameter check validated bindings[i].property while the substitution loop wrote "index" for a props_scalar -- so every props_scalar document was rejected at registration with "props_scalar does not declare 'value'", which is true of the parameter and false of the target. bound_parameter() now answers both questions in one place: which parameter is written, and whether the value is the host constant or the slot it lives in. Validation and substitution call it, so they cannot drift. The target still names the graph property the constant arrives on -- "value", the same as constant_scalar -- because keeping the spelling identical across the two binding times is what lets the type be swapped without rewriting "constants". Naming "index" instead is rejected, since the slot comes from the entry's position and a target that appeared to set it would be misleading. --- include/numsim-materials/umat/json_model.h | 50 ++++++++++++++-------- tests/test_props_scalar.cpp | 19 ++++++++ 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/include/numsim-materials/umat/json_model.h b/include/numsim-materials/umat/json_model.h index cc2ca57..b63e81b 100644 --- a/include/numsim-materials/umat/json_model.h +++ b/include/numsim-materials/umat/json_model.h @@ -89,14 +89,34 @@ void ensure_materials_registered() { }); } -/// The parameter a binding will actually write. +/// What a binding writes into the document. /// -/// Its own function because it is the single place that has to stay in step -/// with the substitution loop below — validating one name and writing another -/// is how a target ends up half-checked. -inline std::string bound_parameter(const nlohmann::json& /*material*/, - const connection_source& binding) { - return binding.property; +/// One function for both the validation and the substitution below: checking +/// one parameter name and then writing a different one is exactly how a target +/// ends up half-verified, which is the defect this validation exists to close. +struct constant_binding_target { + std::string parameter; + /// props_scalar is told which SLOT it owns and reads the number itself on + /// every call; everything else has the number written in now. + bool writes_slot{false}; +}; + +inline constant_binding_target bound_parameter( + const nlohmann::json& material, const connection_source& binding) { + if (material.value("type", std::string{}) == "props_scalar") { + // The target names the graph property the constant will arrive on, which + // props_scalar publishes as "value" just as constant_scalar does. Keeping + // the spelling identical across the two binding times is the point — + // swapping the type must not force "constants" to be rewritten — so the + // parameter actually written ("index") is not what the document says. + if (binding.property != "value") + throw fatal_error( + "json_model: a props_scalar target names the property it publishes, " + "which is \"value\" — got \"" + binding.property + + "\"; the slot comes from the entry's position in \"constants\""); + return {"index", true}; + } + return {binding.property, false}; } /// Reject a target naming a parameter the material does not declare. @@ -122,7 +142,7 @@ void require_declared_parameter(const nlohmann::json& material, // after this one. if (!factory.contains(type)) return; - const auto wanted = bound_parameter(material, binding); + const auto wanted = bound_parameter(material, binding).parameter; std::vector declared; for (const auto& [key, unused] : factory.schema(type)) declared.push_back(key); if (std::find(declared.begin(), declared.end(), wanted) != declared.end()) @@ -245,15 +265,11 @@ typename umat_registry::builder make_json_builder( if (!material.contains("name") || material["name"].get() != bindings[i].material) continue; - // Same binding, two binding TIMES. A props_scalar is told which slot it - // owns and reads the number on every call; anything else has the number - // copied in now and keeps it for the life of the graph. The position in - // "constants" is the slot either way, so the document says the same - // thing and only the material type decides when it is read. - if (material.value("type", std::string{}) == "props_scalar") - material["index"] = i; - else - material[bindings[i].property] = props[i]; + // Same binding, two binding TIMES — resolved by the same function that + // validated the target at registration, so the two cannot drift. + const auto target = bound_parameter(material, bindings[i]); + material[target.parameter] = + target.writes_slot ? nlohmann::json(i) : nlohmann::json(props[i]); } for (const auto& material : doc["materials"]) diff --git a/tests/test_props_scalar.cpp b/tests/test_props_scalar.cpp index cc06adc..2994296 100644 --- a/tests/test_props_scalar.cpp +++ b/tests/test_props_scalar.cpp @@ -416,6 +416,25 @@ TEST(PropsScalarJson, ConstantsPositionBindsTheSlot) { 1e-9); } +/// The target names the graph property the constant arrives on — "value" — and +/// not the "index" parameter the builder actually writes. Keeping the spelling +/// identical to constant_scalar's is what lets the type be swapped without +/// rewriting "constants", so the wrong spelling has to be rejected rather than +/// quietly accepted. +TEST(PropsScalarJson, RejectsATargetNamingIndexRatherThanTheProperty) { + const char* names_index = R"({ + "materials": [{"type": "props_scalar", "name": "K"}], + "constants": ["K::index"] + })"; + EXPECT_THROW(u::make_json_builder(names_index), u::fatal_error); + + const char* names_property = R"({ + "materials": [{"type": "props_scalar", "name": "K"}], + "constants": ["K::value"] + })"; + EXPECT_NO_THROW(u::make_json_builder(names_property)); +} + /// Same material NAME, changed constants, no rebuild and no fatal error — the /// registry's PROPS-consistency check must stand down for a model that reads /// its constants per call, because there is nothing baked in to contradict. From 270c2574db77a31b5fdcf1753e7d05eff6b51e5f Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 23:04:03 +0200 Subject: [PATCH 6/9] umat: shorten the comments --- .../numsim-materials/umat/umat_interface.h | 21 ++++++------------- tests/test_umat_interface.cpp | 17 +++++++-------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/include/numsim-materials/umat/umat_interface.h b/include/numsim-materials/umat/umat_interface.h index 27719cc..3e1859c 100644 --- a/include/numsim-materials/umat/umat_interface.h +++ b/include/numsim-materials/umat/umat_interface.h @@ -207,9 +207,8 @@ class umat_registry { std::unique_ptr ctx; std::unique_ptr solid; std::unique_ptr ps; - /// The constants the context was built from. The graph is built once and - /// reused, so a later call arriving with different ones would mean the - /// cached parameters no longer describe this material. + /// What the context was built from; the graph is reused, so different + /// constants on a later call would no longer describe this material. std::vector props; }; @@ -229,18 +228,10 @@ class umat_registry { const auto key = normalise_cmname(cmname.data(), cmname.size(), buf); auto& cache = thread_cache(); if (auto it = cache.find(key); it != cache.end()) { - // PROPS cannot vary for a given material name — two *MATERIAL blocks must - // have distinct names — so anything different means the deck contradicts - // the cached graph, and the constants baked into it would be silently - // wrong for every subsequent call. Comparing the VALUES, not just the - // count: a same-length array with different numbers is the case that - // actually reaches a material, and NPROPS doubles is nothing next to an - // evaluation. - // - // Two different faults, so two messages. A wrong count is a deck error; - // equal counts with different numbers is a dispatch error, and there the - // useful thing is WHICH constant disagrees — a check that only ever - // explains is worth the words. + // PROPS cannot vary for one material name, so anything different + // contradicts the graph the constants were baked into. Values, not just + // the count: same length with different numbers is the case that reaches + // a material. Two faults, two messages — the check only ever explains. if (it->second.props.size() != props.size()) throw fatal_error( "numsim UMAT: material '" + std::string(key) + "' was built from " + diff --git a/tests/test_umat_interface.cpp b/tests/test_umat_interface.cpp index 5285802..5f7078c 100644 --- a/tests/test_umat_interface.cpp +++ b/tests/test_umat_interface.cpp @@ -166,9 +166,8 @@ struct Registration { // against an already-built name the NPROPS-consistency check fires first // and require_props is never reached. registry::instance().register_model("COLDNAME", build_deck_elastic, de); - // Likewise used by exactly one test: it has to warm the cache itself with a - // known set of constants, so any other test touching it would decide the - // outcome. + // One test only: it warms the cache itself, so a shared name would let + // test order decide the outcome. registry::instance().register_model("VALUEPROBE", build_deck_elastic, de); // Deliberately lower-case, to prove the registry folds case on both sides. @@ -769,17 +768,16 @@ TEST(UmatInterface, ChangingNpropsForTheSameNameIsFatal) { call_umat("STIFF", stress, statev.data(), ddsdde, stran, dstran, 0.0, 0.1, 3, 3, 6, 0, &pnewdt, nullptr, nullptr, nullptr, nullptr, three, 3); EXPECT_EQ(FatalProbe::count, 1); - // Both counts, so a user reading NPROPS=3 is not left matching one number. + // Both counts, so NPROPS=3 is not left to be matched against one number. EXPECT_NE(FatalProbe::last.find("2 constants"), std::string::npos) << FatalProbe::last; EXPECT_NE(FatalProbe::last.find("supplies 3"), std::string::npos) << FatalProbe::last; } -/// The same contradiction with the count held fixed — two constants either way, -/// different numbers. This is the case that actually reaches a material: a -/// count check accepts it and every subsequent call silently returns the FIRST -/// call's stiffness, giving a converged analysis with the wrong moduli. +/// Same count, different numbers — the case that reaches a material. A count +/// check accepts it and serves the first call's stiffness forever: a converged +/// analysis with the wrong moduli. TEST(UmatInterface, ChangingPropsValuesForTheSameNameIsFatal) { FatalProbe probe; std::vector statev(1, 0.0); @@ -801,8 +799,7 @@ TEST(UmatInterface, ChangingPropsValuesForTheSameNameIsFatal) { EXPECT_EQ(FatalProbe::count, 1) << "a same-length PROPS with different numbers must be reported, not " "silently served from the cached graph"; - // The message has to name WHICH constant disagrees and both values: this - // check never changes a result, it only ever explains one. + // Must name which constant disagrees, and both values. EXPECT_NE(FatalProbe::last.find("constant 1"), std::string::npos) << FatalProbe::last; EXPECT_NE(FatalProbe::last.find("100.0"), std::string::npos) From 4355591111fb42cfbd593ebb7f087b0949a08e6e Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 23:10:13 +0200 Subject: [PATCH 7/9] umat: shorten the comments --- .../numsim-materials/materials/props_scalar.h | 43 ++++------ include/numsim-materials/umat/json_model.h | 25 +++--- .../umat/material_point_evaluator.h | 43 ++++------ .../umat/plane_stress_evaluator.h | 5 +- tests/test_props_scalar.cpp | 82 +++++++------------ 5 files changed, 77 insertions(+), 121 deletions(-) diff --git a/include/numsim-materials/materials/props_scalar.h b/include/numsim-materials/materials/props_scalar.h index 642463d..b4f5f64 100644 --- a/include/numsim-materials/materials/props_scalar.h +++ b/include/numsim-materials/materials/props_scalar.h @@ -12,27 +12,22 @@ namespace numsim::materials { /// A scalar taken from the host's material-constants array on every call. /// -/// constant_scalar bakes its number into the graph at construction, which is -/// right when the constants are fixed for a material name — Abaqus PROPS, -/// where two *MATERIAL blocks must have distinct names. This one records only -/// WHICH slot it owns and reads the number from the host each call, for hosts -/// whose constants genuinely vary: CalculiX interpolates the *USER MATERIAL -/// constants by temperature, so they can differ from one call to the next. +/// constant_scalar bakes its number in at construction, which is right when the +/// constants are fixed for a material name (Abaqus PROPS). This one records only +/// WHICH slot it owns, for hosts whose constants vary — CalculiX interpolates +/// them by temperature. /// -/// Rebuilding the graph per call would also be correct — nothing is retained -/// between calls — but it measures 22x an evaluation even from a hand-written -/// builder (6.7 us against 302 ns), against nothing at all for reading a slot. +/// Rebuilding the graph per call would also be correct, since nothing is +/// retained between calls, but it measures 22x an evaluation (6.7 us against +/// 302 ns) where reading a slot is free. /// -/// bind() DEREFERENCES. It never stores the pointer, and that is the whole -/// safety argument: a host constants array may be a per-call temporary, so a -/// kept pointer reads a dead stack slot on the next call — and usually returns -/// the right number anyway, because the slot is commonly reused. That failure -/// survives every test and breaks somewhere else. +/// bind() DEREFERENCES and keeps no pointer: a host array may be a per-call +/// temporary, and a kept pointer would read a dead stack slot next call — +/// usually returning the right number, because the slot is commonly reused. /// -/// The property is PLAIN and there is NO update callback, exactly as in -/// constant_scalar. Two consequences, both load-bearing: statev_map never sees -/// it, so it costs no STATEV slot; and the value is in place before -/// ctx.update() runs, so where the sort happens to put it cannot matter. +/// Plain property, no update callback, as constant_scalar: nothing reaches +/// statev_map, and the value is in place before ctx.update() so ordering +/// cannot matter. /// /// Parameters: /// "name": material name @@ -48,11 +43,10 @@ class props_scalar final : public material_base, Traits> { explicit props_scalar(Args&&... args) : base(std::forward(args)...), m_value(base::template add_output("value")), - // By value, not by reference into the parameter store: a later insert() - // can relocate anything past the handler's small buffer. + // By value: a later insert() can relocate anything past the handler's + // small buffer. m_index(base::template get_parameter("index")) { - // Until the first bind(). A model that never binds would otherwise publish - // whatever the property storage happened to hold. + // Until the first bind(), rather than whatever the storage held. m_value = value_type{}; } @@ -64,9 +58,8 @@ class props_scalar final : public material_base, Traits> { /// Copy this material's constant out of the host's array. /// - /// Callers under the UMAT layer go through material_point_evaluator, which - /// range-checks every reader once per call and reports a short array as the - /// setup fault it is. The check here is the backstop for direct C++ use. + /// material_point_evaluator range-checks every reader once per call, so this + /// is the backstop for direct C++ use. void bind(std::span props) { if (m_index >= props.size()) throw std::out_of_range( diff --git a/include/numsim-materials/umat/json_model.h b/include/numsim-materials/umat/json_model.h index 06473bc..41ce973 100644 --- a/include/numsim-materials/umat/json_model.h +++ b/include/numsim-materials/umat/json_model.h @@ -77,26 +77,23 @@ void ensure_materials_registered() { }); } -/// What a binding writes into the document. -/// -/// One function for both the validation and the substitution below: checking -/// one parameter name and then writing a different one is exactly how a target -/// ends up half-verified, which is the defect this validation exists to close. +/// What a binding writes into the document. One function for both the +/// validation and the substitution below — checking one parameter name and +/// writing another is how a target ends up half-verified. struct constant_binding_target { std::string parameter; - /// props_scalar is told which SLOT it owns and reads the number itself on - /// every call; everything else has the number written in now. + /// props_scalar is told its SLOT and reads the number itself each call; + /// everything else has the number written in now. bool writes_slot{false}; }; inline constant_binding_target bound_parameter( const nlohmann::json& material, const connection_source& binding) { if (material.value("type", std::string{}) == "props_scalar") { - // The target names the graph property the constant will arrive on, which - // props_scalar publishes as "value" just as constant_scalar does. Keeping - // the spelling identical across the two binding times is the point — - // swapping the type must not force "constants" to be rewritten — so the - // parameter actually written ("index") is not what the document says. + // The target names the property the constant arrives on — "value", as for + // constant_scalar — so swapping the type does not force "constants" to be + // rewritten. The parameter written ("index") is therefore not what the + // document says. if (binding.property != "value") throw fatal_error( "json_model: a props_scalar target names the property it publishes, " @@ -245,8 +242,8 @@ typename umat_registry::builder make_json_builder( if (!material.contains("name") || material["name"].get() != bindings[i].material) continue; - // Same binding, two binding TIMES — resolved by the same function that - // validated the target at registration, so the two cannot drift. + // Resolved by the function that validated the target, so the two + // cannot drift. const auto target = bound_parameter(material, bindings[i]); material[target.parameter] = target.writes_slot ? nlohmann::json(i) : nlohmann::json(props[i]); diff --git a/include/numsim-materials/umat/material_point_evaluator.h b/include/numsim-materials/umat/material_point_evaluator.h index 74045c3..04ce250 100644 --- a/include/numsim-materials/umat/material_point_evaluator.h +++ b/include/numsim-materials/umat/material_point_evaluator.h @@ -130,9 +130,8 @@ class material_point_evaluator { m_statev = std::make_unique>(m_ctx, exclusions); - // Materials that read the host's constants on every call. Collected rather - // than configured: listing them would be a second place to keep in step - // with the graph, and a missed entry is a silently stale modulus. + // Collected rather than configured: a hand-listed set is a second thing to + // keep in step with the graph, and a missed entry is a stale modulus. for (auto* material : m_ctx.materials()) if (auto* reader = dynamic_cast*>(material)) { m_props_readers.push_back(reader); @@ -200,15 +199,11 @@ class material_point_evaluator { value_type dtime, value_type* stress6, value_type* tangent36, std::span drot = {}) { - // A model that reads its constants per call publishes zero for every one of - // them until bind_props() runs. Left to itself that means moduli of zero, an - // all-zero DDSDDE and a host that fails to converge with nothing naming the - // cause — a plausible number for a modulus, and a degenerate one. - // - // Catches never-bound, not stale: the plane-stress solve runs this repeatedly - // for one host call and must not have to re-bind per iterate, so there is no - // way to tell "deliberately the same constants" from "forgot to re-bind". - // Under the registry that gap does not exist — it binds on every call. + // Every reader publishes zero until bind_props() runs: moduli of zero, an + // all-zero DDSDDE, and a host that fails to converge with nothing naming + // the cause. Catches never-bound, not stale — the plane-stress solve + // re-runs this per iterate without re-binding, so the two are + // indistinguishable from here. The registry binds on every call. if (!m_props_readers.empty() && !m_props_bound) throw fatal_error( "material_point_evaluator: this model reads its material constants " @@ -234,15 +229,10 @@ class material_point_evaluator { tangent_to_buffer(*m_tangent, tangent36); } - /// Copy the host's material constants into the graph. - /// - /// Call once per host call, BEFORE evaluate()/evaluate_canonical(). Separate - /// from `call` on purpose: the plane-stress solve runs the graph repeatedly - /// for one host call, and the constants are the same for every iterate, so - /// they bind once outside that loop. - /// - /// A no-op unless the model contains props_scalar materials — a model whose - /// constants were baked in at build time never reaches the loop. + /// Copy the host's material constants into the graph, once per host call and + /// before evaluating. Separate from `call` because the plane-stress solve + /// runs the graph repeatedly for one call and the constants do not change + /// between iterates. A no-op without props_scalar materials. void bind_props(std::span props) { if (m_props_readers.empty()) return; if (props.size() < m_props_needed) @@ -260,13 +250,12 @@ class material_point_evaluator { return !m_props_readers.empty(); } - /// True when host constant @p slot is read per call, so a changed value there - /// is the model working as intended rather than a contradiction. + /// True when host constant @p slot is read per call, so a change there is + /// intended rather than a contradiction. /// - /// Per slot, not per model: a document may mix the two binding times — a - /// temperature-dependent modulus beside a genuinely fixed yield stress — and - /// answering this for the whole model would wave the baked constants through - /// as well, leaving them silently at their first value. + /// Per slot, not per model: a document may mix the two binding times, and + /// answering for the whole model would wave the BAKED constants through as + /// well, leaving them silently at their first value. [[nodiscard]] bool is_live_prop(std::size_t slot) const noexcept { return slot < m_live_slots.size() && m_live_slots[slot]; } diff --git a/include/numsim-materials/umat/plane_stress_evaluator.h b/include/numsim-materials/umat/plane_stress_evaluator.h index 8d5de81..26b9465 100644 --- a/include/numsim-materials/umat/plane_stress_evaluator.h +++ b/include/numsim-materials/umat/plane_stress_evaluator.h @@ -66,9 +66,8 @@ class plane_stress_evaluator { /// Iterations the last evaluate() needed, for diagnostics. [[nodiscard]] int last_iterations() const noexcept { return m_last_iters; } - /// Bind the host's constants once for the whole out-of-plane solve. They do - /// not depend on the iterate, so binding inside the loop would be repeated - /// work with no effect. + /// Once for the whole out-of-plane solve: the constants do not depend on the + /// iterate. void bind_props(std::span props) { m_inner->bind_props(props); } diff --git a/tests/test_props_scalar.cpp b/tests/test_props_scalar.cpp index 2994296..2ff8e94 100644 --- a/tests/test_props_scalar.cpp +++ b/tests/test_props_scalar.cpp @@ -50,8 +50,7 @@ TEST(PropsScalar, PublishesTheConstantAtItsIndex) { EXPECT_DOUBLE_EQ(ctx.get("K", "value"), 22.0); } -/// Before the first bind the property must be a definite value rather than -/// whatever the property storage happened to hold. +/// A definite value before the first bind, not whatever the storage held. TEST(PropsScalar, IsZeroBeforeTheFirstBind) { ctx_type ctx; param_type p; @@ -74,9 +73,8 @@ TEST(PropsScalar, RejectsAnIndexPastTheSuppliedConstants) { EXPECT_THROW(k.bind(std::span(props, 2)), std::out_of_range); } -/// Plain, not history — so it costs no STATEV slot. This is the property that -/// makes it usable for a modulus at all; external_scalar_source would consume -/// one slot per constant. +/// Plain, not history: no STATEV slot. external_scalar_source would cost one +/// per constant. TEST(PropsScalar, CostsNoStatevSlot) { ctx_type ctx; param_type p; @@ -167,8 +165,7 @@ T call_once(u::material_point_evaluator& eval, return ddsdde[0]; } -/// The whole point: the same graph, two different constant sets, two different -/// stiffnesses — with no rebuild in between. +/// The point: one graph, two constant sets, two stiffnesses, no rebuild. TEST(PropsScalar, ChangedConstantsChangeTheTangentWithoutARebuild) { ctx_type ctx; build_live(ctx); @@ -183,13 +180,9 @@ TEST(PropsScalar, ChangedConstantsChangeTheTangentWithoutARebuild) { EXPECT_NEAR(call_once(eval, soft), c1111(100.0, 40.0), 1e-9); } -/// The stored-pointer trap. A host constants array may be a per-call temporary; -/// an implementation that kept `const double*` and dereferenced it during -/// update() would read whatever now occupies that memory. -/// -/// Here the buffer is deliberately overwritten AFTER bind_props and before the -/// graph runs. Dereferencing at bind time makes that irrelevant; keeping the -/// pointer makes the tangent follow the clobbered values. +/// The stored-pointer trap. The buffer is overwritten AFTER bind_props and +/// before the graph runs: dereferencing at bind time makes that irrelevant, +/// keeping the pointer makes the tangent follow the clobbered values. TEST(PropsScalar, ReadsTheConstantsAtBindTimeNotAtUpdateTime) { ctx_type ctx; build_live(ctx); @@ -219,8 +212,7 @@ TEST(PropsScalar, ReadsTheConstantsAtBindTimeNotAtUpdateTime) { } /// Every reader publishes zero until bind_props runs, so an unbound model would -/// otherwise evaluate with moduli of zero: an all-zero DDSDDE, a host that fails -/// to converge, and nothing naming the cause. +/// evaluate with moduli of zero and an all-zero DDSDDE. TEST(PropsScalar, EvaluatingBeforeBindingIsFatal) { ctx_type ctx; build_live(ctx); @@ -245,10 +237,8 @@ TEST(PropsScalar, EvaluatingBeforeBindingIsFatal) { EXPECT_NEAR(ddsdde[0], c1111(100.0, 40.0), 1e-9); } -/// The plane-stress path binds once for the whole out-of-plane solve. Nothing -/// else here exercises it, and it is the one place binding meets an ITERATIVE -/// evaluator — if a change moved the bind inside the loop, or dropped the -/// forward, only this notices. +/// The one place binding meets an ITERATIVE evaluator. If a change moved the +/// bind inside the loop, or dropped the forward, only this notices. TEST(PropsScalar, PlaneStressUsesTheLiveConstants) { ctx_type ctx; build_live(ctx); @@ -270,15 +260,14 @@ TEST(PropsScalar, PlaneStressUsesTheLiveConstants) { ps.bind_props(props); ps.evaluate(c); - // Condensed plane-stress modulus, from the constants the host supplied. + // Condensed modulus, derived from the constants the host supplied. const T E = 9 * K * G / (3 * K + G); const T nu = (3 * K - 2 * G) / (2 * (3 * K + G)); EXPECT_NEAR(ddsdde[0], E / (1 - nu * nu), 1e-9); EXPECT_NEAR(stress[2], 0.0, 1e-10) << "sigma_33 must be driven to zero"; } -/// Too few constants is a *USER MATERIAL setup error, so it must be fatal -/// rather than something a smaller increment could fix. +/// Too few constants is a setup error: fatal, not a cutback. TEST(PropsScalar, TooFewConstantsIsFatalAtTheEvaluator) { ctx_type ctx; build_live(ctx); @@ -288,8 +277,7 @@ TEST(PropsScalar, TooFewConstantsIsFatalAtTheEvaluator) { EXPECT_THROW(eval.bind_props(std::span(only_one, 1)), u::fatal_error); } -/// A model with baked constants must not pay for machinery it does not use, -/// and must still report itself as such to the registry. +/// A baked-constant model reports itself as such and pays for nothing. TEST(PropsScalar, BakedConstantModelsHaveNoLiveProps) { ctx_type ctx; param_type p; @@ -325,8 +313,8 @@ TEST(PropsScalar, BakedConstantModelsHaveNoLiveProps) { // Through JSON and the real umat_ entry point // --------------------------------------------------------------------------- -/// Identical to the baked document except for the material type — which is the -/// claim the JSON layer makes, so it is worth asserting rather than describing. +/// Identical to the baked document but for the material type — the claim the +/// JSON layer makes, so it is asserted rather than described. const char* kLive = R"({ "materials": [ {"type": "external_strain_source", "name": "strain_in"}, @@ -348,9 +336,8 @@ registry::config json_config() { return cfg; } -/// K baked, G live, both listed in "constants". A document is free to mix the -/// two binding times — a temperature-dependent modulus beside a genuinely fixed -/// one — and the consistency check has to be answered per slot, not per model. +/// K baked, G live. A document may mix the two binding times, so the +/// consistency check has to be answered per slot rather than per model. const char* kMixed = R"({ "materials": [ {"type": "external_strain_source", "name": "strain_in"}, @@ -367,8 +354,8 @@ const char* kMixed = R"({ struct Registration { Registration() { u::register_json_model("LIVEELASTIC", kLive, json_config()); - // One name per mixed test: each has to warm the cache itself with known - // constants, so sharing a name would let test order decide the outcome. + // One name per test: each warms the cache itself, so sharing would let + // test order decide the outcome. u::register_json_model("MIXEDBAKED", kMixed, json_config()); u::register_json_model("MIXEDLIVE", kMixed, json_config()); } @@ -408,19 +395,16 @@ T uniaxial_tangent(const std::string& name, const T* props, int nprops) { return ddsdde[0]; } -/// The constants array position is the slot: no "index" appears in the -/// document, yet K takes PROPS[0] and G takes PROPS[1]. +/// Position is the slot: no "index" in the document, yet K takes PROPS[0]. TEST(PropsScalarJson, ConstantsPositionBindsTheSlot) { const T props[2] = {250.0, 90.0}; EXPECT_NEAR(uniaxial_tangent("LIVEELASTIC", props, 2), c1111(250.0, 90.0), 1e-9); } -/// The target names the graph property the constant arrives on — "value" — and -/// not the "index" parameter the builder actually writes. Keeping the spelling -/// identical to constant_scalar's is what lets the type be swapped without -/// rewriting "constants", so the wrong spelling has to be rejected rather than -/// quietly accepted. +/// The target names the property the constant arrives on — "value" — not the +/// "index" the builder writes. Identical spelling to constant_scalar is what +/// lets the type be swapped, so the wrong one is rejected. TEST(PropsScalarJson, RejectsATargetNamingIndexRatherThanTheProperty) { const char* names_index = R"({ "materials": [{"type": "props_scalar", "name": "K"}], @@ -435,9 +419,8 @@ TEST(PropsScalarJson, RejectsATargetNamingIndexRatherThanTheProperty) { EXPECT_NO_THROW(u::make_json_builder(names_property)); } -/// Same material NAME, changed constants, no rebuild and no fatal error — the -/// registry's PROPS-consistency check must stand down for a model that reads -/// its constants per call, because there is nothing baked in to contradict. +/// The registry's consistency check stands down for live constants: nothing +/// was baked in for a changed array to contradict. TEST(PropsScalarJson, ChangedDeckConstantsAreHonouredNotRejected) { const T first[2] = {100.0, 40.0}; const T second[2] = {300.0, 140.0}; @@ -452,11 +435,9 @@ TEST(PropsScalarJson, ChangedDeckConstantsAreHonouredNotRejected) { // Mixed documents: baked and live constants side by side // --------------------------------------------------------------------------- -/// Changing a BAKED constant must still be fatal even though the same document -/// has a live one. Answering has_live_props() for the whole model instead let -/// this through: G tracked the deck while K silently kept its first value, and -/// the returned tangent was wrong-but-plausible with no diagnostic — exactly -/// the failure the check exists to catch. +/// A changed BAKED constant is still fatal beside a live one. Answering +/// has_live_props() per model let this through: G tracked the deck while K +/// kept its first value, with no diagnostic. TEST(PropsScalarJson, ChangingABakedConstantIsFatalEvenBesideALiveOne) { const T first[2] = {100.0, 40.0}; const T changed_both[2] = {300.0, 140.0}; // K baked, G live @@ -473,15 +454,12 @@ TEST(PropsScalarJson, ChangingABakedConstantIsFatalEvenBesideALiveOne) { EXPECT_EQ(fatal_count, 1) << "a changed baked constant must be reported, not served from the " "cached graph because some OTHER constant is live"; - // The wrong-but-plausible answer this used to return, named so a regression - // cannot pass by returning it. + // The wrong-but-plausible answer this used to return. EXPECT_FALSE(std::abs(got - c1111(100.0, 140.0)) < 1e-9) << "K kept its first value while G followed the deck"; } -/// The other half: with the baked constant left alone, changing only the live -/// one is honoured. Without this, the fix above could simply reject every mixed -/// document and still pass. +/// The other half, so the fix cannot pass by rejecting every mixed document. TEST(PropsScalarJson, ChangingOnlyTheLiveConstantIsHonouredInAMixedDocument) { const T first[2] = {100.0, 40.0}; const T live_changed[2] = {100.0, 140.0}; // K unchanged, G changed From fa83292d147f70021b8d9de56b0bac257064fcf5 Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 23:42:15 +0200 Subject: [PATCH 8/9] umat: cover the registry's plane-stress bind PropsScalar.PlaneStressUsesTheLiveConstants drives plane_stress_evaluator directly, so it covered the forward inside the evaluator and left the registry dispatch that calls it uncovered -- deleting ts.ps->bind_props(props) passed the whole suite. The half that was fixed was the half nothing was going to break. Now driven through umat_ with NDI=2, NSHR=1, asserting the condensed E/(1-v^2) derived from the constants the host supplied. With the registry line removed it aborts on the never-bound guard, so the test is load-bearing either way. --- tests/test_props_scalar.cpp | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test_props_scalar.cpp b/tests/test_props_scalar.cpp index 2ff8e94..e8026c7 100644 --- a/tests/test_props_scalar.cpp +++ b/tests/test_props_scalar.cpp @@ -395,6 +395,41 @@ T uniaxial_tangent(const std::string& name, const T* props, int nprops) { return ddsdde[0]; } +/// The registry's plane-stress dispatch, which binds through a different call +/// than the solid path. PropsScalar.PlaneStressUsesTheLiveConstants covers the +/// evaluator's forward; nothing covered the registry line that calls it, so +/// deleting it passed the whole suite. +TEST(PropsScalarJson, PlaneStressBindsThroughTheRegistry) { + constexpr T K = 100.0, G = 40.0; + const T props[2] = {K, G}; + const fortran_name cm("LIVEELASTIC"); + + T statev[2] = {0, 0}; + const T stran[3] = {0, 0, 0}; + const T dstran[3] = {0.001, 0, 0}; + T stress[3] = {0}, ddsdde[9] = {0}, pnewdt = 1.0; + T sse = 0, spd = 0, scd = 0, rpl = 0, ddsddt[3] = {0}, drplde[3] = {0}; + T drpldt = 0; + const T time[2] = {0, 0}; + T dtime = 0.1; + const T temp = 0, dtemp = 0, predef = 0, dpred = 0, celent = 1; + const T coords[3] = {0}, drot[9] = {1, 0, 0, 0, 1, 0, 0, 0, 1}; + const T dfg[9] = {1, 0, 0, 0, 1, 0, 0, 0, 1}; + int noel = 1, npt = 1, layer = 1, kspt = 1, jstep = 1, kinc = 1; + int ndi = 2, nshr = 1, ntens = 3, nstatv = 2, nprops = 2; + + umat_(stress, statev, ddsdde, &sse, &spd, &scd, &rpl, ddsddt, drplde, &drpldt, + stran, dstran, time, &dtime, &temp, &dtemp, &predef, &dpred, cm.buf, + &ndi, &nshr, &ntens, &nstatv, props, &nprops, coords, drot, &pnewdt, + &celent, dfg, dfg, &noel, &npt, &layer, &kspt, &jstep, &kinc, 80); + + EXPECT_DOUBLE_EQ(pnewdt, 1.0); + const T E = 9 * K * G / (3 * K + G); + const T nu = (3 * K - 2 * G) / (2 * (3 * K + G)); + EXPECT_NEAR(ddsdde[0], E / (1 - nu * nu), 1e-9) + << "the constants must reach the material on the plane-stress path too"; +} + /// Position is the slot: no "index" in the document, yet K takes PROPS[0]. TEST(PropsScalarJson, ConstantsPositionBindsTheSlot) { const T props[2] = {250.0, 90.0}; From c010efc72423af3517256f207e9698637d06a91f Mon Sep 17 00:00:00 2001 From: petlenz Date: Tue, 18 Aug 2026 21:35:45 +0200 Subject: [PATCH 9/9] materials: drop two comments from props_scalar --- include/numsim-materials/materials/props_scalar.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/include/numsim-materials/materials/props_scalar.h b/include/numsim-materials/materials/props_scalar.h index b4f5f64..2dd68bf 100644 --- a/include/numsim-materials/materials/props_scalar.h +++ b/include/numsim-materials/materials/props_scalar.h @@ -43,8 +43,6 @@ class props_scalar final : public material_base, Traits> { explicit props_scalar(Args&&... args) : base(std::forward(args)...), m_value(base::template add_output("value")), - // By value: a later insert() can relocate anything past the handler's - // small buffer. m_index(base::template get_parameter("index")) { // Until the first bind(), rather than whatever the storage held. m_value = value_type{}; @@ -57,9 +55,6 @@ class props_scalar final : public material_base, Traits> { } /// Copy this material's constant out of the host's array. - /// - /// material_point_evaluator range-checks every reader once per call, so this - /// is the backstop for direct C++ use. void bind(std::span props) { if (m_index >= props.size()) throw std::out_of_range(