Repository navigation
Open
Labels
enhancementNew feature or requestNew feature or request
Description
Grouped low-severity items from the 2026-08-02 full-repo review (finding F-18); split out sub-issues if/when picked up. Pattern follows #14.
- (a) Dual SymbolDecl storage — inputs/params are copied into
m_inputs_cache/m_parameters_cache(recipe.h:339-398);ensure_dt_symbol_registered_must setis_time_stepon both copies (recipe.h:1048-1049). Store indices intom_symbolsinstead (the codebase already prefers index-over-pointer elsewhere). - (b) Role equality is name-only; attribute-mismatch guard covers only the built-in catalogue (
recipe.h:73-76,validate_role_attributes1001-1024). Two custom roles sharing a name with different attributes silently coexist. Track first-seen custom role names per model and reject re-declaration with different attributes. - (c) roles:: globals are dynamically initialized — SIOF trap, self-documented at
recipe.h:84-89; the examples' registry pattern is onestatic ConstitutiveModelaway. Meyers-singleton accessors would close it (API break — batch with another break). - (d) Eager-branch piecewise evaluation — untaken
if_then_elsebranch still evaluates (NaN hazard for guarded inv/log/sqrt); documented attensor_code_emit.h:160-167. Fix requires statement-level branch lowering; keep on the roadmap (interacts with Local-Newton robustness: detect non-convergence + guard singular Jacobian (scalar + coupled) #85's NaN policy). - (e) HeaderPurity is a substring grep (
cmake/check_header_purity.cmake:38-45) — matches comments (false-positive class) and does not test self-containment (false-negative class). Add a per-header lone-TU compile target; all public headers currently pass (verified g++-14/C++23), so it starts green. - (g) Default LA policy defaulted in 3+ places —
recipe.h:677,recipe_render.h:115, CodeEmitPass fallbackpass_bodies.h:592-594, per-target ctors. MakePassContext::linear_algebraunconditionally set in one place; drop the fallback. - (h) Document copy semantics of ConstitutiveModel — copies share cas leaf nodes (safe today since leaf mutation happens only at declaration time — but a latent trap for future post-copy mutation), and state-variable handles do not rebind to copies (
model_tokencheck). - (i) API polish — dead
typeparam onCodeGenContext::emit_temporary(codegen_context.h:68); duplicateKindenums (recipe.h:130vs271);auto const&-returning public accessors hide their types (recipe.h:895-901); target-factory triple sync (names vector / if-chain / default constant) → one static table.
(f) install-rule gating + LICENSE install was filed with the packaging issue #133.
Metadata
Metadata
Assignees
Labels
enhancementNew feature or requestNew feature or request
Activity
petlenz commentedon Aug 2, 2026
Three additions from the PR-batch critical review (2026-08-02):
timeout-minuteson the workflow jobs (360-min default; the ASan leg is the one to watch), and GitHub auto-disablesschedule:workflows after 60 days of repo inactivity (email warning first) — worth knowing rather than fixing, but atimeout-minutes: 45per job is one line each.ScalarCodeEmit'sstd::complex<double>branch is unreachable at the current cas pin —make_expression<scalar_constant>(std::complex<...>)throws "complex values are not supported" at construction (discovered while trying to test it; documented in ScalarCodeEmitTest.cpp). Either keep as defensive dead code (current state, format+non-finite-guard hardened) or remove when convenient.numsim_material.cpp:617constructs a rawscalar_constant(-1.0)on the tangent path; today it only reaches the emitter as a mul leaf, so the Negation emitters can emit '--5.0' (invalid C++) for a bare negative constant — add paren guard #136 paren guard never fires there — incidental to the shipped recipes' shape, not structural. A NumSimMaterialTarget test with a residual whose dR/dz wraps that node inscalar_negativewould pin the interaction end-to-end (the visitor-level guards are unit-tested as of PR Parenthesise negation of a leading-minus operand (#136) #147's follow-up).