From e0bf450b40c6e7e893845a9db37b1a375a890e93 Mon Sep 17 00:00:00 2001 From: petlenz Date: Sun, 16 Aug 2026 09:11:00 +0200 Subject: [PATCH 1/3] 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 4420ded79670f145c68503d465ca07f44f8de26a Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 22:50:01 +0200 Subject: [PATCH 2/3] 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 270c2574db77a31b5fdcf1753e7d05eff6b51e5f Mon Sep 17 00:00:00 2001 From: petlenz Date: Mon, 17 Aug 2026 23:04:03 +0200 Subject: [PATCH 3/3] 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)