From ab502925419c769f36ea5262e995538beb5aa0a2 Mon Sep 17 00:00:00 2001 From: petlenz Date: Sun, 2 Aug 2026 23:19:25 +0200 Subject: [PATCH] Deduplicate target emission helpers (#139) --- .../code_emit/linear_algebra_emitter.h | 13 + .../code_emit/spectral_decompose_emit.h | 10 + src/targets/moose_material.cpp | 20 +- src/targets/numsim_material.cpp | 286 +++++++++--------- src/targets/standalone_cxx.cpp | 19 +- 5 files changed, 182 insertions(+), 166 deletions(-) diff --git a/include/numsim_codegen/code_emit/linear_algebra_emitter.h b/include/numsim_codegen/code_emit/linear_algebra_emitter.h index c70296c..9e452e9 100644 --- a/include/numsim_codegen/code_emit/linear_algebra_emitter.h +++ b/include/numsim_codegen/code_emit/linear_algebra_emitter.h @@ -190,6 +190,19 @@ class ArmadilloLinearAlgebraEmitter final : public LinearAlgebraEmitter { return eigen_linear_algebra_emitter(); } +// Include-gating predicate (#139): does the EMITTED body actually use `la`'s +// backend? Keyed on the emitter's usage marker so the include decision tracks +// the emitted code, not a re-derived coupling predicate (PR #83 round-2 #4) — +// gating on a predicate could diverge from what was emitted (e.g. a future +// pass-synthesized coupling) → a missing header for code that uses it. Shared +// by every target that gates a linalg include on `body`; pass the SAME `la` +// that drove emission so they cannot disagree. +[[nodiscard]] inline auto uses_linear_algebra(std::string const &body, + LinearAlgebraEmitter const &la) + -> bool { + return body.find(la.usage_marker()) != std::string::npos; +} + } // namespace numsim::codegen #endif // NUMSIM_CODEGEN_LINEAR_ALGEBRA_EMITTER_H diff --git a/include/numsim_codegen/code_emit/spectral_decompose_emit.h b/include/numsim_codegen/code_emit/spectral_decompose_emit.h index 84b821e..3875b2a 100644 --- a/include/numsim_codegen/code_emit/spectral_decompose_emit.h +++ b/include/numsim_codegen/code_emit/spectral_decompose_emit.h @@ -20,6 +20,16 @@ namespace numsim::codegen { inline constexpr std::string_view spectral_runtime_qualifier = "numsim::codegen::rt::"; +// Include-gating predicate (#139): does the EMITTED body call into the +// spectral runtime? Keyed on `spectral_runtime_qualifier` (shared with the +// emitters, see above) so the `#include ` +// decision tracks actual emitted usage — shared by every target that gates +// the spectral-runtime include. +[[nodiscard]] inline auto uses_spectral_runtime(std::string const &body) + -> bool { + return body.find(spectral_runtime_qualifier) != std::string::npos; +} + // Emit (once per distinct tensor argument) the shared spectral decomposition // that the eigenvalue / eigenprojection / eigenvector emitters read from, and // return the temporary's name. `arg` is the already-emitted C++ name of the diff --git a/src/targets/moose_material.cpp b/src/targets/moose_material.cpp index f0d8b77..6398801 100644 --- a/src/targets/moose_material.cpp +++ b/src/targets/moose_material.cpp @@ -308,20 +308,14 @@ void emit_init_stateful_body(std::ostream &os, ConstitutiveModel const &model) { auto emit_source(ConstitutiveModel const &model, std::string const &app_name, LinearAlgebraEmitter const &la) -> std::string { - // Emit the Layer-2 function first so the linalg-include decision tracks the - // ACTUAL emitted code (PR #83 round-2 #4): gating on a re-derived coupling - // predicate could diverge from what was emitted (e.g. a future pass- - // synthesized coupling) → a missing header for code that uses it. Keying on - // the emitter's usage marker cannot drift. The SAME `la` drives emission and - // the include (per-target, injected) so they cannot disagree. + // Emit the Layer-2 function first so both include decisions track the + // ACTUAL emitted code via the shared predicates (uses_linear_algebra / + // uses_spectral_runtime, #139) — keying on the emitters' markers cannot + // drift from what was emitted. The SAME `la` drives emission and the + // include (per-target, injected) so they cannot disagree. std::string const body = model.emit_compute_function(la); - bool const needs_la = body.find(la.usage_marker()) != std::string::npos; - // Spectral materials pull in the shipped tmech-only runtime helper. Keyed on - // actual emitted usage via the shared `spectral_runtime_qualifier` constant - // so it can't drift from what the emitters produced — same principle as the - // linalg marker. - bool const needs_spectral = - body.find(spectral_runtime_qualifier) != std::string::npos; + bool const needs_la = uses_linear_algebra(body, la); + bool const needs_spectral = uses_spectral_runtime(body); std::ostringstream os; os << "// Auto-generated by numsim-codegen. Do not edit.\n\n"; diff --git a/src/targets/numsim_material.cpp b/src/targets/numsim_material.cpp index f80e4cf..5d3290d 100644 --- a/src/targets/numsim_material.cpp +++ b/src/targets/numsim_material.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -82,6 +83,109 @@ std::string tensor_cxx_type(std::size_t dim, std::size_t rank) { std::to_string(rank) + ">"; } +// ── Helpers shared by the rate and residual emission paths (#139) ────────── +// Both paths consume the same collected shapes and emit the same boilerplate; +// keeping them in ONE place means a hardening fix cannot land in one path and +// silently miss the sibling. + +// Declared symbols split into (decls, name-set) — the shape both paths bind. +struct CollectedSymbols { + std::vector decls; + std::set names; +}; + +// Scalar parameters the emitted expressions may reference, skipping the +// framework time step: the integrator/solver owns discretization, so neither a +// rate material nor a (rate-independent) residual material ever sees `dt`. +CollectedSymbols collect_scalar_params(ConstitutiveModel const &model) { + CollectedSymbols out; + for (auto const &p : model.parameters()) { + if (p.is_time_step || p.kind != SymbolDecl::Kind::Scalar) continue; + out.decls.push_back(p); + out.names.insert(p.name); + } + return out; +} + +// Tensor inputs (e.g. strain): each is wired from a producer material via a +// Global-edge input_property, read by the property name `` (the +// producer must publish a property of that name), with a `_source` +// string parameter naming the producer. The scope guards already rejected +// scalar inputs on both paths. +CollectedSymbols collect_tensor_inputs(ConstitutiveModel const &model) { + CollectedSymbols out; + for (auto const &in : model.inputs()) { + out.decls.push_back(in); + out.names.insert(in.name); + } + return out; +} + +// A non-finite default would emit `value_type{nan}`/`{inf}` (no such C++ +// literal) and an invalid JSON number — reject rather than emit broken code. +void reject_non_finite_default(SymbolDecl const &p) { + if (p.default_value.has_value() && !std::isfinite(*p.default_value)) { + throw std::runtime_error( + "NumSimMaterialTarget: parameter '" + p.name + + "' has a non-finite default (" + fmt(*p.default_value) + + "); cannot emit as a C++ literal or JSON number."); + } +} + +// parameters() schema: every scalar parameter (default → set_default, else +// is_required), the path's fixed source parameter (`solver_source` / +// `integrator_source`), and one required `_source` per tensor input. +void emit_parameters_schema(std::ostream &h, + std::vector const ¶ms, + std::vector const &tensor_inputs, + char const *source_param) { + h << " static input_parameter_controller parameters() {\n"; + h << " input_parameter_controller para{base::parameters()};\n"; + for (auto const &p : params) { + if (p.default_value.has_value()) { + h << " para.template insert(\"" << p.name + << "\").template add(value_type{" + << fmt(*p.default_value) << "});\n"; + } else { + // Defensive: `add_parameter` always sets a default, so this is currently + // unreachable via the public API. Kept (with the JSON-omission sibling + // in the config emission) for the day a no-default/required-parameter + // API lands. + h << " para.template insert(\"" << p.name + << "\").template add();\n"; + } + } + h << " para.template insert(\"" << source_param << "\")\n"; + h << " .template add();\n"; + for (auto const &ti : tensor_inputs) { + h << " para.template insert(\"" << ti.name << "_source\")\n"; + h << " .template add();\n"; + } + h << " return para;\n"; + h << " }\n\n"; +} + +// Emitted-member uniqueness guard: every generated `m_` member basename +// must be distinct, else the class gets duplicate members / initializers +// (uncompilable). The recipe enforces symbol-name uniqueness, but the emitter +// SYNTHESIZES extra names (fixed members, `out_`, `_source`) +// that a recipe symbol can still collide with. Both paths claim their names +// through this one guard so the hazard's message cannot drift. +class MemberUniquenessGuard { +public: + void claim(std::string const &base) { + if (!m_bases.insert(base).second) { + throw std::runtime_error( + "NumSimMaterialTarget: emitted member 'm_" + base + + "' would be duplicated — a recipe symbol collides with a synthesized " + "member name; rename the offending state/parameter/input/output."); + } + } + +private: + std::set m_bases; +}; + // First-increment scope: exactly one scalar state variable + one scalar // evolution equation `dx/dt = f(x, params)`, and NOTHING this increment can't // emit. Rejecting (rather than silently emitting a partial material) is the @@ -250,22 +354,10 @@ std::vector emit_residual_material(ConstitutiveModel const &model) auto const &sv = svs[req.state_variable_idx]; auto const &cur_name = model.symbols()[sv.current_symbol_idx].name; - // Scalar parameters the residual / outputs may reference (skip the framework - // time step — a residual material is rate-independent in this increment). - std::vector params; - std::set param_names; - for (auto const &p : model.parameters()) { - if (p.is_time_step || p.kind != SymbolDecl::Kind::Scalar) continue; - params.push_back(p); - param_names.insert(p.name); - } - - std::vector tensor_inputs; - std::set tensor_input_names; - for (auto const &in : model.inputs()) { - tensor_inputs.push_back(in); - tensor_input_names.insert(in.name); - } + // Scalar parameters the residual / outputs may reference + the wired tensor + // inputs (shared collectors, #139). + auto const [params, param_names] = collect_scalar_params(model); + auto const [tensor_inputs, tensor_input_names] = collect_tensor_inputs(model); // #92: internal variables (state vars set by a post-solve update equation). // Each carries its own history (scalar or tensor); their `_old` value is bound @@ -317,12 +409,7 @@ std::vector emit_residual_material(ConstitutiveModel const &model) reject_reserved(cur_name, "state variable name"); for (auto const &p : params) { reject_reserved(p.name, "parameter name"); - if (p.default_value.has_value() && !std::isfinite(*p.default_value)) { - throw std::runtime_error( - "NumSimMaterialTarget: parameter '" + p.name + - "' has a non-finite default (" + fmt(*p.default_value) + - "); cannot emit as a C++ literal or JSON number."); - } + reject_non_finite_default(p); } // The Newton increment is a compute()-local named `d`; a tensor input // OR an internal variable's `_old` local (also bare compute()-locals) named @@ -642,23 +729,15 @@ std::vector emit_residual_material(ConstitutiveModel const &model) // Emitted-member uniqueness guard (same hazard as the rate path: synthesized // member names can collide with recipe symbols). { - std::set member_bases; - auto claim = [&member_bases](std::string const &base) { - if (!member_bases.insert(base).second) { - throw std::runtime_error( - "NumSimMaterialTarget: emitted member 'm_" + base + - "' would be duplicated — a recipe symbol collides with a synthesized " - "member name; rename the offending state/parameter/input/output."); - } - }; - claim("solver"); // the fixed material_ref member m_solver - claim(cur_name); - for (auto const &iv : internals) claim(iv.name); // internal-var histories - for (auto const &p : params) claim(p.name); - for (auto const &o : outputs) claim("out_" + o.name); + MemberUniquenessGuard guard; + guard.claim("solver"); // the fixed material_ref member m_solver + guard.claim(cur_name); + for (auto const &iv : internals) guard.claim(iv.name); // internal-var histories + for (auto const &p : params) guard.claim(p.name); + for (auto const &o : outputs) guard.claim("out_" + o.name); for (auto const &ti : tensor_inputs) { - claim(ti.name); - claim(ti.name + "_source"); + guard.claim(ti.name); + guard.claim(ti.name + "_source"); } } @@ -760,28 +839,9 @@ std::vector emit_residual_material(ConstitutiveModel const &model) } } - // parameters() schema - h << " static input_parameter_controller parameters() {\n"; - h << " input_parameter_controller para{base::parameters()};\n"; - for (auto const &p : params) { - if (p.default_value.has_value()) { - h << " para.template insert(\"" << p.name - << "\").template add(value_type{" - << fmt(*p.default_value) << "});\n"; - } else { - h << " para.template insert(\"" << p.name - << "\").template add();\n"; - } - } - h << " para.template insert(\"" - << contract::solver_source_param << "\")\n"; - h << " .template add();\n"; - for (auto const &ti : tensor_inputs) { - h << " para.template insert(\"" << ti.name << "_source\")\n"; - h << " .template add();\n"; - } - h << " return para;\n"; - h << " }\n\n"; + // parameters() schema (shared writer, #139) + emit_parameters_schema(h, params, tensor_inputs, + contract::solver_source_param); // compute() h << " // Solves R(" << cur_name << ", inputs)=0 for the increment via\n"; @@ -952,27 +1012,11 @@ auto NumSimMaterialTarget::emit(ConstitutiveModel const &model) const auto const &eq = model.evolution_equations()[0]; auto const &cur_name = model.symbols()[sv.current_symbol_idx].name; - // Scalar parameters the rate may reference (skip the framework time-step: - // the integrator owns discretization, so a rate material never sees `dt`). - std::vector params; - std::set param_names; - for (auto const &p : model.parameters()) { - if (p.is_time_step || p.kind != SymbolDecl::Kind::Scalar) continue; - params.push_back(p); - param_names.insert(p.name); - } - - // Tensor inputs (e.g. strain): each is wired from a producer material via a - // Global-edge input_property, read by the property name `` (the - // producer must publish a property of that name). Each gets a `_source` - // string parameter naming the producer. The rate-scope guard already - // rejected scalars. - std::vector tensor_inputs; - std::set tensor_input_names; - for (auto const &in : model.inputs()) { - tensor_inputs.push_back(in); - tensor_input_names.insert(in.name); - } + // Scalar parameters the rate may reference + the wired tensor inputs + // (shared collectors, #139 — the rate-scope guard already rejected scalar + // inputs). + auto const [params, param_names] = collect_scalar_params(model); + auto const [tensor_inputs, tensor_input_names] = collect_tensor_inputs(model); // Reserved-name guard: a state/parameter named like an emitted fixed member // (`rate`, `rate_derivative`, `integrator_source`) would collide. @@ -988,14 +1032,7 @@ auto NumSimMaterialTarget::emit(ConstitutiveModel const &model) const "' collides with an emitted member (rate/rate_derivative/" "integrator_source); rename it."); } - // A non-finite default would emit `value_type{nan}`/`{inf}` (no such C++ - // literal) and an invalid JSON number — reject rather than emit broken code. - if (p.default_value.has_value() && !std::isfinite(*p.default_value)) { - throw std::runtime_error( - "NumSimMaterialTarget: parameter '" + p.name + - "' has a non-finite default (" + fmt(*p.default_value) + - "); cannot emit as a C++ literal or JSON number."); - } + reject_non_finite_default(p); } // Rate-leaf guard: every scalar symbol the rate references must be the state @@ -1180,35 +1217,24 @@ auto NumSimMaterialTarget::emit(ConstitutiveModel const &model) const for (auto const &o : outputs) if (o.is_tensor) has_tensor = true; - // Comprehensive emitted-member uniqueness guard. Every generated `m_` - // member basename must be distinct, else the class gets duplicate members / - // initializers (uncompilable). The recipe enforces symbol-name uniqueness, but - // the emitter SYNTHESIZES extra names — `rate`, `rate_derivative`, - // `integrator_source`, `out_`, `_source` — that a recipe symbol - // can still collide with (a tensor input literally named "rate"; an input - // "strain" alongside a parameter "strain_source"; an input named "integrator" - // whose "_source" param clashes with the fixed integrator_source). Reject - // loudly rather than emit broken C++. (The is_reserved_name checks above still - // fire first for state/parameter cases, giving a more specific message.) + // Comprehensive emitted-member uniqueness guard: `rate`, `rate_derivative`, + // `integrator_source`, `out_`, `_source` are all synthesized + // names a recipe symbol can still collide with (a tensor input literally + // named "rate"; an input "strain" alongside a parameter "strain_source"; an + // input named "integrator" whose "_source" param clashes with the fixed + // integrator_source). (The is_reserved_name checks above still fire first + // for state/parameter cases, giving a more specific message.) { - std::set member_bases; - auto claim = [&member_bases](std::string const &base) { - if (!member_bases.insert(base).second) { - throw std::runtime_error( - "NumSimMaterialTarget: emitted member 'm_" + base + - "' would be duplicated — a recipe symbol collides with a synthesized " - "member name; rename the offending state/parameter/input/output."); - } - }; - claim(contract::rate_property); - claim(contract::rate_derivative_property); - claim(contract::integrator_source_param); - claim(cur_name); - for (auto const &p : params) claim(p.name); - for (auto const &o : outputs) claim("out_" + o.name); + MemberUniquenessGuard guard; + guard.claim(contract::rate_property); + guard.claim(contract::rate_derivative_property); + guard.claim(contract::integrator_source_param); + guard.claim(cur_name); + for (auto const &p : params) guard.claim(p.name); + for (auto const &o : outputs) guard.claim("out_" + o.name); for (auto const &ti : tensor_inputs) { - claim(ti.name); - claim(ti.name + "_source"); + guard.claim(ti.name); + guard.claim(ti.name + "_source"); } } @@ -1296,31 +1322,9 @@ auto NumSimMaterialTarget::emit(ConstitutiveModel const &model) const } } - // parameters() schema - h << " static input_parameter_controller parameters() {\n"; - h << " input_parameter_controller para{base::parameters()};\n"; - for (auto const &p : params) { - if (p.default_value.has_value()) { - h << " para.template insert(\"" << p.name - << "\").template add(value_type{" - << fmt(*p.default_value) << "});\n"; - } else { - // Defensive: `add_parameter` always sets a default, so this is currently - // unreachable via the public API. Kept (with the JSON-omission sibling - // below) for the day a no-default/required-parameter API lands. - h << " para.template insert(\"" << p.name - << "\").template add();\n"; - } - } - h << " para.template insert(\"" - << contract::integrator_source_param << "\")\n"; - h << " .template add();\n"; - for (auto const &ti : tensor_inputs) { - h << " para.template insert(\"" << ti.name << "_source\")\n"; - h << " .template add();\n"; - } - h << " return para;\n"; - h << " }\n\n"; + // parameters() schema (shared writer, #139) + emit_parameters_schema(h, params, tensor_inputs, + contract::integrator_source_param); // compute() h << " // " << contract::rate_property << " = f(" << cur_name << "); " diff --git a/src/targets/standalone_cxx.cpp b/src/targets/standalone_cxx.cpp index ded02ee..1faa0f1 100644 --- a/src/targets/standalone_cxx.cpp +++ b/src/targets/standalone_cxx.cpp @@ -9,19 +9,14 @@ namespace numsim::codegen { auto StandaloneCxxTarget::emit(ConstitutiveModel const &model) const -> std::vector { - // Emit the function first so the linalg-include decision tracks ACTUAL usage - // in the emitted code, not a re-derived coupling predicate (PR #83 round-2 - // #4) — keying on the emitter's usage marker cannot drift from what was - // emitted. The library is whatever default_linear_algebra_emitter() selects. + // Emit the function first so both include decisions track ACTUAL usage in + // the emitted code via the shared predicates (uses_linear_algebra / + // uses_spectral_runtime, #139) — keying on the emitters' markers cannot + // drift from what was emitted. The library is whatever + // default_linear_algebra_emitter() selects. std::string const body = model.emit_compute_function(m_la); - bool const needs_la = body.find(m_la.usage_marker()) != std::string::npos; - // Key the spectral-runtime include on ACTUAL emitted usage — the spectral - // handlers emit calls under `spectral_runtime_qualifier` (spectral_decompose - // / divided_difference / confluent_derivative). Sharing that constant with - // the emitters means a namespace rename can't silently drop the include. Same - // no-drift principle as the linalg marker above. - bool const needs_spectral = - body.find(spectral_runtime_qualifier) != std::string::npos; + bool const needs_la = uses_linear_algebra(body, m_la); + bool const needs_spectral = uses_spectral_runtime(body); std::ostringstream os; os << "// Auto-generated by numsim-codegen. Do not edit.\n\n";