materials: let weighted_sum and isotropic_damage source a tangent separately

[petlenz]

Stacked on #28.

Both materials assumed the material producing a constituent's stress also produces its tangent. That holds for linear_elasticity and stops holding the moment the stiffness is its own material (#28), so both gain an optional override.

weighted_sum: tangent_sources

One entry per term. Positional matching is a trap on its own — a shorter list applies each override to the wrong constituent: both names resolve, wire_inputs() succeeds, and the only symptom is a wrong summed tangent, i.e. degraded Newton convergence while the stresses still converge correctly. Nothing in a log would show it.

So the length must match the term count exactly, and "" keeps a term's own tangent — which is also what lets a non-leading term be overridden alone.

isotropic_damage: tangent_source

Declared optional and checked with contains() at the use site rather than defaulted to "", so unset and deliberately-empty stay distinguishable.

Tests

weighted_sum had no test file at all. It has five now: the mixture rule itself, absent tangent_sources, an override, the empty-entry escape, and the length check in both directions.

Split out of #26.

[petlenz]

Critical review — probed rather than read. No correctness defects found. One dead-code trap and one limitation worth documenting.

What I checked and could not break:

• The "" is an error, not a fallback claim holds. An empty tangent_source resolves as a material name of "", which fails at wire time like any unknown name. So unset and deliberately-empty really do stay distinguishable, as the comment says.
• The length check covers both directions, and the "" escape does let a non-leading term be overridden alone — that combination is what makes positional matching safe here.
• isotropic_damage's contains() at the use site is right: a set_default of "" would have collapsed the two states.

Two notes inline.

---

Dead code: weighted_sum::update() (line 95, outside the diff)

This override is never called. property_engine::update() dispatches per property — prop->traits().update() — and add_output("stress", &weighted_sum::update_stress) installs those callbacks directly. Nothing invokes the material's virtual update(); only backward_euler and vector_newton route it deliberately, by assigning traits().update = [this]{ this->update(); }.

So today it is harmless duplication. The trap is directional: someone adding a third output and wiring it only into update() — the function that looks like the entry point — gets a property that silently never recomputes. That is the same failure mode as the intra-material ordering hazard this stack exists to fix, arrived at from the other side.

Pre-existing, not introduced here, but this PR is the one touching the file. I would delete it, or keep it and say in a comment that it exists for a solver-driven context and is not the engine's entry point.

[petlenz]

Limitation worth stating in the doc comment rather than a defect.

The length check closes the case where a short list shifts every override onto the wrong constituent. It cannot close the case where a correct-length list names the wrong material — say tangent_sources = {"", "matA_stiff"} when term 1 is matB. Both names resolve, wire_inputs() succeeds, and the symptom is identical to the bug this check was added for: correct stresses, a wrong summed tangent, degraded Newton convergence and nothing in any log.

That is inherent to decoupling stress from tangent — the whole point of the parameter — so no validation can catch it. But the class comment currently reads as though supplying one entry per term is sufficient for correctness, and it is only sufficient for alignment. One sentence saying the entry must name the material that produced that term's stiffness would earn its place.

[petlenz]

Both done, no behaviour change.

The dead update() is deleted, with a comment in its place saying why there isn't one — that the engine drives each output through its add_output callback, and that backward_euler/vector_newton only use update() because they route it themselves. Better than silence: the absence is now deliberate rather than looking like an oversight.

The class comment now states the limitation outright — one entry per term buys alignment, not correctness, and no validation can close the wrong-name case because decoupling stress from tangent is the point of the parameter.

[petlenz]

Second review: no findings. Re-probed the earlier ones and both are closed — the split messages now carry both counts and the offending slot, and the weighted_sum/isotropic_damage notes are documentation-only so there was nothing behavioural to re-verify beyond the suite (194/194).

Findings this round are on #30 (one low) and #31 (one medium).

[petlenz]

Third review pass, across all open PRs. No new findings here — everything from the previous two rounds is closed and stays closed.

Re-verified one path I had stopped exercising: with Eigen and nlohmann found installed rather than fetched, configure/build/test are clean end to end. Recent rounds only tested the fetch path, which is what CI does but not what a developer machine does. Both now pass.

Findings this round are on #10 (high: the Drucker-Prager apex return is reached by no test) and #17 (medium: zero_blocks is validated by name, never by value). There is also a pre-existing packaging defect affecting every PR — the installed package cannot be consumed, because the exported target set references numsim-core::numsim-core, which is fetched and never exported. Reproduced on main, so it predates this stack; detail on #10.

[petlenz]

Critical review. No defects — but the feature has no consumer, which is worth deciding deliberately.

tangent_sources (weighted_sum) and tangent_source (isotropic_damage) are declared, tested, and used by nothing else in the repository. Grepped across the tip of the stack: outside weighted_sum.h itself, no occurrence.

The rationale was real — once #28 decomposes the elastic stiffness, a constituent's stress and tangent have different owners — but no model in the repo is built that way yet. That makes this speculative generality: correct, tested, and answering a question nobody has asked.

The counter-argument, which I think wins: this PR is also the first test file weighted_sum has ever had. Five tests covering the mixture rule itself, not just the new parameter. Closing the PR discards those too.

So the options are:

1. Merge as-is — accept a small unused parameter to keep the tests.
2. Split — keep the tests, drop tangent_sources until something needs it. Costs a rebase; leaves weighted_sum covered and the API unchanged.
3. Close — loses both.

I would take 1 or 2 over 3. Flagging it because "tested" and "needed" are different claims, and I made the first one earlier without checking the second.

Also note: this PR and #38 both add lines to tests/CMakeLists.txt and conflict there — trivial to resolve, but it will surface when both land.