From 3480eb4e82ac9967ba8a32c110c6a43ca1fd3bea Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 16 Sep 2026 15:30:11 -0400 Subject: [PATCH] Set the solver defaults to what measured best, and end the drift The code defaults and docker-compose.dev.yml disagreed on four variables. Production sets no solver vars so it ran the code defaults; every benchmark ran through the dev container. specs/002 wrote its winning values into the code on 2026-09-10 and never touched the compose file, which still carried 2026-07-16 values. The tests pin the code defaults, so the suite FAILED in the project's own dev container -- and a failing top-level testset aborts the file, so 141 of 151 assertions never ran. Re-measured on 23,022 wide-curator cases, one catalog build, one variable at a time, each arm's env and catalog mount verified and its raw values checked to differ from the baseline: DS_ASSEMBLY_LIMITING 1 -> 0 -196 DS_AND_MODE hill_log -> hill_sat -90 DS_INHIBITOR_EPS 1e-3 -> 1e-12 +11 Assembly-limiting is the driver, restoring the 2026-07-16 result. Adam's hypothesis that the epsilon drove the gap does not hold: its whole range is worth 11-14 cases against a 248-case gap. Epsilon swept separately. 1e-6 and 1e-12 are BIT-IDENTICAL (same 269 changes, same +109/-98), so at or below 1e-6 it is a pure divide-by-zero guard -- confirming specs/006 FR-002. 1e-4 scores 3 better in 23,022 and is NOT taken: it still changes results, so it is still a model parameter, and picking it optimises a guard against the evaluation set. Defaults now: hill_log, assembly on, eps 1e-12, hill_sat_eps 1e-5 (inert). Compose corrected to match, so the suite passes in the dev container and production computes what the benchmark measures. Recorded honestly: specs/002 R5 was not wrong that hill_sat implements the stated AND intent -- 10x10 reads 99.98 against hill_log's 74.06. It just scores 90 worse. Why the compression helps is unexplained, so this is an empirical default and test_and_curves names its testset accordingly rather than claiming design intent. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 19 ++- bench/analysis/cyclic_error_direction.py | 21 ++++ docker-compose.dev.yml | 4 +- specs/009-solver-defaults/research.md | 140 +++++++++++++++++++++++ src/core/reaction_model.jl | 4 +- test/test_and_curves.jl | 27 ++++- test/test_config_validation.jl | 19 ++- 7 files changed, 221 insertions(+), 13 deletions(-) create mode 100644 specs/009-solver-defaults/research.md diff --git a/CLAUDE.md b/CLAUDE.md index 11c4220..6e81979 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -182,10 +182,21 @@ The reaction model is driven by `DS_*` environment variables, resolved once per solve into a `ReactionEvalConfig` (see `resolve_reaction_eval_config` in `reaction_model.jl`). The **code defaults are the validated winning config** — env vars only override for benchmark sweeps: -- `DS_INHIBITION_MODE=divide`, `DS_AND_MODE=hill_sat`, `DS_OR_MODE=mean`, - `DS_ASSEMBLY_LIMITING=0`, `DS_HILL_SAT_EPS=1e-5`, `DS_HILL_LOG_ZMAX=10.0` - Behaviour is pinned by `test/test_and_curves.jl`; the rationale and the - A/B that set them is `specs/002-upregulation-propagation/`. +- `DS_INHIBITION_MODE=divide`, `DS_AND_MODE=hill_log`, `DS_OR_MODE=mean`, + `DS_ASSEMBLY_LIMITING=1`, `DS_INHIBITOR_EPS=1e-12`, `DS_HILL_SAT_EPS=1e-5`, + `DS_HILL_LOG_ZMAX=10.0`. Behaviour is pinned by `test/test_and_curves.jl`. + Re-measured 2026-09-16 on 23,022 wide-curator cases, one variable at a time: + `specs/009-solver-defaults/research.md`. That supersedes the `hill_sat` / + `assembly_limiting=0` choice in `specs/002`, which was made on 564 + experimental cases and cost 286 on the curator set. + **`docker-compose.dev.yml` must be kept in step with these.** It drifted for + six days — production runs the code defaults, every benchmark runs the dev + container, and the tests pin the code defaults, so the divergence made the + suite fail in the project's own container while measuring a config production + never ran. +- `DS_AND_MODE=hill_log` is an **empirical** default, not a principled one. + `hill_sat` implements the stated AND intent more faithfully (10x10 = 99.98 vs + 74.06) and still scores 90 cases worse. Why compression helps is unexplained. - SCC-condensation solve is on by default (`DS_SCC_SOLVE=1`); the legacy flat iteration and the `"fixed_point"` `SteadyStateParams.method` are not used by the CLI or API (both use the penalty/SCC path). diff --git a/bench/analysis/cyclic_error_direction.py b/bench/analysis/cyclic_error_direction.py index 211c451..3a33605 100644 --- a/bench/analysis/cyclic_error_direction.py +++ b/bench/analysis/cyclic_error_direction.py @@ -169,6 +169,27 @@ def main() -> int: print(f"\n{len(diffs)} pathways compared. median difference {med:+.3f}; " f"cyclic worse in {sum(1 for d in diffs if d > 0)}, " f"better in {sum(1 for d in diffs if d < 0)}") + # COVERAGE. The control needs >=20 of BOTH kinds in a pathway, which + # excludes pathways where cyclic readouts dominate -- exactly where a + # real loop effect would hide. State what fraction of the cyclic + # population the control actually saw, so the conclusion is not read + # more widely than the evidence. + compared = {pw for pw in {k[0] for k in per_pw} + if per_pw[(pw, "cyclic")][0] >= 20 + and per_pw[(pw, "acyclic")][0] >= 20} + seen = sum(per_pw[(pw, "cyclic")][0] for pw in compared) + allc = sum(v[0] for k, v in per_pw.items() if k[1] == "cyclic") + pw_with_cyclic = {k[0] for k, v in per_pw.items() + if k[1] == "cyclic" and v[0] > 0} + print(f"COVERAGE: the control saw {seen} of {allc} cyclic NORMAL cases " + f"({seen/allc:.1%}), in {len(compared)} of {len(pw_with_cyclic)} " + f"pathways that have any.") + excl = sorted(((per_pw[(pw,'cyclic')][0], pw) for pw in pw_with_cyclic + if pw not in compared), reverse=True)[:5] + if excl: + print("largest EXCLUDED pathways (cyclic NORMAL n, acyclic n):") + for n, pw in excl: + print(f" {n:>5} {per_pw[(pw,'acyclic')][0]:>5} {pw[:52]}") else: print("\nNO pathway has enough of both -- cyclic and acyclic readouts do " "not co-occur, so the raw comparison is BETWEEN pathways only.") diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 1e37d19..bd40306 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -30,7 +30,7 @@ services: - DS_INHIBITION_MODE=${DS_INHIBITION_MODE:-divide} # Smoothing constant ε for divide-form inhibition; non-load-bearing in # [0.0005, 0.01]. - - DS_INHIBITOR_EPS=${DS_INHIBITOR_EPS:-0.001} + - DS_INHIBITOR_EPS=${DS_INHIBITOR_EPS:-1e-12} # Floor for divide-form H. 0 = full repression possible; 0.95 = each # inhibitor edge can only repress by 5% (Adam's idea for negative-feedback # loops that otherwise over-repress through compounding). @@ -45,7 +45,7 @@ services: # hill_sat boundary-transition width (Hjelmfelt smoothing). Identity # outside ~ε of the UI=0 and UI=100 boundaries; small default keeps the # operating range (UI 0.25–10) effectively exact-multiplicative. - - DS_HILL_SAT_EPS=${DS_HILL_SAT_EPS:-0.001} + - DS_HILL_SAT_EPS=${DS_HILL_SAT_EPS:-1e-5} # hill_sat saturation sharpness — legacy soft-min exponent, unused by # the current Hjelmfelt formulation but kept for backward compat. - DS_HILL_SAT_K=${DS_HILL_SAT_K:-8.0} diff --git a/specs/009-solver-defaults/research.md b/specs/009-solver-defaults/research.md new file mode 100644 index 0000000..c1fab4e --- /dev/null +++ b/specs/009-solver-defaults/research.md @@ -0,0 +1,140 @@ +# Solver defaults, re-measured on the wide curator set + +**Created**: 2026-09-16 +**Status**: Landed +**Supersedes**: the default choices in `specs/002-upregulation-propagation` R5 + +## Why this exists + +The code defaults and `docker-compose.dev.yml` disagreed on four variables. +Production (`docker-compose.prod.yml`) sets no solver variables, so it ran the +code defaults; every benchmark ran through the dev container. `specs/002` +(2026-09-10, `17bc6be`) wrote its winning values into the code and never touched +the compose file, which still carried values from 2026-07-16 (`c9805b2`). + +The tests pinned the code defaults, so `test_config_validation.jl` **failed +inside the project's own dev container** — and because a failing top-level +`@testset` aborts the file, 141 of its 151 assertions silently never ran. + +## Method + +Wide curator set, Release97, one shared catalog build (`cat_fresh`), paired and +conditioned. Each arm changes ONE variable from the config that won the +head-to-head, and is compared against it. Every arm verified before use: +container env dumped, catalog mount asserted, and raw `pred_ui` checked to +differ from the baseline — a config change that produces bit-identical output +has not applied. + +Two earlier attempts at this comparison were discarded, for the record: + +1. A broken health-check (`read -t 2 < /dev/zero` returns instantly) let the + benchmark start against a booting server. +2. `--force-recreate` drops `PATHWAY_CATALOG`, silently swapping the catalog the + server serves. Two builds of one pathway share NO uuids, so every observation + matched nothing and every readout reported baseline — a complete 23,908-case + run with ONE distinct predicted value, scored and reported as meaningful. + +Both now have guards: PR #27 (benchmark aborts on catalog mismatch) and PR #29 +(the API reports or rejects observations naming absent nodes). + +## Head-to-head + +| config | scored | correct | accuracy | macro-F1 | +|---|---|---|---|---| +| code defaults (`hill_sat`, assembly off, eps 1e-12) | 22,902 | 18,931 | 0.8266 | 0.7939 | +| dev compose (`hill_log`, assembly on, eps 1e-3) | 22,902 | 19,179 | 0.8374 | 0.8031 | + +768 predictions changed, net **+248** for the dev-compose config. Raw values +differed on 57.3% of cases, so the arms genuinely differ. + +## Attribution — one variable at a time + +From the dev-compose config, moving each variable toward the code defaults: + +| variable moved | changed | net | +|---|---|---| +| `DS_ASSEMBLY_LIMITING` 1 → 0 | 613 | **−196** | +| `DS_AND_MODE` `hill_log` → `hill_sat` | 171 | **−90** | +| `DS_INHIBITOR_EPS` 1e-3 → 1e-12 | 269 | **+11** | + +Sum −275 against −248 measured jointly; they do not compose additively. + +**Assembly-limiting is the driver.** This restores the result already on file +from 2026-07-16, where assembly-limiting was the first real curator win. + +## The epsilon sweep + +All at the winning config, paired against eps 1e-3: + +| `DS_INHIBITOR_EPS` | correct | accuracy | macro-F1 | net | +|---|---|---|---|---| +| 1e-3 | 19,278 | 0.8374 | 0.8035 | — | +| 1e-4 | 19,292 | 0.8380 | 0.8045 | +14 | +| 1e-6 | 19,289 | 0.8379 | 0.8041 | +11 | +| 1e-12 | 19,289 | 0.8379 | 0.8041 | +11 | + +**1e-6 and 1e-12 are bit-identical** — same 269 changes, same +109/−98. At or +below 1e-6 the epsilon has saturated into a pure divide-by-zero guard, which is +exactly what `specs/006` FR-002 required and nobody had confirmed. + +1e-4 scores 3 better in 23,022. It is **not** taken: at 1e-4 the value still +changes results, so it is still acting as a model parameter, and choosing it +optimises a guard constant against the evaluation set. + +## The experimental ground truth agrees + +`specs/002` chose on the experimental set, so overturning it on the curator set +alone would be picking the benchmark that agrees. Measured directly, same +catalog, epsilon held at 1e-12 in both arms: + +| defaults | scored | correct | accuracy | macro-F1 | +|---|---|---|---|---| +| old (`hill_sat`, assembly off) | 846 | 597 | 0.7057 | 0.6116 | +| new (`hill_log`, assembly on) | 846 | **616** | **0.7281** | **0.6521** | + +Net **+19** (+26/−7); per pathway −4 TP53, −1 WNT, +1 ERBB2. `propagator_missed` +falls from 89 to 84. + +So the change is not a ground-truth artifact: it wins on both. That does not +retroactively make `specs/002` wrong on its own evidence — it ran a different +comparison on 564 cases against a different baseline and a catalog that no +longer exists (its own research.md records the crash). It does mean the choice +does not survive re-measurement on either set today. + +## Decision + + DS_AND_MODE = hill_log (was hill_sat) + DS_ASSEMBLY_LIMITING = true (was false) + DS_INHIBITOR_EPS = 1e-12 (unchanged; compose corrected from 1e-3) + DS_HILL_SAT_EPS = 1e-5 (unchanged; inert under hill_log) + +Code and compose now agree, so the test suite passes in the dev container and +production computes what the benchmark measures. + +## The unresolved tension + +`specs/002` R5 was not wrong about the curve. `hill_sat` really does implement +the stated AND intent — 10x10 reads 99.98 against `hill_log`'s 74.06, and a lone +node at UI 50 reads 50.00 against 41.43. `hill_log` compresses throughout the +operating range, which by that argument is a defect. + +It also scores 90 cases better on 23,022 curator cases. + +So the compression is doing useful work that "correct" multiplication does not, +and **nobody has explained why**. Candidate: compression damps the multi-hop +amplification that drives false-change calls, which is the dominant error mode +(see `project_over_coupling_is_the_target`, and the reach/false-change +table in `specs/008-cycle-handling`). That is a hypothesis, not a finding. + +Until it is explained this is an **empirical default, not a principled one**. +`test/test_and_curves.jl` names the testset accordingly rather than claiming the +defaults implement the design intent. + +## What this says about the 564-case set + +`specs/002` chose `hill_sat` and assembly-off on +31 over 564 experimental +cases. On 23,022 curator cases those choices cost 286 between them. Both +measurements are real; they answer different questions against different ground +truths. FR-008 exists because the small set has now reversed at scale +repeatedly, and this is the clearest instance yet — it silently set the +repository's defaults for six days. diff --git a/src/core/reaction_model.jl b/src/core/reaction_model.jl index 33af185..43c7091 100644 --- a/src/core/reaction_model.jl +++ b/src/core/reaction_model.jl @@ -818,7 +818,7 @@ function resolve_reaction_eval_config()::ReactionEvalConfig # reads 41.4 and at UI 100 reads 74.1. It is identity at the 0.85/1.15 # classification cutoffs, which is why every previous check passed it. # hill_sat saturates only at the 0 and 100 boundaries, as specified. - _mode_env("DS_AND_MODE", "hill_sat"), + _mode_env("DS_AND_MODE", "hill_log"), _mode_env("DS_OR_MODE", "mean"), # OFF. The limiting-reactant rule aggregated a complex's subunits with # min, and min(elevated, baseline) is EXACTLY baseline — so a complex @@ -830,7 +830,7 @@ function resolve_reaction_eval_config()::ReactionEvalConfig # were pinned across a median of 17 nodes, some adjacent to the # readout — conditions where an increase barely had to cross a complex # and the clamp therefore cost almost nothing. - _bool_env("DS_ASSEMBLY_LIMITING", false), + _bool_env("DS_ASSEMBLY_LIMITING", true), # 1e-5, not 1e-3. The epsilon smooths the saturation corners, but at # 1e-3 it EXCEEDS the internal values where knockouts live (~0.0006) # and acts as a floor on the whole network: 0.25x0.25 read 0.09 diff --git a/test/test_and_curves.jl b/test/test_and_curves.jl index 2b9d740..1f33852 100644 --- a/test/test_and_curves.jl +++ b/test/test_and_curves.jl @@ -104,14 +104,35 @@ end @test and_fold([0.0, 1.0]; mode="hill_sat", eps="1e-5") < 0.01 end -@testset "defaults implement the design intent" begin +@testset "defaults are what measured best, which is NOT the design intent" begin + # An honest name, because these two disagree and the tension is real. + # + # specs/002 R5 argued `hill_sat` implements Adam's stated AND intent -- + # "if it is near 1 I want it to be extremely close to pure multiplication" + # -- and it does: 10x10 reads 99.98 under hill_sat against 74.06 under + # hill_log, and a lone node at UI 50 reads 50.00 against 41.43. By that + # argument hill_log is wrong, and the testsets above still pin those + # curve shapes because the argument has not been withdrawn. + # + # But specs/002 chose on +31 over 564 experimental cases. Re-measured + # 2026-09-16 on 23,022 wide-curator cases, one catalog build, one variable + # at a time: `hill_sat` costs -90 and `assembly_limiting=false` costs -196. + # FR-008 makes the wide set the decision basis, so the defaults follow the + # measurement and this test records that they are not the design intent. + # + # What that means: the compression hill_log applies inside the operating + # range is apparently doing useful work that "correct" multiplication does + # not. Nobody has explained why. Until someone does, this is an empirical + # default, not a principled one -- see specs/009-solver-defaults. for name in ("DS_AND_MODE", "DS_HILL_SAT_EPS", "DS_ASSEMBLY_LIMITING") haskey(ENV, name) && delete!(ENV, name) end config = resolve_reaction_eval_config() - @test config.and_mode == "hill_sat" + @test config.and_mode == "hill_log" + @test config.assembly_limiting == true + # Inert while and_mode is hill_log; kept correctly sized so switching the + # AND mode cannot silently restore a 10%-of-baseline epsilon. @test config.hill_sat_eps ≈ 1e-5 - @test config.assembly_limiting == false end diff --git a/test/test_config_validation.jl b/test/test_config_validation.jl index 83adab2..20004f6 100644 --- a/test/test_config_validation.jl +++ b/test/test_config_validation.jl @@ -146,14 +146,29 @@ end # hill_sat / clamp-off / eps 1e-5 as of specs/002-upregulation-propagation. # test/test_and_curves.jl covers why; this only pins that the defaults are # what that feature measured. - @test config.and_mode == "hill_sat" + # Re-measured 2026-09-16 on 23,022 wide-curator cases at Release97, one + # catalog build, each arm changing ONE variable and paired against the + # others. specs/002 set `hill_sat` and `assembly_limiting=false` on +31 + # over 564 experimental cases; on the curator set those same two choices + # cost -90 and -196 respectively. The wide set decides (FR-008), so they + # are reverted here. Attribution and numbers: specs/009-solver-defaults. + @test config.and_mode == "hill_log" @test config.or_mode == "mean" - @test config.assembly_limiting == false + @test config.assembly_limiting == true + # Only consulted when and_mode is hill_sat, so inert at the current + # default. Kept sized correctly so switching AND mode does not also + # silently re-introduce a 10%-of-baseline epsilon. @test config.hill_sat_eps == 1e-5 # A divide-by-zero guard, and nothing else. Baseline is 0.01, so this # must stay orders of magnitude below it — at the old 1e-3 it was 10% of # baseline and silently set the de-repression ceiling, compressed the # response curve and shifted maximum suppression. + # Swept 2026-09-16: 1e-6 and 1e-12 give BIT-IDENTICAL predictions (same + # 269 changes, same +109/-98), so at or below 1e-6 this is a pure + # divide-by-zero guard -- exactly what specs/006 FR-002 required and + # nobody had confirmed. 1e-4 scores 3 cases better in 23,022 but still + # changes results, i.e. still acts as a model parameter; taking it would + # be tuning a guard against the evaluation set. @test config.inhibitor_eps == 1e-12 @test config.inhibitor_eps < 0.01 / 1000 @test config.or_combine == "max"