Skip to content

feat(cal): Tier-1 validator ports from SubmodelTarget - #77

Merged
jeliason merged 3 commits into
mainfrom
feature/cal-target-validator-ports
Jul 27, 2026
Merged

feat(cal): Tier-1 validator ports from SubmodelTarget#77
jeliason merged 3 commits into
mainfrom
feature/cal-target-validator-ports

Conversation

@jeliason

Copy link
Copy Markdown
Member

Six validators the submodel schema has enforced for a while, brought over to CalibrationTarget. Each was measured against the 52 live pdac calibration targets before it was written, so the migration cost is known rather than assumed — and that measurement changed the design of two of them.

Validator Live hits Severity
source relevance walks secondary sources too 1 warn
context mismatch must be justified 0 raise
centre channel must not be the population range 26 raise
no hardcoded non-integer floats in distribution_code 5 raise
snippet must appear in the source PDF 0 (context-gated) raise
assumed_* / uncertainty inputs (earlier commit) 1 raise

The centre-channel check found a real defect

An across_patient target uses two channels: samples carries the population spread that hierarchical inference reads as omega, while median + ci95 pin the centre and must shrink with n. 26 of the 49 population targets return the population's own 2.5/97.5 percentiles as ci95. The check does not infer this — it compares the two and finds them identical:

tam_density  n=113   ci95     [18.64, 698.9]
                     samples  [18.64, 698.9]   <- 2.5/97.5 percentiles

The same width then goes into omega and into the flat likelihood as measurement noise, so the target is weighted as though one simulated patient could land anywhere in the cohort.

The trap is population.summarize(), whose ci95 is np.percentile(samples, 2.5/97.5) by construction. It is the obvious helper to call and it silently produces a population-scale centre channel. Fixed at the source: summarize() gains n= / rng= / n_boot=, and with n it subsample-bootstraps the median (shape-agnostic, which matters for the skewed lognormal marginals the density targets use). Verified on tam_density — the centre narrows 8.7x while samples and median_obs do not move at all. 8.7x rather than sqrt(113)=10.6x is correct: the SE of a median is ~1.25 sd/sqrt(n).

Without the helper change the error message had no actionable fix for 24 of the 26 (bootstrap_median needs per-patient values they do not have), and the only other escape — population_spread: center_only — would have deleted the omega channel. The message now leads with summarize(n=...) and flags what the center_only route destroys.

Why the hardcoded-value ban is scoped

The submodel allowlist ({0,1,2,1.96,1.645}) fails 50 of 50 live targets, because distribution_code is Monte-Carlo statistics: seeds (42 x49), draw counts (10000 x22), percentile arguments, IQR->SD factors (1.349). A ban that flags every target gets switched off. Scoped to non-integer floats it flags 5, all real — a paper's 2–21% range read straight into np.log(), and a detection floor plus tolerance band belonging in assumptions. Integer-valued measurements are a known gap, documented on the validator.

Downstream

31 of the 52 live pdac targets will not load until they are migrated (26 need n= threaded through plus a recomputed ci95; 5 need their literals declared). Mechanical, but it moves x_obs for half the corpus, so it wants one batch plus a fresh flat run. The pdac-side maple pin should not move until then.

Two existing fixtures were themselves examples of what is now rejected — the population-sample fixture returned population percentiles as ci95, and the clipping fixture wrote its floor inline. Both updated.

408 passed, 6 skipped.

jeliason added 3 commits July 26, 2026 21:19
Ports SubmodelTarget.validate_no_assumed_or_uncertainty_inputs onto
CalibrationTargetEstimates. Submodel targets have refused fabricated spread
for a while; calibration targets never were, which is how a target reached
the corpus carrying:

  assumed_cv_fraction = 1.0
  'Neither standard deviation nor interquartile range was reported for any
   immune cell density measurement; CV assumed at 1.0 as a conservative
   estimate.'

A fabricated CV is not conservative -- it sets the weight the target pulls
with in the likelihood.

Two rules. 'assumed' anywhere in an input name is rejected regardless of
input_type. Uncertainty-smelling names (cv, sigma, uncertainty,
fold_uncertainty, translation_sd) are rejected on non-measurement types
(experimental_condition, inferred_estimate, derived_arithmetic) -- dispersion
a paper actually reports is a measurement and belongs on direct_parameter /
proxy_measurement with a snippet.

Reported spread stays legal: sd_*, sem_*, *_q3 and friends are untouched,
and a genuinely author-reported CV is fine when typed as a measurement.

Measured against the pdac corpus before writing: 449 inputs across the live
calibration targets, one violation (treg_fraction_baseline), zero false
positives.

Also hoists a late module-level import in the test file to satisfy ruff E402.
Five validators the submodel schema has enforced for a while, brought over
to CalibrationTarget. Each was measured against the 52 live pdac targets
before landing, so the migration cost is known rather than assumed.

- source relevance now walks primary AND secondary sources. Previously only
  the primary was inspected, so a secondary could be a wiki, a preprint or
  an unreviewed database and never be reported. 1 live hit (a UniProt
  molecular-weight lookup). Stays a warning; the message says whether the
  source supplies values used in the derivation.

- validate_context_mismatch_justified: a declared pharmacological/genetic
  perturbation must carry perturbation_relevance, and tme_compatibility=low
  must carry notes. Both raise, matching the submodel side, and both cover
  secondary sources. Warnings do not survive a batch load. 0 live hits, so
  the escalation is free.

- centre-channel check inside validate_derivation_code: an across_patient
  target must not return the population's own 2.5/97.5 percentiles as ci95.
  That encodes the spread twice — omega reads it from `samples`, and flat
  inference reads the same width as measurement noise, so the target is
  weighted as though one simulated patient could land anywhere in the
  cohort. 26 live hits, all via population.summarize() with no n.

  population.summarize() gains n=/rng=/n_boot=. With n it subsample-
  bootstraps the median, so ci95 shrinks with n while `samples` passes
  through untouched. Shape-agnostic, which matters for the skewed lognormal
  marginals the density targets use. Bare summarize() is still correct for
  a center_only target. Without this the error message had no actionable
  fix for 24 of the 26 (bootstrap_median needs per-patient values they do
  not have), and the only way to clear it would have deleted the omega
  channel.

- hardcoded values in distribution_code, scoped to non-integer floats. The
  submodel allowlist ({0,1,2,1.96,1.645}) fails 50 of 50 targets because
  distribution_code is Monte-Carlo statistics — seeds, draw counts,
  percentile arguments, IQR->SD factors. A ban that flags everything gets
  switched off. Scoped to non-integer floats it flags 5, all real: a
  fraction range read out of a paper into np.log(), and a detection floor
  plus tolerance band belonging in assumptions. Integer-valued measurements
  are a known gap, documented on the validator.

- validate_snippets_against_pdfs, context-guarded on papers_dir.
  validate_input_values_in_snippets checks the value against the snippet,
  but both are LLM-written and can be jointly invented; this checks the
  snippet against the paper. Zero cost until the extraction pipeline passes
  papers_dir.

Two existing fixtures were themselves examples of what is now rejected: the
population-sample fixture returned population percentiles as ci95, and the
clipping fixture wrote its floor inline. Both updated.

408 passed.
CI's ruff runs a wider rule set than pre-commit's (634 errors repo-wide, red
on main for at least three merges). This branch added 7 on top; clearing them
keeps the touched files at main's baseline of 51.

One was a real leftover, not style: summarize() had
`flat = mag if mag.ndim == 1 else mag`, a no-op branch from an earlier draft
that treated the 1-D and 2-D cases separately. Patients index axis 0 either
way, so the indexing collapses to one path.
@jeliason
jeliason merged commit 3433fbc into main Jul 27, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant