engine: LTM fidelity fixes from the paper audit - #1062
Merged
Merged
Conversation
A relative loop score is a loop's share of its cycle partition: its raw score over the sum of |score| across every loop of the partition (reference section 4.4). The exhaustive path grouped by (partition, slot) instead -- an arrayed loop's slot k competed only with other loops' slot k, and a scalar loop was broadcast into every slot bucket -- so on any model whose elements are coupled the denominators missed most of the partition's members. On test/cross_element_ltm the births loop read 1/2 at NYC and 2/3 at Boston where the partition holds four active members and the share is 1/3; on the arrays investigation's sliced_sum corpus model the dominant-period verdict was wrong for the whole run. A slot index is a position in one loop's own dimension space, so two arrayed loops over different dimension lists, or a scalar loop, attach no shared meaning to "slot k"; the partition is the only sound group. ltm_post::compute_rel_loop_scores is now the single owner: every (loop, slot) with a loop_score column is one member of its slot's partition (an unresolved slot is a Solo member of its own), the denominator is the partition-wide sum, a scalar loop has exactly one series and an arrayed loop one per slot, laid out step-major with the loop's own slot count as stride. The stride/broadcast machinery, the slot-0 convenience view, and the FFI's streaming per-element pair are deleted; libsimlin caches the owner's whole output per SimState and reads the requested slot (or the argmax-abs collapse the layout also uses) out of it, and discovery's rank_and_filter accumulates its partition totals and divides through the same group_totals / relative_series pair, so the two surfaces cannot disagree on the rule. The link-level normalization shares the same accumulator (NaN excluded, Inf kept, finite overflow saturating). Numbers pinned by tests, through the real pipeline on cross_element_ltm at every checked step: births 1/3 at both elements, the NYC migration_out loop -1/6, the cross-element migration_in loop +1/6, partition magnitudes summing to 1 -- from the engine owner, the libsimlin FFI (subscripted and bare ids), and pysimlin (Sim and Run.loops). A property test over generated per-slot partition vectors checks the owner bit-for-bit against a naive per-member reference and the partition identity at every step. The arrays investigation's de-subscripting oracle (report B), rerun on this tree with its dumper example rebuilt against the new owner: the relative-score rows that were wrong become exact on cross_element (4 -> 0 bad), sliced_sum (12 -> 0), per_element_eqns (4 -> 0), disjoint_dim (5 -> 0), mapped_dims (2 -> 0), share_sum (3 -> 0), bare_reducer, scalar_cofactor, variable_backed and dt_quarter (3 -> 0 each); raw scores and edges are unchanged on every model. The rows still wrong (cross_agg, cross_agg4, and the reducer-split models mean/min/max/stddev_loop, scalar_cofactor_inline, two_d, migration_matrix) are the stitching and split-partial defects, not the normalization. The oracle's summary lines: cross-agg-ltm: values 6 checked/0 bad | edges ok=12 bad=0 missing=0 split=0 | loops matched=7 raw_bad=0 rel_bad=7 only_arrayed=0 only_scalar=1 cross-element-ltm: values 11 checked/0 bad | edges ok=16 bad=0 missing=0 split=0 | loops matched=8 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=0 arrayed-population-ltm: values 15 checked/0 bad | edges ok=12 bad=0 missing=0 split=0 | loops matched=6 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=0 a2a_diagonal: values 15 checked/0 bad | edges ok=12 bad=0 missing=0 split=0 | loops matched=6 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=0 bare_reducer: values 7 checked/0 bad | edges ok=12 bad=0 missing=0 split=0 | loops matched=3 raw_bad=0 rel_bad=0 only_arrayed=3 only_scalar=3 cross_agg4: values 8 checked/0 bad | edges ok=20 bad=0 missing=0 split=0 | loops matched=15 raw_bad=0 rel_bad=15 only_arrayed=0 only_scalar=9 disjoint_dim: values 10 checked/0 bad | edges ok=14 bad=0 missing=0 split=0 | loops matched=6 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=0 dt_quarter: values 9 checked/0 bad | edges ok=18 bad=0 missing=0 split=3 | loops matched=13 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=1 fixed_broadcast: values 7 checked/0 bad | edges ok=5 bad=1 missing=0 split=0 | loops matched=3 raw_bad=1 rel_bad=0 only_arrayed=0 only_scalar=0 mapped_dims: values 9 checked/0 bad | edges ok=12 bad=0 missing=0 split=0 | loops matched=6 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=0 max_loop: values 6 checked/0 bad | edges ok=11 bad=1 missing=0 split=3 | loops matched=10 raw_bad=4 rel_bad=6 only_arrayed=0 only_scalar=1 mean_loop: values 6 checked/0 bad | edges ok=9 bad=3 missing=0 split=3 | loops matched=10 raw_bad=6 rel_bad=10 only_arrayed=0 only_scalar=1 migration_matrix: values 21 checked/0 bad | edges ok=27 bad=36 missing=0 split=9 | loops matched=283 raw_bad=274 rel_bad=283 only_arrayed=0 only_scalar=503 min_loop: values 6 checked/0 bad | edges ok=11 bad=1 missing=0 split=3 | loops matched=10 raw_bad=4 rel_bad=6 only_arrayed=0 only_scalar=1 per_element_eqns: values 6 checked/0 bad | edges ok=8 bad=0 missing=0 split=0 | loops matched=4 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=0 scalar_cofactor_inline: values 7 checked/0 bad | edges ok=12 bad=6 missing=0 split=3 | loops matched=13 raw_bad=9 rel_bad=13 only_arrayed=0 only_scalar=13 scalar_cofactor: values 8 checked/0 bad | edges ok=14 bad=0 missing=0 split=0 | loops matched=3 raw_bad=0 rel_bad=0 only_arrayed=4 only_scalar=4 share_sum: values 9 checked/0 bad | edges ok=18 bad=0 missing=0 split=3 | loops matched=13 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=1 sliced_sum: values 17 checked/0 bad | edges ok=24 bad=0 missing=0 split=0 | loops matched=12 raw_bad=0 rel_bad=0 only_arrayed=0 only_scalar=0 stddev_loop: values 6 checked/0 bad | edges ok=9 bad=3 missing=0 split=3 | loops matched=10 raw_bad=6 rel_bad=10 only_arrayed=0 only_scalar=1 two_d: values 12 checked/0 bad | edges ok=30 bad=12 missing=0 split=6 | loops matched=84 raw_bad=64 rel_bad=84 only_arrayed=0 only_scalar=352 variable_backed: values 7 checked/0 bad | edges ok=12 bad=0 missing=0 split=0 | loops matched=3 raw_bad=0 rel_bad=0 only_arrayed=3 only_scalar=3 Discovery's ranking statistic (mean_relative_contribution) divides through relative_series too, and the enumerated universe's totals (retain_circuits) go through add_to_total, so the summand rule has one owner on every path.
The flow-to-stock link score was the 2023 paper's Eq. 3 written over stock differences, `time_step * (PREVIOUS(flow) - PREVIOUS(PREVIOUS(flow)))` over the stock's second difference. Under Euler that is the paper's value one dt late, on a different window from every other link in the loop (GH #544); it breaks the paper's aggregation invariance at a fixed t; and it cost two nested-lag capture helpers per link and a two-step startup guard. Every stock with a scored flow-to-stock edge now gets a synthetic net-flow aux `$⁚ltm⁚net⁚{stock} = (inflows) - (outflows)`, shaped like the stock and minted beside the score by `shaped_link_score` (deduplicated by name), and the score is the ordinary instantaneous link score of that aux with respect to the flow: the paper's implementation option (b), section 4.4. The aux is a linear sum, so the partial is closed-form, `sign * |Δflow / Δnet|`, read over the same [t - dt, t] window as every other link, with no dt factor, no stock history, and a first value at the step after the start. The aux is not a causal node (it is in no loop and no link) and sorts ahead of every score in the evaluation order. A structural flow-to-stock edge has one emission owner: the Bare shape of the per-shape emitter, routed before the shape-driven emitters and without consulting the reference-site IR. That closes two wrong-number paths: a scalar flow into an arrayed stock took the scalar-to-arrayed emitter and was scored as a partial of the stock's initial-value equation (a stock's ast() is its initial); and a stock whose per-element initial reads its own flow had that initial's sites taken as the edge's shapes, minting one identical arrayed score per element, or, for a pinned-element read in a two-dimensional initial, diverting the edge to the per-element arm and dropping every loop through the stock. C-LEARN has two such scalar-flow edges (CH4 and CO2 emissions into their arrayed atmosphere stocks). Pinned in tests/integration/ltm_flow_to_stock.rs (new): the paper's table (in 5 -> 10, out 4 -> 5) scores 1.25 / -0.25 at the step of the change; a stock with separate nonlinear flows and its twin with one net flow agree on LS(flow -> stock) and on the relative loop scores to 1e-12 at the same t; births 0.1a / deaths a/20 split 2/3 : -1/3 from t = 1; a scalar flow into an arrayed stock scores 5/5.45 and 5/7.4 per element, the loop through it +1 with its drain loop at -2; an outflow-only stock scores exactly -1; a per-element initial reading its own flow yields one arrayed score with no warning; the net aux is absent from every loop and causal edge. The isolated-loop invariant holds at dt in {1, 0.5, 0.25, 0.125} with a one-step startup guard. The logistic golden keeps its +dt relabel: the shift is a labeling convention for every link score (the reference tool labels the score computed over [t, t + dt] with t), not a flow-to-stock timing effect, and the design and reference docs now say so. Re-baselined, each deliberately: the two LTM character goldens (only the flow-to-stock arms move, to the new guard form); the value-gate golden (growth -> pop is exactly 1 from t = 1, the net aux's slots appear); the two fragment goldens (fixture 10 gained a dynamic-index read so it still reaches the implicit-helper emission site, since the retired nested lag was its only helper -- the same reason five other fixtures now mint their helper through `PREVIOUS(weight[PREVIOUS(idx, idx)])`); the DELAY3 bound-port pin, 0, 1, 2, 4, 8 (was 0, 0, 1, 2, 4: the one-step relabel); and C-LEARN's LTM guardrail, 6,193 -> 6,224 variables (+35 net auxes, +2 arrayed scores, -6 per-element scalars) and 29,447 -> 28,725 slots (-888 nested-lag helper columns, +166 net-aux slots), derived in its rustdoc from `ltm_var_dump` and a column diff of `simlin simulate --ltm`. The Euler-only refusal keeps firing on flow-to-stock emission; its docs no longer cite the retired numerator as the reason.
The static polarity of an edge into a module instance was Positive for every input port, so a loop closed through a DELAY3's delay-time port was labelled r1 while its runtime score is negative at every step, and every structural-only surface (libsimlin and WASM get_loops, the layout, the r/b/u ids themselves) carried the wrong sign. A module's input port has no sign of its own: the sign the parent sees is the sign of the sub-model's internal pathways from that port to the output the parent reads, exactly the collapse the macro treatment applies to magnitudes. CausalGraph::module_input_polarity composes those pathways. Each link on a pathway is signed by the same get_link_polarity rules a top-level link gets (recursively, so a hop into a nested instance composes the same way), so there is one owner of what a link's sign is; the helper only multiplies signs along each pathway and compares. The edge is Positive or Negative when every pathway from every entry port the source feeds, to every output port the parent reads (the sub-model's sinks when it reads none), agrees; it is Unknown when any pathway carries an Unknown link, two pathways or two read ports disagree, the port reaches no read output, or the enumeration was truncated (a dropped pathway could disagree). Read ports are the union over every parent reader, loop or not, because the sign is a property of the edge (the per-exit-port link-score override already scores each loop against the pathway it traverses); a reporting aux that reads a second, opposite-signed port therefore turns the edge and every loop through it Unknown. A fed port that reaches no read output cannot carry a loop and is ignored. Matching the entry port through normalize_module_ref also signs a module-to-module edge wired from an output reference, which fell through to Unknown before, and the stock enrichment uses the same match. Sub-graphs now carry their own instances' graphs, recursively, so a user module wrapping a SMOOTH composes one level down; the recursion is bounded by the module-cycle gate model_detected_loops applies, and compute_link_polarities carries that gate too. Cost: signing a module edge re-enumerates the sub-model's pathways per call, so a graph build pays O(module edges) enumerations instead of one per instance (+28% on a 41-loop DELAY3 model in the reviewer's measurement). A per-(instance, exit set) memo would make it O(instances); it is a known cost, not fixed here. Pinned through model_detected_loops: DELAY3(inp, tau) with tau = 2 + 0.02*s is b1 (every delay_time -> output pathway is Negative: stock/(delay_time/3) on each); SMTH1(0.1*s, 4) stays r1; SMTH1(inp, tau) is b1, following the analyzer's division convention for (input - output)/delay_time (the true sign depends on the gap's sign; the runtime series decides Rux/Bux/U, and narrowing that convention would relabel every gap/adjustment_time link, so it is left alone). One test per arm of the rule in ltm/module_polarity_tests.rs, and pysimlin checks Model.loops and Run.loops agree on b1 for the DELAY3 loop. The module-tests expectation of u1 for a level -> m -> SMTH1 -> inflow loop encoded the old Unknown; every hop of that loop is positive and it is now r1.
The overlay was refused under RK2/RK4 whenever a model emitted a flow-to-stock link score (GH #486). The only justification was the old score's stock-history numerator, which aligned to the Euler update; the score is now a ratio of flow deltas over the stock's net-flow aux and reads no stock history, so the refusal has no premise left. A link score is a ratio of integration-step (dt) deltas reported at the saved steps: `PREVIOUS` reads the state the previous dt step ended in, because the VM snapshots `prev_values` on every dt iteration before the save/advance logic decides whether the row is recorded, so with save_step > dt a recorded score is the last dt step's ratio and not a re-differencing over the saved interval. That is the 2020 paper's form (Schoenberg, Davidsen and Eberlein, section 6.1: "computed at each dt"). Under RK2/RK4 the VM re-evaluates the flows at the restored end-of-step state before snapshotting it (the RK stages' trial-point evaluations are overwritten; wasm mirrors this), so the ratio is taken over the method's own trajectory. The paper puts Runge-Kutta compatibility as "in principle"; the design doc's note states this form and its boundary: cross-method equality holds for flows proportional to their stock, and in general the scores follow the method's trajectory. Pinned (tests/integration/ltm_integration_method.rs): an isolated single-flow loop and the births/deaths model score identically under Euler, RK2 and RK4 -- every link and loop score and every relative loop score equal to 1e-9 at every saved step -- while their stock trajectories differ; a nonlinear outflow (0.02 * s ^ 1.3 against births 0.1 * s) scores -22.744 under Euler and -22.749 under RK4 at t = 2; with save_step = 2 * dt the recorded scores are the dt-resolution run's at the saved times, and the ratio over the whole saved interval is a different number by more than 0.1 (a change to when prev_values is snapshotted fails this). Elsewhere: an RK4 model's LTM columns agree between wasm and the VM; mark2.mdl (a Vensim RK4 model) compiles and runs with the overlay and every model series is the LTM-free run's; through libsimlin an LTM sim on an RK4 model reports exhaustive mode and leaves the trajectory untouched; the MCP read_model reports an RK4 model's loop with no analysisError; pysimlin's run() on an RK4 model neither warns nor falls back to a run without loop analysis, and analyze() reports its loop. Removed with the guard: `effective_non_euler_method`, `model_emits_flow_to_stock_score`, `ltm_non_euler_diagnostic_message`, the GH #486/#663 test block in ltm_unified_tests (its per-element decorated-name probes included), and the refusal-expecting tests in libsimlin, MCP core, pysimlin and the wasm harness. The GH #660 "compile failure surfaces analysis_error" arm keeps its mechanism on a model whose flow reads an undefined variable at every surface (engine, libsimlin, MCP read_model on the wire, pysimlin); the message names the variable that failed to compile, never the missing reference. TestProject gains the `with_save_step` its `with_sim_time` comment already pointed at. The C header is regenerated from the FFI docs; tech-debt item 31 is resolved.
The Rux/Bux/U classification summed a loop's RAW loop scores over time. Raw scores are unbounded: at a dominance inflection every raw score in a partition diverges together, so a handful of inflection steps can outweigh the whole run, and an exogenous change that swamps a target's change shrinks one phase's raw scores to nothing. A series balancing for 200 steps and reinforcing for three inflection steps read confidence 0.23 on the raw base; a lone loop balancing for 40 of its 439 active steps read Rux at 0.9999 only because a ramp had made those 40 raw magnitudes tiny. Both callers -- reclassify_loops_from_results on the exhaustive path and rank_truncate_and_id on the discovery path -- now feed the classifier the loop's partition-relative series from the one normalization owner (compute_rel_loop_scores; the rel_scores discovery already attaches). Each sample is bounded to [-1, 1] and is the loop's share of its partition at that step, so the confidence is the dominance-weighted time share of each sign, and for a loop alone in its partition (relative score exactly +1/-1 while active) the plain time share of its sign: Mostly* then needs the minority sign on at most half a percent of the active steps. The papers define the ratio on instantaneous pathway scores and the base a reference tool uses for loops is undocumented, so this is a recorded judgment (types.rs and the design doc), not a reproduction. Discovery classifies inside rank_truncate_and_id, once the partition totals exist and before ids are assigned. A never-active loop is not a fallback case there: its zero/NaN series never passes retention, so discovery does not report it at all (the exhaustive surface keeps its structural label). Numbers pinned through the real pipeline (exhaustive reclassification, discovery parity, libsimlin Rux and Bux, pysimlin): the GH #679 lone-loop fixture reads Undetermined at its time share, 0.8178 with 399 reinforcing and 40 balancing steps (it was Rux 0.9999 on the raw base; three tests re-baselined with the reason, and the tests read the step counts off the series rather than pinning the number of startup steps); a competing-sibling fixture -- an inflow loop with gain +0.02 flipped to -0.02 for eight steps while the outflow loop's gain rises from 0.01 to 2 and carries the partition -- is MostlyReinforcing at 0.9994 (share 2/3 at about 430 steps, about 0.0099 at 8), and its outflow mirror MostlyBalancing at 0.9994; the yeast alcohol model's growth loop is Undetermined at 0.9655 with 146 positive and 53 negative steps; the synthetic 200-plus-3-step series reads 0.9706 on the relative base against 0.2308 on the raw one; an A2A loop with one reinforcing and one balancing element reads Undetermined at exactly 0 under the all-slots reading.
A per-element arm spelled "nyc, young" (or "NYC,Young") simulated every
element of its variable as 0 with no diagnostic. The compiler keyed the
arm by canonicalizing the whole subscript, which turns the space after
the comma into an underscore ("nyc,_young"), a key no combination of
canonical element names ever equals, so the arm matched nothing and the
element took the arrayed expansion's fabricated zero; the GH #905
advisory accepted the union of a per-part rule and the compiler's
whole-string rule, so the spelling one side resolved and the other
dropped was reported by neither.
CanonicalElementName::from_subscript (and its from_parts twin for a
subscript already split by SubscriptIterator) is now the one owner of
the key: split on the commas outside double quotes, trim and
canonicalize each part, join with ",". Every side of the match uses it
-- the arm keys parse_equation builds, the combination keys
expand_per_element, lower_arrayed_arms, unfilled_arms, the
builtins visitor's per-element instance expansion, the LTM lookup slot
classifier and the per-element GF table layout expand against, the
conveyor init-list key (delegates), the two advisories -- and the JSON,
protobuf and XMILE readers store the canonical spelling. The MDL reader
stores the source spelling; the datamodel invariant is only that every
consumer re-keys through the owner, stated on it, along with the
standing limitation that an element name containing a comma keys like
the combination of its pieces.
Severity follows the consequence, uniformly across sources. An arm
whose subscript names no element of the variable's dimensions is an
unused arm: the existing UnknownElementSubscript warning, now by the
one key. A declared element that no arm names and no EXCEPT default
applies to evaluates to the fabricated 0: a new MissingElementEquation
warning on the variable names every such element in row-major order
and says they evaluate to 0. Coverage is read off the parsed shape the
compiler expands (variable::elements_without_an_arm), never the entry
text: an entry with an empty equation is no arm, an empty EXCEPT
default covers nothing, and a gf-only entry is an arm because the
expansion wraps its table around the fabricated input. It is a
warning, not an error, on Vensim
ground truth rather than reasoning: Vensim defines a subscripted
variable on part of its range as a matter of course (h[DimA] :EXCEPT:
[SubA] = 8 defines h[A1] only), and its own export for the sdeverywhere
except model (test/sdeverywhere/models/except/except.dat) lists no
h[A2] at all -- the element does not exist there, so the 0 the MDL
import gives it is a value nothing in a valid Vensim model reads. As an
error it refused five corpus tests (except, except2, subscript,
directconst and the inline except_basic, whose test now pins the
warning naming a2 and a3 on h). The fidelity fix for imported models,
the MDL reader declaring a partially defined variable over the subrange
its arms cover, is GH #1059.
Pinned through the real pipeline: a proptest that any spacing and
capitalization of a one- to three-dimensional combination keys the
canonical join (and the join is a fixed point); the sliced_sum fixture
through Simlin JSON, where "nyc, young" and " Nyc , Young " now store
the canonical spelling and simulate bit-exact equal to the canonical
run across every saved series; the three arms -- a typo'd arm (one
UnknownElementSubscript warning naming nyc,yung and one
MissingElementEquation warning naming nyc,young on pop), a dropped
la,old arm (the warning names it and pop[la,old] is 0 at every step
while its siblings are untouched), an EXCEPT default covering the
dropped arm (no finding, simulates), and an arm for an element of
another dimension (warning only, simulates); the empty-equation entry,
the empty EXCEPT default and the XMILE element with neither equation
nor gf (each reported, each simulating as 0), and the gf-only element
(unreported, gf(0)); a protobuf-reader pin for a spaced, capitalized
subscript. The unfilled-equation test for a sparse array now also
asserts that the new warning names its armless slot. RealBeer4 and the
rest of the metasd and sdeverywhere corpora load and simulate
unchanged.
A per-element growth loop closed through a variable-backed reducer
(growth[r] = pop*0.02*(1 - total/1000), total = SUM(pop)) was reported
as growth -> pop[boston] -> total -> growth[boston]: four nodes for a
three-link loop, growth twice, once bare. The reported sequence was
read from links[0].from plus every link's to, and Link.from doubles as
the link-score name-resolution flag: build_element_level_loops strips
it to the variable level on a same-element A2A hop (a bracketed from
selects the FixedIndex / cross-dimensional score name, an unbracketed
one the Bare or per-target-element form), while to keeps the element
subscript whenever the target is arrayed. Raw scores were right;
identity was not, so the de-subscripting oracle could not match these
loops to the scalar model's circuits.
loop_node_sequence in db/analysis.rs is now the one owner of the
reported sequence: the enumerated loops of model_detected_loops, a
pin's detected_loop_from_loop, and discovery's FoundLoop summaries
(analysis.rs loop_variables, whose from-led reading equals the to-led
one on discovery's element-level links) all read through it. It reads
the circuit off each link's to, starting from the last link's (the
first node), never from from: every node of an elementary circuit is
the to of exactly one link, and a stitched cross-agg loop visits only
its agg twice, which the existing synthetic-agg trim absorbs (the
stitcher takes pairwise node-disjoint petals, db/ltm/loops.rs, so no
other node can repeat; a debug assertion states that). Only the mixed
branch of build_element_level_loops strips from; the cross-element
builder keeps it subscripted. Links, ids and the pin-dedup rotation
are untouched, so the loop-score equations are the same text as
before: the LTM dumper's every synthetic-variable equation and every
saved series on bare_reducer, variable_backed and scalar_cofactor are
identical before and after.
Pinned: the three mixed loops of bare_reducer (SUM(pop)) and
variable_backed (SUM(pop[*])) report growth[e] -> pop[e] -> total, each
node once, beside the A2A loop pop -> growth, through Simlin JSON in
the engine and through pysimlin Model.loops and Run.loops;
scalar_cofactor's reducer loops report growth[e] -> pop[e] -> weighted
with every sequence visiting each node once; a pin naming {growth, pop,
total} in discovery mode reports its three element-level instances
(pin1:1..3) as the same circuits through the pin path; and the owner's
unit tests cover the same-element A2A hop, bare and cross-element
links, a slotted agg hop and an empty loop. On the de-subscripting oracle every
loop of the three models now matches the scalar model's circuit (6, 6
and 7 matched, none unmatched on either side; before, the three mixed
loops of each were unmatched on both sides).
The changed-first partial froze model dependencies only, so TIME, STEP, RAMP and PULSE advanced inside it: a source with no influence on its target scored +/-1 whenever an exogenous forcing moved the target, and the forcing was credited to every link into it (GH #1016; 41% of C-LEARN's link-score arms carried a live time term). The 2020 paper's partial f(x_t, y_{t-1}) - z_{t-1} reads every input but the isolated one at the previous step, and the clock is an input of f like any other; the identity "a source f does not read scores 0" only closes with the clock among the "all other inputs". This is the reading of the formula; what Vensim and Stella do here is unverified and the decision does not rest on it. One rule, at both changed-first freeze sites (the wrap walker, whose index pass and table-index freeze route through it, and the reducer-body freeze): a call of a builtin the compiler itself classifies Invariance::TimeDependent (the one owner of "reads the clock") is read at the previous step -- a bare TIME through the per-model helper $⁚ltm⁚freeze⁚time = PREVIOUS(TIME), any other call lagged whole with its arguments verbatim (a capture lags the call once; lagging a frozen dep inside it as well would read it two steps back) -- unless the call holds an occurrence of the isolated input's live shape, the occurrence IR's verdict, in which case the call stays live, clock included: the isolated input and the clock are then inseparable in one call, and its whole change is attributed to the input. That residual is documented and pinned, not diagnosed; a read of another element of an arrayed input is not the live shape and freezes with the call. Inside a frozen dependency's subscript index the call is left verbatim, because the enclosing freeze already lags the whole read once (PREVIOUS(arr[TIME]) is arr_{t-1}[TIME_{t-1}]); a model dependency in that position is lagged twice today, the pre-existing residual the zero-slot predicate documents, and the clock does not join it. The run constants DT, INITIAL_TIME and FINAL_TIME are Pure and untouched. The changed-last duals leave the clock live deliberately: their numerator reads every other input at t on both sides, so the clock's motion cancels there. The guard form's own TIME = INITIAL_TIME arm is outside the partial. The helper exists because a frozen clock is one value for the whole model: spelled inline, PREVIOUS(TIME) is a capture per arm and per occurrence, which on C-LEARN v77 cost 9,632 result slots (a third of the row). With the helper, minted once by model_ltm_variables when any arm reads it and registered as a freeze helper so it sorts ahead of every score, the width moves 28,725 -> 28,980 (+255: 36 helper instances, their 36 captures, and 183 captures of time-dependent calls frozen whole) and the emitted count 6,224 -> 6,227 (the helper in main, the ramp_from_to macro model and the stdlib npv template). The zero-slot predicate recognizes the helper as the one-step lag it is, so an arm whose only varying content was the clock is now provably PREVIOUS(target) and omitted to an exact zero. Pinned (tests/integration/ltm_frozen_clock.rs): x = 5 + STEP(2, 2) + 0*s scores LS(s -> x) = 0 at every step (was 1 at t = 2); the isolated loop f = 0.1*s + RAMP(1, 0) scores LS(s -> f) = 0.1*Δs/|Δf| = 0.500, 0.545, 0.587, 0.624, 0.658 at t = 1..5 with the loop score equal to it (was 1); z = x * TIME scores the changed-first numerator x_t * TIME_{t-1} - z_{t-1}, distinguishable from the clock-live form (a constant 1) at every step; the GH #763 repro grow = 1 + MIN(pop[*] * TIME) keeps the PREVIOUS(agg) anchor, so the row that is never the argmin scores 0; z = x + arr[TIME] over arr = [1, 2, 4, 8, 16] scores 0.1667, 0.1379, 0.1213, 0.1118 at t = 2..5, the once-lagged form, where a twice-lagged clock flips the sign from t = 3; and f = 1 + STEP(0.1*s, 2) pins the residual. The text-level rows enumerate every Invariance class a partial can meet, both changed-last duals keep their clock live, and the helper is minted once per model and only when read. Measured on the LTM corpus and C-LEARN v77 with a simlin simulate --ltm run on the previous and the new CLI: the five corpus models (logistic_growth, hero_culture, arrayed_population, cross_agg, cross_element) change no link-score or loop-score series. C-LEARN changes 146 of 5,998 link-score series, 86 of them to an identical zero; every one lies on an edge no retained loop uses, and analyze() reports the same 153 loops, identical relative-score series and identical 565 dominant periods before and after, so no dominant loop changes at any step. The remaining changes replace the clock-credited +/-1 with the paper's value: rs_global_pfc -> proportion_of_global_to_cop_pfc[..] (ZIDZ of a lookup of Time over the global sum) goes from an alternating +/-1 to the large negative ratio of a target that barely moves while its inputs cancel, the "very large values" case section 3.1 of the reference describes. The C-LEARN slot-maxima digest is re-pinned with its derivation split across the two commits that moved it: the net-flow aux commit (05e4dae) moved slots 20,221 -> 20,337 and nonzero slots 3,106 -> 3,198 without re-pinning this #[ignore]d release-only gate, and the frozen clock moves them 20,337 -> 20,338 and 3,198 -> 2,876 (the omitted clock-only arms). The lookup-pin and value-gate expectations are re-derived under the frozen clock; the reference doc's section 3.1 note, the design doc, the engine CLAUDE.md and the performance doc state the rule.
The 2026-09-07 LTM audit established that both hero models auto-flip to discovery, so the exhaustive path's compile-time loop list is empty on every model that matters and the TS engine has no discovery binding; that the post-simulation product of a loop's recorded link scores reproduces the in-simulation loop_score column to one ulp on all seven fixtures; and that all-edge instrumentation costs no more than loop-edge instrumentation plus per-loop score variables. This plan makes the post-simulation pipeline the only loop-scoring path, keeps Johnson only for the structural preview and ids, injects pins into the pipeline, treats an inline reducer as a real loop node, deletes static polarity and the shortest-path fallback, and gates sub-model pathway emission on SCC membership. The plan embeds five product decisions (ids without a polarity letter, element-level loops as the reported unit, named-aggregate reducer semantics, static polarity deleted, synthetic CLI columns hidden) and its acceptance criteria are drafted, not yet validated with the owner; both are called out in the document.
Every one of the hero_culture fixture's fourteen loopMetadata entries
was rejected at sync ("a pinned loop must name at least two
variables"), so the fixture never exercised a pin and the rejection
named only the loop's name, five of which read "Technical Debt Spiral".
The fixture was the wrong side. LoopMetadata.uids are VARIABLE uids
everywhere else: the TypeScript types say so, SetLoopName mints a uid
on each named variable, and the MCP open path assigns every variable a
uid so that SetLoopName can resolve it. The fixture's entries were
written with the uids of the variables' diagram elements while no
variable carried a uid at all, so the projection resolved nothing.
Variable and view-element uids share one number space
(patch::next_available_uid takes the maximum over both), so the
regenerated fixture gives each of the 27 variables the uid of its own
element and leaves the entries untouched; all fourteen now resolve,
validate against the causal graph, and score.
A pin that cannot be scored is now reported with the entry's own
identity: PinnedLoopSpec carries the entry's uids as written, the
subset that matched no variable, and whether any variable of the model
carried a uid at sync (uids live only on the datamodel, so the fact
travels on the spec); every rejection arm goes through one reject
site that appends the unresolved-uid clause, so the two-variable gate
reads "'Ghost' (uids [7, 4]) names 0; uids [4, 7] match no variable's
uid; no variable in the model carries a uid (a pin names variables by
their uid, which SetLoopName assigns)", the not-a-cycle arm names the
entry's uids the same way, and the no-stock and failed-expansion arms
name a dropped uid too. A pin that drops a stale uid but whose
survivors still form a cycle scores that cycle and says so, naming the
dropped uids and the cycle scored instead ("... dropped uids [99],
which match no variable's uid, and scored the loop its remaining
variables form instead: births -> population"): a renamed or
re-created variable must never silently turn a pin into a different
loop under the same name. The name alone was never an identity.
Pinned: the fixture's fourteen pins produce no warning and, in
discovery mode, fourteen non-trivial pin{n} loop scores whose
relative scores are finite, bounded and non-zero; a pin on a model
whose variables carry no uid is reported with its name, its uids, the
unresolved uids and the no-uid cause; a pin with one stale uid beside
a resolving one names the stale uid and not the no-uid cause; a pin
whose survivors are no cycle names its uids and the dropped one; a pin
with a stale uid beside two resolving ones scores their cycle under
pin1 and warns naming it, and that warning has its row in the
once-per-project warning-family test.
The eight LTM fidelity fixes were implemented on two branches with disjoint file ownership so two implementers could work in parallel: ltm-fidelity carried the scoring-generator fixes (net-flow flow-to-stock scores, LTM under every integration method, the frozen clock) and ltm-fidelity-post the post-simulation and structural ones (per-partition relative scores, input-to-module polarity, the relative-series polarity base, the element-subscript key owner, loop node sequences, pin identity). The only textual conflict was the integration harness's module list. Two runtime-polarity fixtures from the post-simulation branch pinned exact active-step counts that assumed the pre-merge two-step startup; on the merged tree the first scored step is start + dt, so the yeast test now reads its counts off the series and the balanced-loop fixture flips at TIME < 12 so its sign split (48/32) is unbalanced by a margin rather than by an odd step count.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1062 +/- ##
========================================
Coverage 93.22% 93.22%
========================================
Files 253 253
Lines 153504 153270 -234
========================================
- Hits 143098 142892 -206
+ Misses 10406 10378 -28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Why
A four-way audit of the Loops That Matter implementation against the five papers (reports and re-runnable experiments in the session scratchpad; summary in the memory note
ltm_audit_2026_09_07) found that the instantaneous link scores, loop and relative scores, cycle partitions and module composites reproduce the papers exactly, and that eight things do not. This PR fixes the eight, one hook-verified and adversarially reviewed commit each, and adds the design plan for the next work package.What changed
05e4dae4). The score was the 2023 paper's Eq. 3 written over stock second differences, which put it one step late, broke the paper's aggregation invariance at fixed t (identical split-flow and net-flow models disagreed by up to 0.86 in link score), and put a loop's links on two time windows (GH ltm-augment: instantaneous and flow-to-stock link scores use inconsistent time windows within a loop #544; the dominant loop differed at 7 of 229 steps on a two-stock oscillator at dt 0.25). Every flow-to-stock score is now the instantaneous partial of$⁚ltm⁚net⁚{stock}, no dt factor, no stock history, first value at start+dt. Also fixes a scalar flow into an arrayed stock being scored against the stock's initial-value equation (two edges in C-LEARN). The golden fixture's one-step shift is a labeling convention for every link (the reference tool labels the [t, t+dt] score with t), verified by inverting the closed-form share on each golden column; the docs no longer attribute it to flow-to-stock timing.3afd6376). The Euler-only refusal (GH ltm: emit compile-time error when ltm_enabled is true and integration is non-Euler #486) was justified only by the stock-history numerator. Scores are ratios of integration-step deltas reported at saved steps; a proportional-flow model scores identically under Euler, RK2 and RK4 (pinned), and asave_step = 2 dtrun pins the dt-step form.f5e03257, GH ltm: should a ceteris-paribus partial freeze TIME? (today it does not, and the target's autonomous time evolution is attributed to the link) #1016). TIME, STEP, RAMP and PULSE were live inside partials, so an input with zero influence scored 1 and exogenous forcing was credited to every link (41 percent of C-LEARN's arms carried a live time term). One rule at both changed-first freeze sites; the changed-last duals keep the clock live because it cancels; a call holding an occurrence of the isolated input's live shape stays live (documented residual). C-LEARN's 153 loops, relative series and 565 dominant periods are unchanged; 146 link-score series change, 86 to an exact zero.8cbcf0cf). Normalization was per (partition, slot) bucket, which produced wrong dominance verdicts on every coupled arrayed model (cross_element's births loop scored 1/2 and 2/3 instead of 1/3). One function now serves discovery, the FFI, pysimlin and the layout; the de-subscripting oracle's relative-score rows are exact on all 22 audit models.6a90255e). The edge was hard-coded positive, so loops through a DELAY3 or SMTH1 delay-time port were labelled reinforcing while scoring negative at every step.95f848ff). Raw-sum confidence let three inflection steps outweigh two hundred balancing steps. A loop alone in its partition now has a confidence equal to the time share of its sign; the lone-loop Rux fixtures re-baseline to Undetermined and competing-sibling fixtures keep the Rux and Bux arms covered at every surface.e5319fbd). A JSON subscript written"nyc, young"never matched the compiler's key and silently zeroed every element with no diagnostic.CanonicalElementName::from_subscriptis the one key owner across the compiler, the readers and the advisory; a declared element with no equation arm is a Warning naming it (kept a Warning because Vensim's own exports omit undefined elements; the reader-side fix is GH engine: MDL reader declares partially defined subscripted variables over the full dimension and fabricates zeros for undefined elements #1059).e85914f5). Mixed loops through a named reducer reportedgrowth -> pop[boston] -> total -> growth[boston]; the sequence now comes from one owner reading the circuit.b27550ae). The hero_culture fixture's fourteen pins were all rejected at sync because the fixture used view-element uids where loop metadata means variable uids everywhere else; the fixture is regenerated, a pin that cannot be scored reports its name, uids and the unresolved ones, and a pin that drops a stale uid but still scores warns and names the cycle it scored.765b7a3b,docs/design-plans/2026-09-07-ltm-single-mode.md). The audit's architectural verdict: both hero models auto-flip, so the structural loop surface is empty on every model that matters; post-simulation loop scores match in-simulation ones to one ulp; single mode deletes the exhaustive path, the fallback and static polarity. Its acceptance criteria and five embedded product decisions are drafted for the owner's validation, not yet executed.Evidence
z = x*TIME, the frozen-index model, cross_element shares 1/3, 1/3, -1/6, +1/6, the competing-sibling Rux/Bux fixtures at 0.9994, the yeast loop at 0.967 with 147 positive and 53 negative steps on the merged tree.What the evidence does not establish
Fixes #544
Fixes #1016
🤖 Generated with Claude Code
https://claude.ai/code/session_011UGXEWqCKwNEVU75MUduNR