Skip to content

Commit a8332b9

Browse files
orebasclaude
andcommitted
Postcampaign P0+P1 batch: ordering convention, _has_trfn, projection-NaN, warns, seeds, guard
P0 (probe-adjudicated): - #1 process_raw_solution: probe = LATENT (0/79 registry models reorder under MTK 11, adversarial declaration order preserved) — fixed as convention unification: the pre-err parameter pass resolves by findfirst against MTK order exactly like the states loop (identical behavior when orders match); the redundant post-err second pass is deleted; the seeded-rescue producer's MIXED-convention raw_sol (states MTK-ordered, params original-ordered) is unified to MTK order. - #2 _has_trfn checked p_true, but trfn vars are transform-added STATES (live in ic) — was always false, so multipoint never disabled for transcendental models. Now checks ic ∪ p_true. - #3 MP→SP projection fabricated 0.0 for unmapped vars — now NaN (fails the candidate's ODE solve → err 1e15 → ranked out; the 0.0 solved plausibly). Same class as the Phase-C data-evaluator fixes. P1: - The three multipoint failure catches (template build / combo eval / HC solve) no longer gate their @warn on opts.diagnostics (maxlog=10) — a systematically failing multipoint stage was invisible at default verbosity. - Structural RNG seeded: the QR-pivot draw deciding WHICH unidentifiable parameter gets fixed; both template rank-probe loops (matching the already-seeded noise-frontier twin); the terminal direct-opt rescue (hash-derived per problem); _track_gamma_straight's default rng. fast_core's lone unseeded noisy sampling seeded (+ using Random). - Final polish ODE solve guarded (try/catch + polish_ode_maxiters, mirroring the in-loss solve) — a throwing integrator in the terminal rescue paths no longer kills the estimation. Tracker checkboxes updated. Full suite: 755/755 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 136b23e commit a8332b9

6 files changed

Lines changed: 79 additions & 32 deletions

docs/2026-06-10_postcampaign_review.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ surface (261 unique exports) plus two god files remain the big structural debts.
1616

1717
## P0 — correctness / broken-now (probe, then fix)
1818

