materials: collapse small_strain_plasticity into a dedicated Drucker-Prager (unblocks #33) - #43
Merged
Merged
Conversation
…Prager
After J2 moved out, small_strain_plasticity<Traits, YieldFunction> had exactly
one instantiation. A template parameter with one argument is not generality, it
is indirection, and it cost:
- a has_apex_return concept plus three if constexpr / requires sites, guarding
a branch the only remaining user always has;
- a yield function passed as a C++ OBJECT through a "yield_function"
parameter.
That second one was not just noise. The JSON reader has no converter for the
object, so Drucker-Prager could not be configured from a document at all -- the
blocker behind #33, where it had to stay unregistered because a document naming
it would silently get a default-constructed cone (eta = beta = k = 0), which
builds, runs, never yields, and looks like elasticity.
eta, beta and K_bulk are now ordinary scalar parameters. Verified: a
Drucker-Prager model built entirely from a JSON document reproduces the C++
reference bit-for-bit, alpha = 0.042067012632805115 either way.
The apex is now unconditional -- the concept and every if constexpr are gone --
because a cone always has one.
Drucker-Prager: 1178.9 -> 721.9 ns/step over the two commits (1.63x)
Bit-identical to the pre-refactor implementation on a 20-step path, all 17
digits of alpha, stress and tangent.
The yield function survives as an internal member rather than a template
parameter: it holds the verified apex algebra, and rewriting that to save a
file would have traded a real risk for a cosmetic gain.
This was referenced Sep 5, 2026
Member
Author
|
Critical review. No findings, and the #33 unblock is real — I confirmed a Drucker-Prager model built entirely from JSON reproduces the C++ reference exactly ( One judgement worth stating so it is not rediscovered: the yield function stays as an internal member rather than being inlined. That keeps the verified apex algebra untouched, and the apex is the branch that had zero test coverage until this stack added it. Inlining it to save a file would have put the least-verified code through the largest rewrite. Net-negative diff (+55/-74), mostly a rename, 50/50. Follow-up for whoever lands this: |
This was referenced Sep 6, 2026
The 1 header(s) this branch introduces follow the convention set on feature/drucker-prager: the guard is the file's own name, no NUMSIM_MATERIALS_ prefix. Checked against every dependency header and /usr/include for a prior #define of each new name -- none.
# Conflicts: # include/numsim-materials/materials/drucker_prager_plasticity.h
petlenz
added a commit
that referenced
this pull request
Sep 6, 2026
main did not build. tests/test_tangent_generator.cpp, added by #28, includes materials/small_strain_plasticity.h, which #43 renamed away. The test it belonged to asserted that isotropic_tangent is a drop-in for linear_elasticity as a plasticity "elastic_source". Plasticity no longer has an elastic_source: #44 removed it because the closed-form stress and tangent require an isotropic C_e, so accepting an arbitrary rank-4 tangent advertised a generality the material cannot honour. The premise is gone. The drop-in claim is still covered against consumers that do source a tangent: TangentSource.* in the same file, and weighted_sum's tangent_sources tests in #29. The other nine tests in this file exercise isotropic_tangent directly and are untouched. How this reached main: the PR was retargeted from its stacked base to main, and retargeting did not trigger a fresh CI run. The green check I read predated the retarget, so it had been tested against the old base where the header still existed. Git reported no conflict either -- nothing edited the same lines. From here every merge is verified by building the merge result locally, not by trusting a check-run whose base may be stale. 217/217 tests pass.
petlenz
added a commit
that referenced
this pull request
Sep 6, 2026
Drucker-Prager was held out of the factory with a stated condition: register it once the yield function is expressible from a document. #43 met that condition by making eta, beta and K_bulk plain required scalars instead of members of a C++ yield_function object the JSON reader could not convert. Registered now, and the test that pinned its absence is replaced by two that pin the reason the absence was needed: - a complete Drucker-Prager document builds and yields - a document missing "eta" throws and leaves no material behind, rather than silently getting eta = beta = k = 0 and running as elasticity local_newton is registered too. Without it #33 would still not be closed: every return map names its solver through "solver_source", so a deck could name j2_plasticity but not the solver it requires, and the model still could not be built from a document. The J2 document in this file also needed updating -- it named backward_euler as the plasticity solver, which is the callback mode #40 removed, and passed elastic_source, which #44 removed when plasticity took ownership of its own elastic tangent. One honest limitation recorded in the test rather than papered over: the exception for a missing parameter carries only a COUNT, "missing 1 required parameter(s)". numsim-core's input_parameter_controller prints the names to stdout and throws the count separately, so a deck typo is loud but not self-explanatory. Fixing that is a numsim-core change; what this test pins is that the cone cannot be built without its parameters. 258/258 tests pass on the merge result, verified locally.
petlenz
added a commit
that referenced
this pull request
Sep 6, 2026
TheOptOutListHasNoStaleEntries did exactly what it exists for: kNotForJson exempted small_strain_plasticity and rk_plasticity, and neither header exists any more -- #43 and #40 split them into j2_plasticity, drucker_prager_plasticity and j2_rk_plasticity, each its own header and, since #36, its own factory entry. The exemption for tensor_component_stepper stays: it is a template over Rank, registered as tensor_component_stepper_rank1 and _rank2. 263/263 tests pass on the merge result, verified locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
3 of 5. Stacked on the J2 split.
With J2 moved out,
small_strain_plasticity<Traits, YieldFunction>had exactly one instantiation. A template parameter with one argument is not generality, it is indirection, and it cost:has_apex_returnconcept plus threeif constexpr/requiressites, guarding a branch the only remaining user always hasyield_functionparameterThe second one is why #33 is stuck
The JSON reader has no converter for that object, so Drucker-Prager could not be configured from a document at all. That is why it had to stay unregistered in #36: a document naming it would silently get a default-constructed cone (
eta = beta = k = 0) that builds, runs, never yields, and is indistinguishable from elasticity.eta,betaandK_bulkare ordinary scalar parameters now. A Drucker-Prager model built entirely from JSON reproduces the C++ reference exactly:So
drucker_prager_plasticitycan be registered in #36 rather than deliberately excluded — that follow-up belongs on #36, once this lands.The apex is unconditional now; the concept and every
if constexprare gone, because a cone always has one.Not done
The yield function survives as an internal member rather than a template parameter. It holds the verified apex algebra, and rewriting that to save a file would trade real risk for a cosmetic gain.
Bit-identical, all 17 digits, on a 20-step path. 50/50 tests. Mostly a rename: +55/-74 across 5 files.