feat(cal): Tier-1 validator ports from SubmodelTarget - #77
Merged
Conversation
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.
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
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.
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.distribution_codeassumed_*/ uncertainty inputs (earlier commit)The centre-channel check found a real defect
An
across_patienttarget uses two channels:samplescarries the population spread that hierarchical inference reads as omega, whilemedian+ci95pin the centre and must shrink with n. 26 of the 49 population targets return the population's own 2.5/97.5 percentiles asci95. The check does not infer this — it compares the two and finds them identical: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(), whoseci95isnp.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()gainsn=/rng=/n_boot=, and withnit subsample-bootstraps the median (shape-agnostic, which matters for the skewed lognormal marginals the density targets use). Verified ontam_density— the centre narrows 8.7x whilesamplesandmedian_obsdo 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_medianneeds 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 withsummarize(n=...)and flags what thecenter_onlyroute destroys.Why the hardcoded-value ban is scoped
The submodel allowlist (
{0,1,2,1.96,1.645}) fails 50 of 50 live targets, becausedistribution_codeis Monte-Carlo statistics: seeds (42x49), draw counts (10000x22), 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 intonp.log(), and a detection floor plus tolerance band belonging inassumptions. 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 recomputedci95; 5 need their literals declared). Mechanical, but it movesx_obsfor 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.