19-
- [ ] **#1 `process_raw_solution` parameter-ordering self-disagreement.** First pass
19+
- [x] **#1 `process_raw_solution` parameter-ordering self-disagreement.** FIXED 2026-06-10 — probe: LATENT (0/79 registry models + adversarial case reorder under MTK 11); single findfirst pass, second pass deleted, seeded-rescue producer's MIXED-convention vector unified to MTK order. First pass
2020
assigns params positionally by `original_parameters` index, solves the ODE and
2121
computes `err` from that assignment; a second pass then re-keys the same dict by
2222
`findfirst` in MTK order (`parameter_estimation.jl:779-783` vs `:830-840`; states
@@ -26,13 +26,13 @@ surface (261 unique exports) plus two god files remain the big structural debts.
2626
err/solution and the returned parameter dict describe DIFFERENT vectors.
2727
VERIFIED code; latent (no CI model reorders). **Probe:** a model whose MTK
2828
param order differs from declaration order; assert err matches the dict.
29-
- [ ] **#2 `_has_trfn` inspects `p_true`, but trfn vars live in `ic`**
29+
- [x] **#2 `_has_trfn` inspects `p_true`, but trfn vars live in `ic`**
3030
always false, so multipoint never disables for transcendental models
3131
(`optimized_multishot:1731` vs `transcendental_utils.jl:620-633`); downstream
3232
the count-mismatch fallback bakes PROBE-point data into combo equations
3333
(`multipoint_template.jl:771-790`) → wrong-time candidates with wrong
3434
provenance, silently. VERIFIED.
35-
- [ ] **#3 Multipoint→single-point projection fabricates `0.0`** for unmapped vars
35+
- [x] **#3 Multipoint→single-point projection fabricates `0.0`** for unmapped vars
3636
(`optimized_multishot:2457`) — the exact class Phase C exterminated in the data
3737
evaluators. Fix: NaN (existing finite guards reject). VERIFIED mechanism.
3838
- [ ] **#4 `Pkg.test()` is broken: `Random` missing from the test target**
@@ -48,7 +48,7 @@ surface (261 unique exports) plus two god files remain the big structural debts.
4848

4949
## P1 — silent failure / state / flakiness
5050

51-
- [ ] **Multipoint failures invisible at default verbosity:** template-build,
51+
- [x] **Multipoint failures invisible at default verbosity:** template-build,
5252
combo-eval, and multipoint-HC catches all gate their `@warn` on
5353
`opts.diagnostics` (`optimized_multishot:2313,2406,2435`) — incl. the Phase-C5
5454
one whose comment promises the opposite. Ungate with `maxlog`. VERIFIED.
@@ -66,7 +66,7 @@ surface (261 unique exports) plus two god files remain the big structural debts.
6666
`_NOISE_VALIDATION_CACHE` keyed by `objectid` — unbounded session growth +
6767
GC-reuse wrong-Jacobian collision risk (`noise_frontier_construction.jl:664`).
6868
VERIFIED growth/race chain; INFERRED collision.
69-
- [ ] **Unseeded RNG in structural decisions + stochastic src paths reachable from
69+
- [x] **Unseeded RNG in structural decisions + stochastic src paths reachable from
7070
CI:** `_rank_based_fix_candidates` pivot RNG decides WHICH unidentifiable param
7171
gets fixed (`parameter_estimation.jl:420`); rank-probe `randn` in template
7272
stripping (`multipoint_template.jl:135,283` — the NF twin seeds the identical
@@ -82,7 +82,7 @@ surface (261 unique exports) plus two god files remain the big structural debts.
8282
`data_values` shorter than `data_vars` (`multipoint_template.jl:1062,1139`);
8383
legacy range-builder admits non-contiguity the NF builder sorts away. Assert
8484
equality. VERIFIED code / INFERRED trigger.
85-
- [ ] **Unguarded final ODE re-solve in polish** kills the whole estimation in the
85+
- [x] **Unguarded final ODE re-solve in polish** kills the whole estimation in the
8686
nothing-else-worked path (`parameter_estimation.jl:2375`; bare callers
8787
`optimized_multishot:2910`, `parameter_estimation.jl:2836`). Wrap like the loss.
8888
VERIFIED.

src/core/homotopy_continuation.jl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,9 @@ This is distinct from `ParameterHomotopy` (the straight real parameter path, no
749749
real discriminant and stall (`terminated_max_steps`).
750750
"""
751751
function _track_gamma_straight(hc_system, starts, p_start, p_target;
752-
show_progress = false, rng = MersenneTwister(), max_seeds = 5, target_count = length(starts))
752+
show_progress = false, rng = MersenneTwister(0x67a44a01), max_seeds = 5, target_count = length(starts))
753+
# Deterministic default rng (postcampaign review P1): all in-package callers
754+
# pass rng explicitly; an unseeded default made direct calls run-to-run random.
753755
ps = ComplexF64.(p_start)
754756
pt = ComplexF64.(p_target)
755757
best_res = nothing

src/core/multipoint_template.jl

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,11 @@ function _rank_aware_topdown_strip(equations::Vector, variables::Vector,
131131
f_combined = _compile_system_function(equations, variables)
132132
J_full = nothing
133133
best_full_rank = 0
134+
# Seeded rank probes (postcampaign review P1): equation-stripping decisions
135+
# must be deterministic; the noise-frontier twin already seeds identically.
136+
probe_rng = MersenneTwister(0x9f4d0329)
134137
for _ in 1:n_rank_probes
135-
rand_point = randn(n_var) .* 10.0
138+
rand_point = randn(probe_rng, n_var) .* 10.0
136139
J_probe = ForwardDiff.jacobian(f_combined, rand_point)
137140
r = rank(J_probe; atol = rank_atol)
138141
if r > best_full_rank
@@ -279,8 +282,11 @@ function _sensitivity_aware_strip(equations::Vector, variables::Vector,
279282
f_combined = _compile_system_function(equations, variables)
280283
J_full = nothing
281284
best_full_rank = 0
285+
# Seeded rank probes (postcampaign review P1): equation-stripping decisions
286+
# must be deterministic; the noise-frontier twin already seeds identically.
287+
probe_rng = MersenneTwister(0x9f4d0329)
282288
for _ in 1:n_rank_probes
283-
rand_point = randn(n_var) .* 10.0
289+
rand_point = randn(probe_rng, n_var) .* 10.0
284290
J_probe = ForwardDiff.jacobian(f_combined, rand_point)
285291
r = rank(J_probe; atol = rank_atol)
286292
if r > best_full_rank

src/core/optimized_multishot_estimation.jl

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,7 +1728,13 @@ function optimized_multishot_parameter_estimation(PEP::ParameterEstimationProble
17281728

17291729
# Check if we should use parameter homotopy and/or multi-point template.
17301730
# Both can run — single-point always runs, multipoint adds solutions to the pool.
1731-
_has_trfn = any(startswith(replace(string(k), "(t)" => ""), "_trfn_") for k in keys(PEP.p_true))
1731+
# trfn vars are STATES added by transform_pep_for_estimation — they land in
1732+
# PEP.ic, not p_true (postcampaign review P0#2: the old p_true-only check was
1733+
# always false, so multipoint never disabled for transcendental models).
1734+
_has_trfn = any(
1735+
startswith(replace(string(k), "(t)" => ""), "_trfn_")
1736+
for k in Iterators.flatten((keys(PEP.ic), keys(PEP.p_true)))
1737+
)
17321738
use_multipoint = opts.use_multipoint && opts.system_solver == SolverHC && !_has_trfn
17331739
use_param_homotopy = opts.use_parameter_homotopy && opts.system_solver == SolverHC && n_points >= 3
17341740

@@ -2311,7 +2317,9 @@ function optimized_multishot_parameter_estimation(PEP::ParameterEstimationProble
23112317
n_points = opts.multipoint_n_points, diagnostics = opts.diagnostics)
23122318
end
23132319
catch e
2314-
opts.diagnostics && @warn "[MULTIPOINT] Template build failed" exception = e
2320+
# Ungated (postcampaign review P1): a systematically failing template
2321+
# build must not be invisible at default verbosity.
2322+
@warn "[MULTIPOINT] Template build failed" exception = e maxlog = 10
23152323
nothing
23162324
end
23172325
if !isnothing(result)
@@ -2406,7 +2414,7 @@ function optimized_multishot_parameter_estimation(PEP::ParameterEstimationProble
24062414
catch e
24072415
# A systematic combo-eval failure must not be
24082416
# indistinguishable from "no combos available".
2409-
opts.diagnostics && @warn "[MULTIPOINT] combo evaluation failed" exception = (e, catch_backtrace())
2417+
@warn "[MULTIPOINT] combo evaluation failed" exception = (e, catch_backtrace()) maxlog = 10
24102418
end
24112419
end
24122420
_t_mpt_eval_elapsed = time() - _t_mpt_eval_start
@@ -2433,7 +2441,7 @@ function optimized_multishot_parameter_estimation(PEP::ParameterEstimationProble
24332441
precomputed_generic_solutions = _mpt_sols0,
24342442
precomputed_generic_params = _mpt_p0)
24352443
catch e
2436-
opts.diagnostics && @warn "[MULTIPOINT] HC solve failed" exception = e
2444+
@warn "[MULTIPOINT] HC solve failed" exception = e maxlog = 10
24372445
nothing
24382446
end
24392447
_t_mpt_solve_elapsed = time() - _t_mpt_solve_start
@@ -2453,8 +2461,12 @@ function optimized_multishot_parameter_estimation(PEP::ParameterEstimationProble
24532461
n_projected = 0
24542462
for (pidx, combo_sols) in enumerate(solutions_by_combo)
24552463
for sol in combo_sols
2456-
# Project: extract only the components matching single-point vars
2457-
projected = Float64[idx > 0 ? sol[idx] : 0.0 for idx in mp_to_sp]
2464+
# Project: extract only the components matching single-point vars.
2465+
# Unmapped vars are NaN (NEVER 0.0 — a fabricated value enters the
2466+
# candidate pool as plausible data; the downstream finite guards
2467+
# reject NaN cleanly). Postcampaign review P0#3, same class as
2468+
# the Phase-C data-evaluator fixes.
2469+
projected = Float64[idx > 0 ? sol[idx] : NaN for idx in mp_to_sp]
24582470
push!(interp_solutions, projected)
24592471
push!(interp_time_indices, evals[pidx].time_indices[1])
24602472
push!(interp_source_types, :multipoint)
@@ -2775,7 +2787,12 @@ function optimized_multishot_parameter_estimation(PEP::ParameterEstimationProble
27752787
end
27762788

27772789
raw_sol = raw_ic
2778-
append!(raw_sol, Float64[params_dict[p] for p in PEP.model.original_parameters])
2790+
# Parameter block in CURRENT (MTK) order — the convention
2791+
# process_raw_solution decodes; the states above (raw_ic)
2792+
# are already MTK-ordered (unknown_syms). Was
2793+
# original_parameters: a MIXED-convention vector
2794+
# (postcampaign review P0#1).
2795+
append!(raw_sol, Float64[params_dict[p] for p in ModelingToolkit.parameters(PEP.model.system)])
27792796

27802797
ordered_s, ordered_p, ode_solution, err = _with_detailed_timing_context(:algebraic_resolve_seeded_candidate) do
27812798
process_raw_solution(
@@ -2893,8 +2910,13 @@ function optimized_multishot_parameter_estimation(PEP::ParameterEstimationProble
28932910
# Draw on unit scale and clamp into bounds when present — uniform-in-bounds
28942911
# under ±1e9 default bounds almost always blows up the ODE and Optim 2's
28952912
# Fminbox hard-errors on the resulting NaN initial mu.
2913+
# Seeded per problem (postcampaign review P1): unseeded draws made
2914+
# this terminal rescue a per-session coin flip — the same class as
2915+
# the direct-opt canary flake (ddb06b4). Hash-derived like the
2916+
# RESOLVE-SEED pattern above.
2917+
_draw_rng = MersenneTwister(UInt(abs(hash((PEP.name, :direct_opt_fallback_seed)))))
28962918
_draw = scale -> begin
2897-
raw = scale .* randn(p_size)
2919+
raw = scale .* randn(_draw_rng, p_size)
28982920
(isnothing(ctx.lb) || isnothing(ctx.ub)) ? raw : clamp.(raw, ctx.lb, ctx.ub)
28992921
end
29002922
p0 = _draw(1.0)

src/core/parameter_estimation.jl

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,13 @@ function _rank_based_fix_candidates(candidate_vars, symbolic_identifiable_funcs,
415415
for f in funcs_filtered
416416
union!(all_vars, Symbolics.get_variables(f))
417417
end
418+
# Seeded (postcampaign review P1): this Jacobian's pivoted-QR column order
419+
# decides WHICH unidentifiable parameter gets fixed to 1.0 — a structural
420+
# decision that must not vary run-to-run with the global RNG.
421+
fix_probe_rng = MersenneTwister(0x9f4d0329)
418422
val_dict = Dict{Num, Float64}()
419423
for v in all_vars
420-
val_dict[v] = 0.5 + rand()
424+
val_dict[v] = 0.5 + rand(fix_probe_rng)
421425
end
422426
J_num = Array{Float64}(undef, length(funcs_filtered), length(param_syms))
423427
for i in 1:size(J_sym, 1)
@@ -776,10 +780,18 @@ function process_raw_solution(raw_sol, model::OrderedODESystem, data_sample, ode
776780
ordered_states[state] = raw_sol[idx]
777781
end
778782

779-
# Reorder parameters according to original ordering
783+
# Reorder parameters according to original ordering. raw_sol's parameter block
784+
# is in CURRENT (MTK) order — the same convention as the states block above and
785+
# as the producers (unpack_ODE-based assembly). Resolve each original parameter
786+
# to its CURRENT index, exactly like the states loop.
780787
param_offset = length(current_states)
781788
for (i, param) in enumerate(model.original_parameters)
782-
ordered_params[param] = raw_sol[param_offset+i]
789+
idx = findfirst(p -> isequal(p, param), current_params)
790+
if isnothing(idx)
791+
@warn "Parameter $param not found in current parameters, using original index $i"
792+
idx = i
793+
end
794+
ordered_params[param] = raw_sol[param_offset+idx]
783795
end
784796

785797
ic = collect(values(ordered_states))
@@ -827,17 +839,10 @@ function process_raw_solution(raw_sol, model::OrderedODESystem, data_sample, ode
827839
end
828840

829841

830-
# Reorder parameters according to original ordering
831-
param_offset = length(current_states)
832-
for (i, param) in enumerate(model.original_parameters)
833-
# Find the index of this parameter in the current parameters
834-
idx = findfirst(p -> isequal(p, param), current_params)
835-
if isnothing(idx)
836-
@warn "Parameter $param not found in current parameters, using original index $i"
837-
idx = i
838-
end
839-
ordered_params[param] = raw_sol[param_offset+idx]
840-
end
842+
# (The former post-err second parameter pass was folded into the single pass
843+
# above — postcampaign review P0#1: two passes with different ordering
844+
# conventions meant the ODE solve/err and the returned dict could describe
845+
# different parameter vectors whenever MTK order != declaration order.)
841846

842847
_record_detailed_timing!((
843848
category = :process_raw_solution,
@@ -2372,7 +2377,15 @@ function _polish_single_from_context(
23722377
p = Dict(ctx.param_syms .=> param_opt),
23732378
build_initializeprob = false,
23742379
)
2375-
sol_final = ModelingToolkit.solve(prob_final, ctx.solver; saveat = ctx.t_vector, abstol = ctx.abstol, reltol = ctx.reltol)
2380+
# Guarded like the in-loss solves (postcampaign review P1): a throwing
2381+
# integrator here (e.g. SingularException on a rank-deficient candidate) sits
2382+
# in the nothing-else-worked terminal paths and must not kill the estimation.
2383+
sol_final = try
2384+
ModelingToolkit.solve(prob_final, ctx.solver; saveat = ctx.t_vector, abstol = ctx.abstol, reltol = ctx.reltol, maxiters = ctx.polish_ode_maxiters)
2385+
catch e
2386+
@warn "Final polish ODE solve threw; returning result without trajectory" exception = e maxlog = 10
2387+
nothing
2388+
end
23762389

23772390
# Map back to user-facing ordering
23782391
states_out = OrderedDict(s => ic_opt[ctx.state_index[s]] for s in ctx.state_syms_out if haskey(ctx.state_index, s))

test/fast_core.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ using Logging
33
using HomotopyContinuation
44
using ModelingToolkit
55
using OrderedCollections
6+
using Random
67

78
@testset "Fast Core Contracts" begin
89
@testset "Ordered ODE construction" begin
@@ -1841,6 +1842,9 @@ using OrderedCollections
18411842
@test isempty(fallback_ident.all_unidentifiable)
18421843
@test isempty(fallback_ident.good_DD.all_unidentifiable)
18431844

1845+
# Seeded: this was the only unseeded noisy sampling in CI (postcampaign
1846+
# review P1 / lane-4 flakiness audit).
1847+
Random.seed!(20260610)
18441848
opts = EstimationOptions(
18451849
datasize = 21,
18461850
noise_level = 1e-8,

0 commit comments

Comments
 (0)