solvers: select the integration scheme by name (explicit vs implicit from the deck) - #45
Merged
Conversation
… schema
"tableau" was never declared in parameters() at all. The constructor read
get_parameter<const butcher_tableau*>("tableau") while the schema never
inserted it, so it could only be supplied from C++, where parameter_handler's
insert() bypasses the schema. From a document it failed with "Key tableau not
found".
That mattered more than a missing parameter usually would: the tableau IS the
explicit-versus-implicit choice. j2_rk_plasticity already handles explicit
stages (m_is_implicit[i] = |a_ii| >= 1e-30), so forward_euler was always an
explicit J2 integrator -- the capability existed and was unreachable from a
deck.
The parameter is now a scheme NAME resolved by tableau_by_name(), and it is
declared. A raw pointer cannot be expressed in a document, which is the same
shape as Drucker-Prager's yield_function object (#33) and vector_newton's
zero_blocks (#17): a parameter the code reads but the schema does not declare
is invisible to both the document layer and to validation.
rk_integrator had the identical defect and is fixed the same way; leaving one
of the two would have been worse than not starting.
An unknown name now throws and lists the valid schemes, rather than leaving
the parameter absent. Two tests: every published scheme resolves and reports
the right explicit/implicit character, and a typo names itself and the
alternatives.
Verified from JSON:
j2_rk_plasticity (sdirk3, implicit) OK
j2_rk_plasticity EXPLICIT (forward_euler) OK
a typo in the scheme name unknown scheme 'sdirk4' -- expected one of: ...
wire_materials() wrapped the look-up and the type-check in one catch(...), so material_ref::wire()'s precise "is not of the requested type" was discarded and the reference reported as missing instead. The effect on a user: naming a backward_euler where a local_newton is required produced wire_materials(): material 'j2' references missing materials: 'solver' sending them to look for a material sitting right there in their document. That is what makes it worth fixing rather than merely tidying -- the message does not just omit information, it points the wrong way. Now: wire_materials(): material 'j2' references materials of the wrong type: 'solver' (they exist, but are not the type this material requires) The two failures are collected separately and both are reported, so a graph with one of each still names both in one error -- the collect-then-report behaviour was deliberate and is kept. Two tests, one per failure mode, and the wrong-type one is verified against the mutation: restoring the single catch(...) fails it.
Making the scheme a named parameter turned an unreachable wrong answer into a deck-selectable one, and the commit that did it advertised the new reachability without checking the numbers. The stage loop accumulates a(i,j) only for j < i, plus the diagonal. Any coupling above the diagonal is dropped silently. gauss_legendre_4 has a(0,1) = 0.25 - 1/(2*sqrt(3)), so it is integrated as something that is neither Gauss-Legendre nor fourth order. Measured against the monolithic backward-Euler reference on a 60-step uniaxial path: implicit_euler alpha=0.034372 ratio 1.000 F +0.0000 sdirk3 alpha=0.034372 ratio 1.000 F -0.0000 forward_euler alpha=0.034528 ratio 1.005 F -0.1923 rk4 alpha=0.033750 ratio 0.982 F +0.7663 gauss_legendre_4 alpha=-8.805943 ratio -256 F +10880.3 A NEGATIVE equivalent plastic strain, no error, no warning. The explicit schemes' small offsets are expected -- they do not enforce the yield condition exactly -- but the last row is not an accuracy question. rk_integrator dispatches this case to a fully-implicit solve. This class has no such path, so it now refuses rather than pretending, naming the schemes it can integrate. Adding that path would be the better fix and is a larger change than this PR should carry. Two tests: gauss_legendre_4 is rejected, and every DIRK or explicit scheme is still accepted, so the guard cannot pass by refusing everything. Found by an independent review of this PR. The same review claimed a 3x error from j2_rk_plasticity.h:135 passing G where effective_modulus(G) = 3G is expected. The argument IS inconsistent with the other two call sites, but the measured effect is not 3x -- hardening dominates the denominator (3G = 231 against H' = 1000) and the incremental process self-corrects. Left alone here and reported separately rather than fixed on a claim I could not reproduce.
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
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.
Stacked on #40, which is where
j2_rk_plasticityexists.The question that found this
"What if I want a different solver — maybe explicit? I could not simply define that in the JSON file."
Checking that turned out to be worth more than the answer. Three documents:
"tableau"was never declared in the schemaThe constructor read
get_parameter<const butcher_tableau*>("tableau").parameters()never inserted it. So it worked only from C++, whereparameter_handler::insert()bypasses the schema — and from a document it failed identically whether the name was right, wrong, or absent.That matters more than a missing parameter usually would, because the tableau is the explicit-versus-implicit choice.
j2_rk_plasticityalready handles explicit stages:m_is_implicit[i] = std::abs(m_diag[i]) >= 1e-30;So
forward_eulerwas always an explicit J2 integrator. The capability existed and was unreachable from a deck.Third instance of one pattern
zero_blocksinvector_newton— read viacontains(), never declared (Add vector_newton: coupled Newton for mixed scalar/tensor local systems #17)yield_functionin Drucker-Prager — a C++ object, never declared (8 materials are not registered in the factory, so a JSON model cannot express plasticity #33, fixed in materials: collapse small_strain_plasticity into a dedicated Drucker-Prager (unblocks #33) #43)tableauhereA parameter the code reads but the schema does not declare is invisible to both the document layer and to validation. I had been treating these as three unrelated bugs.
The change
tableauis now a scheme name, resolved bytableau_by_name(), and it is declared.rk_integratorhad the identical defect and is fixed the same way — leaving one of the two would have been worse than not starting.An unknown name throws and lists the valid schemes rather than leaving the parameter absent:
Result
Two tests: every published scheme resolves and reports the right explicit/implicit character (
forward_eulerandrk4explicit,sdirk3andgauss_legendre_4not), and a typo names itself and the alternatives.55/55.
What this does not fix
The solver type is still compile-time fixed in
j2_plasticityanddrucker_prager_plasticity—using solver_type = local_newton<Traits>, wired bydynamic_cast. A document names which instance, never which kind. Naming abackward_eulerthere still reports the material as missing rather than as the wrong type, becausewire_materials()collapses every exception into "absent".That is a separate change and arguably not needed: choosing the integrator by material type —
j2_plasticityfor implicit backward Euler,j2_rk_plasticitywith a named scheme for everything else — is the same idiom as #40'sbackward_eulervslocal_newton, and it avoids virtual dispatch on a per-integration-point path. Detail on #40.Also here: the wiring error pointed the wrong way
wire_materials()wrapped the look-up and the type-check in onecatch (...), discardingmaterial_ref::wire()'s precise message. Naming abackward_eulerwhere alocal_newtonis required produced:— sending the user to look for a material sitting right there in their document. The message did not just omit information, it pointed the wrong way. Now:
Both failure kinds are still collected and reported together, so a graph with one of each names both in one error. Two tests, one per mode; the wrong-type one is verified against the mutation that restores the single
catch (...).57/57.