Skip to content

core: write a material parameter in place, without invalidating bound references - #25

Closed
petlenz wants to merge 3 commits into
feature/vector-solverfrom
feature/material-set-parameter
Closed

core: write a material parameter in place, without invalidating bound references#25
petlenz wants to merge 3 commits into
feature/vector-solverfrom
feature/material-set-parameter

Conversation

@petlenz

@petlenz petlenz commented Aug 12, 2026

Copy link
Copy Markdown
Member

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.

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 point, and insert() is
deliberately not used: it 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 and dangles every reference bound at construction.

Why the tests look the way they do

The distinction is invisible with scalar parameters — a double fits the small
buffer, so insert() preserves its address by accident. An implementation built on
insert() would pass any test written against moduli and break on the first
tensor-valued parameter.

test what it pins
BoundReferenceSeesTheNewValue the reference a material holds observes the write
WriteDoesNotRelocateEvenForALargeType address stable for a 256-byte parameter
InsertRelocatesALargeValueButAssignmentDoesNot the reason for the implementation, asserted against parameter_handler directly
ThrowsForAnUnknownKey unknown key is an error, not a silent insert
LeavesOtherParametersAlone no collateral damage

The third is the one worth reviewing: it asserts that insert() does relocate, so
if 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-solver rather than main, which is 32 commits behind it.
184/184 tests pass.

… 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.
petlenz added a commit that referenced this pull request Aug 14, 2026
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.
petlenz added a commit that referenced this pull request Aug 14, 2026
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.
@petlenz

petlenz commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Closing: the motivating use case is solved better elsewhere, and nothing consumes this.

set_parameter existed so material constants could change between calls — the
CalculiX case, where *USER MATERIAL constants are interpolated by temperature.
That is now handled by making the constants graph inputs rather than parameters
(#26): a constant_scalar material publishes the value, the consumer takes it as a
Global input, and the topological sort orders the two correctly. Writing goes through
get_mutable on the published property, which the framework already supported.

That approach is better on every axis this PR was trying to cover:

  • Ordering is guaranteed by construction. A parameter has no edge to the material
    reading it, so nothing orders a change against the values derived from it. An input
    does.
  • Derived state follows. The consumer's callback is on the graph, so it re-runs.
    No invalidation hook, no per-material flag.
  • No STATEV costconstant_scalar publishes a plain (non-history) property, so
    statev_map never sees it. Measured: nstatv = 0 with the moduli not excluded at all.
  • Not slower. 28.1 ns per ctx.update() with a self-guarded callback, against
    31.7 ns for the parameter path this PR served.

What is left without a caller is a general parameter-mutation API whose documented
semantics are that the write lands but has no effect on anything derived at
construction, copied into a member, or used once for wiring — and is local to a single
material, since each holds its own copy of the handler. Four ways for a
successful-looking call to do nothing. Six of the ten review findings on this PR remain
as documented limitations rather than fixes, because they are not fixable at that level.

Every use of set_parameter in this branch is inside its own test file. #26 no longer
uses it and has been rebased off this branch.

What would reopen this: a genuinely fixed-but-late-bound parameter — a value known
only after finalize() that no material derives from. PROPS is not it, since the
builder writes those before construction.

Work is preserved on feature/material-set-parameter if it is ever wanted.

@petlenz petlenz closed this Aug 14, 2026
petlenz added a commit that referenced this pull request Aug 14, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant