core: write a material parameter in place, without invalidating bound references - #25
core: write a material parameter in place, without invalidating bound references#25petlenz wants to merge 3 commits into
Conversation
… references
Materials bind their parameters once, in the constructor:
m_K(base::template get_parameter<value_type>("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 <typename T>
void set_parameter(std::string const& key, T const& value) {
m_parameter_handler.template get<T>(key) = value;
}
Assigning through the NON-CONST get<T>() 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.
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.
…othing
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<double>("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<float> 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<std::string>("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.
The recompute flag encoded a choice — does this stiffness follow its moduli — as a boolean, and a boolean can disagree with how the graph was actually built. It did: the flag was a live reference into the parameter store while the callback was bound once at construction, so flipping it made recomputes() claim tracking that did not exist. That was a symptom. The cause is that a parameter has no edge to the material reading it, so nothing orders a change to it against the values derived from it. K and G are now Global inputs. Which material you wire IS the choice, and it cannot desynchronise from anything: constant_scalar fixed moduli, a plain (non-history) property any "value" producer moduli that vary, e.g. with temperature constant_scalar publishes a PLAIN property deliberately. statev_map enumerates history properties, so a history-valued constant would consume one STATEV slot per integration point, per constant, and would have to be named in the exclusion list forever. Measured: with a plain property, nstatv is 0 with the moduli not mentioned at all. An earlier prototype used external_scalar_source and did cost those slots — that material is history-based because it exists for time, where the old/new pair is the point. The callback is now always bound, because inputs are not wired until finalize() and nothing can be computed in a constructor that reads them. It self-guards on the moduli, so fixed constants cost two comparisons rather than a rank-4 rebuild. Measured per ctx.update(): 28.1 ns guarded against 308.5 ns unguarded, and 31.7 ns for the old parameter-with-no-callback path — so the wired version is not slower than what it replaces, and 11x faster than rebuilding blindly. FixedModuliAreRebuiltExactlyOnce pins the guard via a recomputations() counter. One behaviour genuinely changes: the tangent is built on the first update rather than at construction, since the inputs do not exist before finalize(). Every consumer goes through ctx.update() first, but TangentIsBuiltOnTheFirstUpdateNotAtConstruction records it. EveryProducerIsOrderedBeforeItsConsumer now checks the whole chain — K and G before the stiffness, the stiffness before the stress — rather than one pair. Note this branch no longer uses set_parameter at all: the test writes through ctx.get_mutable on the constant's published property instead. #26 could be rebased off #25 and reviewed independently. 194 tests pass.
The recompute flag encoded a choice — does this stiffness follow its moduli — as a boolean, and a boolean can disagree with how the graph was actually built. It did: the flag was a live reference into the parameter store while the callback was bound once at construction, so flipping it made recomputes() claim tracking that did not exist. That was a symptom. The cause is that a parameter has no edge to the material reading it, so nothing orders a change to it against the values derived from it. K and G are now Global inputs. Which material you wire IS the choice, and it cannot desynchronise from anything: constant_scalar fixed moduli, a plain (non-history) property any "value" producer moduli that vary, e.g. with temperature constant_scalar publishes a PLAIN property deliberately. statev_map enumerates history properties, so a history-valued constant would consume one STATEV slot per integration point, per constant, and would have to be named in the exclusion list forever. Measured: with a plain property, nstatv is 0 with the moduli not mentioned at all. An earlier prototype used external_scalar_source and did cost those slots — that material is history-based because it exists for time, where the old/new pair is the point. The callback is now always bound, because inputs are not wired until finalize() and nothing can be computed in a constructor that reads them. It self-guards on the moduli, so fixed constants cost two comparisons rather than a rank-4 rebuild. Measured per ctx.update(): 28.1 ns guarded against 308.5 ns unguarded, and 31.7 ns for the old parameter-with-no-callback path — so the wired version is not slower than what it replaces, and 11x faster than rebuilding blindly. FixedModuliAreRebuiltExactlyOnce pins the guard via a recomputations() counter. One behaviour genuinely changes: the tangent is built on the first update rather than at construction, since the inputs do not exist before finalize(). Every consumer goes through ctx.update() first, but TangentIsBuiltOnTheFirstUpdateNotAtConstruction records it. EveryProducerIsOrderedBeforeItsConsumer now checks the whole chain — K and G before the stiffness, the stiffness before the stress — rather than one pair. Note this branch no longer uses set_parameter at all: the test writes through ctx.get_mutable on the constant's published property instead. #26 could be rebased off #25 and reviewed independently. 194 tests pass.
|
Closing: the motivating use case is solved better elsewhere, and nothing consumes this.
That approach is better on every axis this PR was trying to cover:
What is left without a caller is a general parameter-mutation API whose documented Every use of What would reopen this: a genuinely fixed-but-late-bound parameter — a value known Work is preserved on |
Neither had a caller outside the tests that existed to exercise them, which is the same reason #25 was closed: do not ship public API nothing uses. recomputations() was a counter for testing the memo guard. invalidate() was added to let a caller recover after writing the "tangent" property directly through material_context::get_mutable(), which would otherwise leave the guard satisfied and the stiffness never rebuilt — but nothing in the repo does that, and get_mutable already documents that the caller is responsible for restoring consistency. The three tests that read the counter now check observable behaviour instead: the tangent is zero before the first update and correct after, repeated updates with fixed moduli stay stable, and a changed modulus still propagates. Two consequences worth stating rather than discovering later. The self-guard is now untested. It is a pure optimisation — 28.1 ns per update against 308.5 ns without it — with no behavioural signature, so removing it would be a silent 11x regression on this path, not a test failure. The get_mutable wedge is reopened. Writing stiffness::tangent directly and not writing it back leaves m_valid true with matching cached moduli, so the guard returns early forever. Recovering means rebuilding the context. 194 tests pass.
Closes #23.
Materials bind their parameters once, in the constructor, and hold a
const value_type&. There was no supported way to change that value afterwards,so a host whose material constants vary between calls could not be represented.
Assigning through the non-const
get<T>()is the point, andinsert()isdeliberately not used: it goes through
insert_or_assign, which replaces the wholestd::any. For a value larger thanstd::any's small buffer that relocates thecontained object and dangles every reference bound at construction.
Why the tests look the way they do
The distinction is invisible with scalar parameters — a
doublefits the smallbuffer, so
insert()preserves its address by accident. An implementation built oninsert()would pass any test written against moduli and break on the firsttensor-valued parameter.
BoundReferenceSeesTheNewValueWriteDoesNotRelocateEvenForALargeTypeInsertRelocatesALargeValueButAssignmentDoesNotparameter_handlerdirectlyThrowsForAnUnknownKeyLeavesOtherParametersAloneThe third is the one worth reviewing: it asserts that
insert()does relocate, soif that ever stops being true the rationale gets revisited rather than silently
becoming folklore.
Scope
Does nothing about quantities derived from parameters — a material that
precomputes in its constructor will not notice a write. That is handled where the
derived value lives; see #24.
Base
Targets
feature/vector-solverrather thanmain, which is 32 commits behind it.184/184 tests pass.