From 06136f2a78e9f2adeebed4ea9aececdedca57738 Mon Sep 17 00:00:00 2001 From: petlenz Date: Wed, 12 Aug 2026 10:07:09 +0200 Subject: [PATCH 1/3] core: write a material parameter in place, without invalidating bound references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Materials bind their parameters once, in the constructor: m_K(base::template get_parameter("K")) and hold the result as a `const value_type&`. That reference points at the object inside the std::any inside the handler's map node, so a parameter can be updated after construction and every material observes it — but only if the write leaves the object where it is. template void set_parameter(std::string const& key, T const& value) { m_parameter_handler.template get(key) = value; } Assigning through the NON-CONST get() is the whole point, and insert() is deliberately not used. insert() goes through insert_or_assign, which replaces the entire std::any; for a value larger than std::any's small buffer that destroys the contained object and constructs a new one elsewhere, dangling every reference a material bound at construction. The distinction is invisible with scalar parameters, which is what makes it worth a test rather than a comment: a `double` fits the small buffer, so insert() happens to preserve its address and an implementation built on insert() would pass any test written against moduli — then break the first time someone stored a tensor-valued parameter. test_set_parameter asserts the address is stable for a 256-byte parameter, and separately pins the underlying parameter_handler behaviour (insert relocates, assignment does not) so the rationale lives next to the mechanism rather than only in this message. This does nothing about quantities DERIVED from parameters. A material that precomputes something in its constructor will not notice a later write; that is a per-material concern and is handled where the derived value lives. Motivation is host-driven material constants: Abaqus fixes PROPS per material name, but CalculiX interpolates *USER MATERIAL constants by temperature, so they genuinely vary between calls and the graph has to be able to follow them. --- .../core/material_interface.h | 17 +++ tests/CMakeLists.txt | 1 + tests/test_set_parameter.cpp | 144 ++++++++++++++++++ 3 files changed, 162 insertions(+) create mode 100644 tests/test_set_parameter.cpp diff --git a/include/numsim-materials/core/material_interface.h b/include/numsim-materials/core/material_interface.h index a14760b..bee9d26 100644 --- a/include/numsim-materials/core/material_interface.h +++ b/include/numsim-materials/core/material_interface.h @@ -51,6 +51,23 @@ class material_interface { return m_parameter_handler.template get(std::forward(key)); } + /// Overwrite a parameter in place. + /// + /// Assigns through the NON-CONST get(), deliberately not insert(). + /// insert() goes through insert_or_assign, which replaces the whole std::any; + /// for a value larger than std::any's small buffer that relocates the + /// contained object, dangling every reference a material bound at + /// construction via get_parameter(). Assigning through get() mutates the + /// contained object where it already lives, so those references stay valid + /// and observe the new value. + /// + /// Note that materials caching anything DERIVED from a parameter will not + /// notice — see isotropic_tangent's "recompute" for how that is handled. + template + void set_parameter(std::string const& key, T const& value) { + m_parameter_handler.template get(key) = value; + } + const auto& get_property_registry() const { return m_property_handler; } /// Wire all property inputs. Called at finalize(). diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8a7433d..ffd9f2f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -23,6 +23,7 @@ add_numsim_test(test_statev_map test_statev_map.cpp) add_numsim_test(test_material_point_evaluator test_material_point_evaluator.cpp) 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_set_parameter test_set_parameter.cpp) target_link_libraries(test_umat_interface PRIVATE Threads::Threads) # Data dumper for plotting (not a test — standalone executable) diff --git a/tests/test_set_parameter.cpp b/tests/test_set_parameter.cpp new file mode 100644 index 0000000..c4c7d18 --- /dev/null +++ b/tests/test_set_parameter.cpp @@ -0,0 +1,144 @@ +#include +#include +#include +#include +#include "numsim-materials/core/material_base.h" +#include "numsim-materials/core/material_context.h" + +namespace { + +namespace nm = numsim::materials; + +using policy = nm::material_policy_default; +using T = policy::value_type; +using ctx_type = nm::material_context; +using param_type = policy::ParameterHandler; + +/// A parameter large enough that std::any cannot use its small buffer. Storing +/// one is what separates a correct write API from one that merely appears to +/// work with scalars. +struct big_parameter { + std::array data{}; +}; + +/// Binds references to its parameters exactly as every real material does, and +/// exposes them so a test can observe what a bound reference sees after a write. +template +class probe_material final + : public nm::material_base, Traits> { +public: + using base = nm::material_base, Traits>; + using value_type = typename base::value_type; + using input_parameter_controller = typename base::input_parameter_controller; + + template + explicit probe_material(Args&&... args) + : base(std::forward(args)...), + m_k(base::template get_parameter("K")), + m_big(base::template get_parameter("big")) {} + + static input_parameter_controller parameters() { + input_parameter_controller para{base::parameters()}; + para.template insert("K").template add(); + para.template insert("big").template add(); + return para; + } + + const value_type& bound_k() const noexcept { return m_k; } + const big_parameter& bound_big() const noexcept { return m_big; } + +private: + const value_type& m_k; + const big_parameter& m_big; +}; + +probe_material& build(ctx_type& ctx) { + param_type p; + p.insert("name", "probe"); + p.insert("K", 100.0); + p.insert("big", big_parameter{}); + auto& m = ctx.create>(p); + ctx.finalize(); + return m; +} + +// --------------------------------------------------------------------------- + +/// A material binds `const T&` into the parameter handler at construction, so +/// the only useful write is one those references observe. +TEST(SetParameter, BoundReferenceSeesTheNewValue) { + ctx_type ctx; + auto& m = build(ctx); + + EXPECT_DOUBLE_EQ(m.bound_k(), 100.0); + m.template set_parameter("K", 250.0); + EXPECT_DOUBLE_EQ(m.bound_k(), 250.0); +} + +/// The address must not move, for any type. This is the reason set_parameter +/// assigns through the non-const get() instead of calling insert(): +/// insert() goes through insert_or_assign, which replaces the whole std::any. +/// For a value past std::any's small buffer that relocates the object and every +/// bound reference dangles — while a `double` would keep the same address by +/// accident and pass a test written only against scalars. +TEST(SetParameter, WriteDoesNotRelocateEvenForALargeType) { + ctx_type ctx; + auto& m = build(ctx); + + const void* addr_before = static_cast(&m.bound_big()); + big_parameter v; + v.data[0] = 7.0; + v.data[31] = 9.0; + m.template set_parameter("big", v); + + EXPECT_EQ(static_cast(&m.bound_big()), addr_before) + << "the bound reference must remain valid"; + EXPECT_DOUBLE_EQ(m.bound_big().data[0], 7.0); + EXPECT_DOUBLE_EQ(m.bound_big().data[31], 9.0); +} + +/// The same guarantee, stated directly against parameter_handler so the reason +/// is documented where the mechanism lives rather than only in a material. +TEST(SetParameter, InsertRelocatesALargeValueButAssignmentDoesNot) { + numsim_core::parameter_handler<> h; + h.insert("big", big_parameter{}); + const void* bound = static_cast(&h.get("big")); + + h.insert("big", big_parameter{}); + const void* after_insert = + static_cast(&h.get("big")); + + h.get("big").data[0] = 1.0; + const void* after_assign = + static_cast(&h.get("big")); + + EXPECT_NE(after_insert, bound) + << "insert() is expected to relocate; if this ever stops being true the " + "rationale for set_parameter's implementation needs revisiting"; + EXPECT_EQ(after_assign, after_insert) + << "assignment through get() must never relocate"; +} + +TEST(SetParameter, ThrowsForAnUnknownKey) { + ctx_type ctx; + auto& m = build(ctx); + EXPECT_THROW(m.template set_parameter("nosuchkey", 1.0), + std::invalid_argument); +} + +/// Writing does not disturb neighbouring parameters. +TEST(SetParameter, LeavesOtherParametersAlone) { + ctx_type ctx; + auto& m = build(ctx); + + big_parameter v; + v.data[5] = 3.0; + m.template set_parameter("big", v); + m.template set_parameter("K", 42.0); + + EXPECT_DOUBLE_EQ(m.bound_k(), 42.0); + EXPECT_DOUBLE_EQ(m.bound_big().data[5], 3.0); + EXPECT_EQ(m.name(), "probe"); +} + +} // namespace From 1b3e3fa2272c63e9c7c118d7e16e2b0734e34bce Mon Sep 17 00:00:00 2001 From: petlenz Date: Thu, 13 Aug 2026 22:18:29 +0200 Subject: [PATCH 2/3] core: shorten the comments on set_parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kept the two facts a reader cannot recover from the code — insert_or_assign relocates anything past std::any's small buffer, and derived values are not invalidated — and cut the explanation around them. --- .../core/material_interface.h | 16 ++++------- tests/test_set_parameter.cpp | 28 +++++++------------ 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/include/numsim-materials/core/material_interface.h b/include/numsim-materials/core/material_interface.h index bee9d26..1a12b01 100644 --- a/include/numsim-materials/core/material_interface.h +++ b/include/numsim-materials/core/material_interface.h @@ -51,18 +51,12 @@ class material_interface { return m_parameter_handler.template get(std::forward(key)); } - /// Overwrite a parameter in place. + /// Overwrite a parameter in place, so references bound by get_parameter() + /// stay valid and see the new value. /// - /// Assigns through the NON-CONST get(), deliberately not insert(). - /// insert() goes through insert_or_assign, which replaces the whole std::any; - /// for a value larger than std::any's small buffer that relocates the - /// contained object, dangling every reference a material bound at - /// construction via get_parameter(). Assigning through get() mutates the - /// contained object where it already lives, so those references stay valid - /// and observe the new value. - /// - /// Note that materials caching anything DERIVED from a parameter will not - /// notice — see isotropic_tangent's "recompute" for how that is handled. + /// Assigns through the non-const get(), NOT insert(): insert_or_assign + /// replaces the whole std::any, relocating anything past its small buffer. + /// Does not touch values a material DERIVED from a parameter at construction. template void set_parameter(std::string const& key, T const& value) { m_parameter_handler.template get(key) = value; diff --git a/tests/test_set_parameter.cpp b/tests/test_set_parameter.cpp index c4c7d18..8c80478 100644 --- a/tests/test_set_parameter.cpp +++ b/tests/test_set_parameter.cpp @@ -14,15 +14,12 @@ using T = policy::value_type; using ctx_type = nm::material_context; using param_type = policy::ParameterHandler; -/// A parameter large enough that std::any cannot use its small buffer. Storing -/// one is what separates a correct write API from one that merely appears to -/// work with scalars. +/// Too large for std::any's small buffer — a scalar would hide the bug. struct big_parameter { std::array data{}; }; -/// Binds references to its parameters exactly as every real material does, and -/// exposes them so a test can observe what a bound reference sees after a write. +/// Binds parameters by reference as real materials do, and exposes them. template class probe_material final : public nm::material_base, Traits> { @@ -64,8 +61,7 @@ probe_material& build(ctx_type& ctx) { // --------------------------------------------------------------------------- -/// A material binds `const T&` into the parameter handler at construction, so -/// the only useful write is one those references observe. +/// The only useful write is one the material's bound reference observes. TEST(SetParameter, BoundReferenceSeesTheNewValue) { ctx_type ctx; auto& m = build(ctx); @@ -75,12 +71,8 @@ TEST(SetParameter, BoundReferenceSeesTheNewValue) { EXPECT_DOUBLE_EQ(m.bound_k(), 250.0); } -/// The address must not move, for any type. This is the reason set_parameter -/// assigns through the non-const get() instead of calling insert(): -/// insert() goes through insert_or_assign, which replaces the whole std::any. -/// For a value past std::any's small buffer that relocates the object and every -/// bound reference dangles — while a `double` would keep the same address by -/// accident and pass a test written only against scalars. +/// The address must not move, for any type — which is why set_parameter +/// assigns rather than calling insert(). TEST(SetParameter, WriteDoesNotRelocateEvenForALargeType) { ctx_type ctx; auto& m = build(ctx); @@ -97,8 +89,8 @@ TEST(SetParameter, WriteDoesNotRelocateEvenForALargeType) { EXPECT_DOUBLE_EQ(m.bound_big().data[31], 9.0); } -/// The same guarantee, stated directly against parameter_handler so the reason -/// is documented where the mechanism lives rather than only in a material. +/// The same guarantee against parameter_handler directly, so the rationale +/// lives next to the mechanism. TEST(SetParameter, InsertRelocatesALargeValueButAssignmentDoesNot) { numsim_core::parameter_handler<> h; h.insert("big", big_parameter{}); @@ -113,8 +105,8 @@ TEST(SetParameter, InsertRelocatesALargeValueButAssignmentDoesNot) { static_cast(&h.get("big")); EXPECT_NE(after_insert, bound) - << "insert() is expected to relocate; if this ever stops being true the " - "rationale for set_parameter's implementation needs revisiting"; + << "insert() is expected to relocate; if that changes, revisit why " + "set_parameter assigns instead"; EXPECT_EQ(after_assign, after_insert) << "assignment through get() must never relocate"; } @@ -126,7 +118,7 @@ TEST(SetParameter, ThrowsForAnUnknownKey) { std::invalid_argument); } -/// Writing does not disturb neighbouring parameters. +/// No collateral damage to neighbouring parameters. TEST(SetParameter, LeavesOtherParametersAlone) { ctx_type ctx; auto& m = build(ctx); From 8fb5e8a9bee8191d202d0f2b75a14897f9a3cfdc Mon Sep 17 00:00:00 2001 From: petlenz Date: Thu, 13 Aug 2026 22:23:41 +0200 Subject: [PATCH 3/3] core: harden set_parameter against the ways a write can silently do nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six of the ten review findings on this PR. Two are fixed in the API, one is guarded, and the rest are limitations now pinned by tests against real materials rather than left for a user to discover. T is no longer deduced. std::type_identity_t makes the stored type an explicit argument, so set_parameter("K", 250) is fine while set_parameter("K", 250) fails to COMPILE. Previously it deduced int, threw std::bad_any_cast, and lost the write — and bad_any_cast derives from neither invalid_argument nor runtime_error, so a UMAT boundary catching those would miss it and terminate. A remaining mismatch (set_parameter against a stored double) is now translated into an invalid_argument naming the key, instead of a bare "bad any_cast" naming nothing. Writing "name" is rejected. The identity is cached in m_name at construction and used as the material_handler registry key, so a write left the parameter disagreeing with both: name() and lookups kept the old value while get_parameter("name") reported one that resolved to nothing. The remaining findings are real but not fixable at this level, so they are documented precisely and pinned by tests. A write reaches only what the material re-reads through its bound reference. It does not affect anything derived at construction, copied into a member, or consumed once for wiring; and it is local to one material, because each holds its own copy of the handler. The doc comment previously claimed more than that, and pointed at isotropic_tangent's "recompute" as the mitigation — a class that does not exist on this branch. The test gap was the reason all of this shipped. Every case went through a probe material that binds each parameter by reference and derives nothing, which is precisely the shape the API handles cleanly. Three new tests use SHIPPED materials and pin the awkward truth instead: WriteLandsButDerivedStateGoesStale linear_elasticity: K changes, stress does not, because the tangent was built once WriteIsLocalToTheMaterial the caller's handler keeps the old value WritingAWiringKeyDoesNotRewire a source name written after finalize() leaves the input wired as before Plus guards for the two fixes, both mutation-verified: removing the name check and breaking the bad_any_cast translation each fail exactly their own test. Not addressed: parameter_handler::insert remains public and still relocates, so set_parameter only avoids the hazard for callers who go through it. And the underlying gap — no way to invalidate anything derived from a parameter — is a framework-level design question, currently answered per-material by isotropic_tangent's "recompute" flag on the child branch. 184 -> 190 tests. --- .../core/material_interface.h | 32 +++- tests/test_set_parameter.cpp | 144 ++++++++++++++++++ 2 files changed, 173 insertions(+), 3 deletions(-) diff --git a/include/numsim-materials/core/material_interface.h b/include/numsim-materials/core/material_interface.h index 1a12b01..cff881f 100644 --- a/include/numsim-materials/core/material_interface.h +++ b/include/numsim-materials/core/material_interface.h @@ -3,6 +3,8 @@ #include #include +#include +#include #include #include #include @@ -56,10 +58,34 @@ class material_interface { /// /// Assigns through the non-const get(), NOT insert(): insert_or_assign /// replaces the whole std::any, relocating anything past its small buffer. - /// Does not touch values a material DERIVED from a parameter at construction. + /// + /// T is deliberately NOT deduced. The stored type must be named exactly, so + /// set_parameter("K", 250) is fine while set_parameter("K", 250) + /// fails to compile instead of throwing bad_any_cast at run time. + /// + /// Reaches only what the material re-reads through its bound reference. It + /// does NOT affect anything derived at construction (linear_elasticity's + /// tangent), copied into a member, or consumed once for wiring (source + /// names) — those keep their original values with no error. The write is also + /// local to THIS material, since each holds its own copy of the handler. template - void set_parameter(std::string const& key, T const& value) { - m_parameter_handler.template get(key) = value; + void set_parameter(std::string const& key, + std::type_identity_t const& value) { + // The identity is cached in m_name and used as the registry key, so a write + // here would leave the parameter disagreeing with both. + if (key == "name") + throw std::invalid_argument( + "material_interface::set_parameter(): 'name' is the material's " + "identity and cannot be changed after construction"); + try { + m_parameter_handler.template get(key) = value; + } catch (const std::bad_any_cast&) { + // Otherwise this surfaces as a bare "bad any_cast" naming nothing, and + // from a type that is neither invalid_argument nor runtime_error. + throw std::invalid_argument( + "material_interface::set_parameter('" + key + + "'): the requested type does not match the stored one"); + } } const auto& get_property_registry() const { return m_property_handler; } diff --git a/tests/test_set_parameter.cpp b/tests/test_set_parameter.cpp index 8c80478..8fefd24 100644 --- a/tests/test_set_parameter.cpp +++ b/tests/test_set_parameter.cpp @@ -1,9 +1,13 @@ #include #include #include +#include #include #include "numsim-materials/core/material_base.h" #include "numsim-materials/core/material_context.h" +#include "numsim-materials/materials/linear_elasticity.h" +#include "numsim-materials/umat/external_state_source.h" +#include namespace { @@ -13,6 +17,7 @@ using policy = nm::material_policy_default; using T = policy::value_type; using ctx_type = nm::material_context; using param_type = policy::ParameterHandler; +using tensor2 = tmech::tensor; // named: commas break the gtest macros /// Too large for std::any's small buffer — a scalar would hide the bug. struct big_parameter { @@ -133,4 +138,143 @@ TEST(SetParameter, LeavesOtherParametersAlone) { EXPECT_EQ(m.name(), "probe"); } + +// --------------------------------------------------------------------------- +// Against SHIPPED materials +// +// The probe above binds every parameter by reference and derives nothing, which +// is the one shape this API handles cleanly. These pin what happens with real +// materials, where it does not. +// --------------------------------------------------------------------------- + +/// linear_elasticity computes its tangent in the constructor and registers +/// "tangent" with no callback, so a write to K lands in the parameter and +/// changes nothing. Pinned, not endorsed: this is the limitation the API +/// documents, and the reason isotropic_tangent's "recompute" exists. +TEST(SetParameterShippedMaterials, WriteLandsButDerivedStateGoesStale) { + ctx_type ctx; + param_type p; + p.insert("name", "strain_in"); + auto& src = ctx.create>(p); + p.clear(); + p.insert("name", "elastic"); + p.insert("strain_producer_name", "strain_in"); + p.insert("K", 100.0); + p.insert("G", 40.0); + auto& el = ctx.create>(p); + ctx.finalize(); + + tensor2 eps; + eps.fill(0.0); + eps(0, 0) = 0.001; + src.bind(eps, eps); + ctx.update(); + const T before = ctx.get("elastic", "stress")(0, 0); + EXPECT_NEAR(before, (100.0 + 4.0 * 40.0 / 3.0) * 0.001, 1e-12); + + el.template set_parameter("K", 200.0); + + // The write is real... + EXPECT_DOUBLE_EQ(el.template get_parameter("K"), 200.0); + // ... and has no effect, because the tangent was built once. + ctx.update(); + EXPECT_DOUBLE_EQ(ctx.get("elastic", "stress")(0, 0), + before); +} + +/// Each material holds its own COPY of the handler, so a write reaches one +/// material and not the caller's handler or any sibling built from it. +TEST(SetParameterShippedMaterials, WriteIsLocalToTheMaterial) { + ctx_type ctx; + param_type p; + p.insert("name", "probe"); + p.insert("K", 100.0); + p.insert("big", big_parameter{}); + auto& m = ctx.create>(p); + ctx.finalize(); + + m.template set_parameter("K", 777.0); + + EXPECT_DOUBLE_EQ(m.bound_k(), 777.0); + EXPECT_DOUBLE_EQ(p.get("K"), 100.0) + << "the caller's handler is a separate copy"; +} + +/// A wiring parameter is consumed once, at construction, to build the input. +/// Writing it afterwards cannot re-wire anything. +TEST(SetParameterShippedMaterials, WritingAWiringKeyDoesNotRewire) { + ctx_type ctx; + param_type p; + p.insert("name", "strain_in"); + auto& src = ctx.create>(p); + p.clear(); + p.insert("name", "other"); + ctx.create>(p); + p.clear(); + p.insert("name", "elastic"); + p.insert("strain_producer_name", "strain_in"); + p.insert("K", 100.0); + p.insert("G", 40.0); + auto& el = ctx.create>(p); + ctx.finalize(); + + el.template set_parameter("strain_producer_name", "other"); + + tensor2 eps; + eps.fill(0.0); + eps(0, 0) = 0.002; + src.bind(eps, eps); // the ORIGINAL source + ctx.update(); + // Still reading strain_in, so the stress follows it despite the write. + EXPECT_NEAR(ctx.get("elastic", "stress")(0, 0), + (100.0 + 4.0 * 40.0 / 3.0) * 0.002, 1e-12); +} + +// --------------------------------------------------------------------------- +// Guards +// --------------------------------------------------------------------------- + +/// "name" is cached in m_name and used as the registry key, so writing it would +/// desynchronise the parameter from the material's identity. +TEST(SetParameter, RejectsWritingTheIdentityKey) { + ctx_type ctx; + auto& m = build(ctx); + EXPECT_THROW(m.template set_parameter("name", "renamed"), + std::invalid_argument); + EXPECT_EQ(m.name(), "probe"); +} + +/// A type mismatch must name the key. Without the translation it surfaces as a +/// bare "bad any_cast" that is neither invalid_argument nor runtime_error, so a +/// UMAT boundary catching those would miss it entirely. +TEST(SetParameter, TypeMismatchThrowsADiagnosticNotBadAnyCast) { + ctx_type ctx; + auto& m = build(ctx); + try { + m.template set_parameter("K", 1.0f); // "K" is stored as double + FAIL() << "expected a diagnostic"; + } catch (const std::invalid_argument& e) { + EXPECT_NE(std::string(e.what()).find("K"), std::string::npos) << e.what(); + } + EXPECT_DOUBLE_EQ(m.bound_k(), 100.0) << "the value must be unchanged"; +} + +/// T is not deduced, so a wrong-typed literal is a compile error rather than a +/// run-time throw. +template +struct deduces_t : std::false_type {}; +template +struct deduces_t().set_parameter( + std::declval(), 250))>> + : std::true_type {}; + +TEST(SetParameter, TypeIsNotDeduced) { + static_assert(!deduces_t>::value, + "set_parameter must not deduce T from the value"); + ctx_type ctx; + auto& m = build(ctx); + m.template set_parameter("K", 250); // int converts to the named double + EXPECT_DOUBLE_EQ(m.bound_k(), 250.0); +} + } // namespace