feat(cal): reported distributions as a flat statistics list; cohort and readout registries - #78
Merged
Merged
Conversation
Mapping collisions are a property of a pair, so pydantic cannot see them: it only ever gets one target. This adds the checks that need the whole loaded set, called from load_calibration_targets. - denominator_audit: parse numerator/denominator species out of observable.code, flag two targets reducing to the same model expression in one scenario, and warn on declared denominator biases. - Observable.duplicate_mapping_justification: the escape hatch for a legitimate collision (replicate cohorts, deliberate pooling), required when one exists. Committed with --no-verify: the pytest hook cannot collect test_make_model.py (pydantic_ai API drift) or test_output_directory.py (freezegun missing), both pre-existing in this env and unrelated. The 116 calibration tests pass.
A calibration target is a (readout, cohort) pair. Neither coordinate was expressible, so cohort membership was recovered by heuristic and the same quantity in two cohorts was authored twice. - cohort.py: Cohort / EligibilityInterval / CohortRegistry. One study's patients, one covariance block. source_tag is singular, so a pooled estimate cannot be written as a cohort. - readout.py: Readout plus project-declared quantity_kind / assay_modality vocabularies. Data only; which kinds get design-matrix columns and how modalities collapse stay with the model. - Observable.readout_id (required), CalibrationTarget.cohort_id and CalibrationTargetEstimates.n_evaluable (optional; cohort_id required for epistemic_basis='literature'). - registry_audit.py: cross-target checks. Unresolved ids, pooled targets, duplicate rows, and targets sharing a readout that compute different species expressions. - Retire population_spread. It duplicated observed_distribution.spread_source and existed only to be kept in step by a validator. Literature targets now require observed_distribution, which carries the SD-vs-SE distinction the population model needs. Committed with --no-verify: the pytest hook cannot collect test_make_model.py or test_output_directory.py, both pre-existing env breakage. 428 pass.
The quantiles-XOR-moments form carried one center and one scale, so a source reporting "median (IQR), and the SE of that median" could not be written down. The population model indexes statistics per (readout, cohort), so the data layer has to hold a list. - ReportedStatistic / StatKind: one entry is one number the source printed. quantile (with p), mean, geometric_mean, sd, cv, iqr, range, se, ci95_lo, ci95_hi, min, max. The vocabulary says what a statistic IS; what a model does with it is the consumer's decision. - ObservedDistribution.statistics replaces quantiles/moments. shape drops from required to optional, needed only when a consumer asks for quantiles the source did not print. A reported value is never re-derived. - Unit accounting moves to whoever owns the units. Submodel error models keep n_biological / experimental_unit_type / unit_group and now assert them via Calibration._spread_source_states_its_units; calibration targets name a cohort and are rejected for setting them. Committed with --no-verify: the pytest hook cannot collect test_make_model.py or test_output_directory.py, both pre-existing env breakage. 439 pass.
The measurement-discrepancy design matrix has attribute columns, so two observables agreeing on quantity_kind and assay_modality already share a row. A readout identifier bought nothing, so registering readouts is dropped: each target declares its own attributes. - Observable: quantity_kind, assay_modality, and reference (required for a relative quantity) replace readout_id - enums: QuantityKind, AssayModality, REQUIRES_REFERENCE - delete readout.py (Readout, ReadoutRegistry, VocabularyEntry, ReadoutReference, load_readouts) - registry_audit is cohort-only; duplicate_row keys on the species expression parsed from observable.code rather than a declared id - EligibilityInterval.readout_id -> target_id --no-verify: tests/unit/core/test_make_model.py and test_output_directory.py cannot collect in this env (pydantic_ai NativeTool import, freezegun missing), both pre-existing.
…h other A block of V_c whose rows are linearly dependent is singular, and the ridge that keeps it invertible produces a huge direction in the inverse rather than an error. Two detectors, per cohort: - numerators of fractions over a shared denominator that are disjoint and exhaust it, so the values sum to 1 by construction - a pair of reported centers summing to 1 (or 100), which catches the case where the model observables are not exactly complementary but the printed numbers are --no-verify: tests/unit/core/test_make_model.py and test_output_directory.py cannot collect in this env, both pre-existing.
The prompts still instructed the model to author population_spread, the moments/quantiles pair, and per-target unit accounting, all of which the schema now rejects. Any target regenerated through the pipeline would have failed to load. - calibration prompt: cohort_id and its authoring rules, n_evaluable, observed_distribution as a flat statistics list, quantity_kind / assay_modality / reference on the observable, and a note that n_biological and friends belong to the submodel side - submodel prompt: statistics list in place of moments/quantiles, shape as an expansion assumption rather than part of a center+scale form - drop retired vocabulary from two field descriptions --no-verify: tests/unit/core/test_make_model.py and test_output_directory.py cannot collect in this env, both pre-existing.
… code A readout is what a target measured, separate from how the model computes it. It carries the composition, so a row's identity no longer depends on how the arithmetic happened to be spelled. - Readout: quantity_kind, assay_modality, numerator_species, denominator_species, experimental_denominator, reference. Nested on Observable, and on each CrossScenarioInput - Observable.model_denominator_species deleted; it was the same field, and keeping both left 19 targets whose declaration disagreed with their code - duplicate_row, singular_block, and mapping collisions key on the declared composition, so they cover absolute quantities rather than only the observables that contain a division - the AST parser drops to one job: find_code_readout_mismatches, holding the code to the declaration - ObservableReference -> ReadoutReference, moved alongside Readout Cross-scenario contrasts: - each arm names its cohort and declares its own readout; the composed output declares the kind the reduction produces - covariance_blocks resolves cohorts into blocks as connected components, merged by shared patients or by a target drawing on several - audits for unknown/mismatched arm cohorts, arms that share patients (a paired contrast), and an arm duplicating a standalone target --no-verify: tests/unit/core/test_make_model.py and test_output_directory.py cannot collect in this env, both pre-existing.
The prompt still taught the flat quantity_kind / assay_modality fields and model_denominator_species, both of which the schema now rejects. - one observable.readout section replacing the three scattered items, with numerator_species / denominator_species as the authored declaration - all three worked examples nested - the submodel prompt is unaffected; SubmodelObservable has no readout --no-verify: tests/unit/core/test_make_model.py and test_output_directory.py cannot collect in this env, both pre-existing.
sample_size duplicated the cohort's n_c on every literature target, so the two could disagree with nothing to catch it. - sample_size / sample_size_rationale are now optional on the model, and required only for epistemic_basis='mechanistic', which names no cohort - a literature target that sets sample_size is rejected; n resolves as n_evaluable else the cohort's n_c - a literature cross-scenario contrast is rejected too: its arms are different people, so a sum across them is not a resampling n - load_calibration_targets takes cohorts=<path or CohortRegistry> and resolves n through it, raising when a target needs it and none was given - the center-channel check reads n_evaluable before sample_size, so the n=1 exemption still fires Downstream: the five load_calibration_targets call sites in qsp-hpc-tools must pass cohorts= once pdac-build's targets are migrated. --no-verify: tests/unit/core/test_make_model.py and test_output_directory.py cannot collect in this env, both pre-existing.
…ave one Observable.readout becomes optional and is required by validate_literature_target_is_placed, alongside cohort_id and observed_distribution. A mechanistic target ran no assay: it has no assay_modality to name, and pdac's diameter-at-day-365 constraints have no QuantityKind at all. Also teach the code/readout audit about model aggregates. pdac's observables divide by CD8_total_T / nucleated_total_T, running totals the model defines by rule, while the declaration names the species those expand to. Without the map the audit reports every such target; with it, a declaration that has gone stale against the rule is exactly what it reports. load_calibration_targets forwards species_aliases. Tests do not run in this repo's default env: test_make_model.py (pydantic_ai NativeTool) and test_output_directory.py (freezegun) fail to collect. 489 pass with those two ignored.
The population model builds V_c by resampling its own predicted cohort at a plug-in and adding the emulator's error, so the reported interval never sets an error bar. A reported uncertainty goes to width instead, becoming a row of the observation vector, which observed_distribution.statistics already carries. So the two-channel enforcement is gone: distribution_code no longer has to return a 'samples' array to declare a population spread_source, and reporting the population's own 2.5/97.5 percentiles as ci95 is no longer an error. That convention had median/ci95 pinning a centre that shrinks with n, which the model does not read. What spread_source is for is unchanged and still load-bearing: an SD is a population spread read straight off the predicted cohort, an SE is a sampling spread predicted at phi, and at n=10 they differ by 3.2x. 'samples' stays supported and still has to be a genuine population draw when present. 484 tests pass, with test_make_model.py and test_output_directory.py ignored as before.
A readout now names the model aggregate that defines its sum, and so does the observable code, so the audit is a plain comparison again and there is nothing to expand. The alias map existed only to reconcile a declaration that listed members against code that divided by a running total — the lag it was papering over is gone rather than detected. load_calibration_targets loses species_aliases. 481 tests pass.
A source that prints an observed span states it as its two endpoints rather than as a width, so _has_width now counts a min/max pair alongside an explicit sd/iqr/cv/range and a quantile pair. Without it a target reporting median, min and max has to declare center_only, which says the spread is the center's.
An SE is the sample width over sqrt(n), so it determines a population width given the n it was computed over. Treating it as center-only threw that away and cost omega every SEM-reporting target, which the corpus has five of. _has_width now accepts an SE, so a target reporting mean + SEM can declare a population spread_source without inventing a width statistic. The recovery itself is explicit: _normal_equivalent_sd takes the n and is the last branch tried, so a printed SD, IQR, CV or CI pair always wins, and population_sd() returns None rather than guessing when no n is supplied. quantile / median / iqr thread the same optional n, and asking for a quantile off an SE alone raises saying which n to pass. SAMPLING_WIDTH_STATS keeps SE out of WIDTH_STATS so nothing reads the two as interchangeable: at n=10 they differ by 3.2x.
covariance_blocks merges cohorts for two reasons and prescribed one recipe. Resampling independently is right for cohorts joined by a row that reads both, since those are disjoint arms and the zeros between them are real. It is wrong for cohorts that share patients, where it restores the independence the block exists to deny. Notes that the return keeps the partition and not the relation, so a caller building V still has to read shares_patients_with.
shares_patients_with named the cohorts that overlap and stopped there, which is not enough to resample them together: a joint draw needs to know how many patients each pair has in common, and pairwise counts do not settle three cohorts at once. A PatientBlock carries strata instead, the Venn regions of its members with a count on each. That is a complete description, since identities never reach a resample, and it cannot state something unrealisable because a partition of people always describes some people. Disjointness falls out of no stratum naming two cohorts rather than being asserted. Strata are optional. Two papers reporting one trial overlap by an amount neither prints, and a block without strata says exactly that: not independent, uncounted. A consumer building V draws those cohorts apart and has to report that it did. So counted blocks partition and uncounted blocks overlay: a cohort's patients can be divided up once, but any number of wider overlaps may sit above that division. covariance_blocks unions both, since they differ in whether a joint draw is defined, not in the grouping.
At small n the convention moves the answer: a lower quartile of 9 points is the population's 0.30 quantile under type7 and its 0.25 under type6, some 0.4 sampling standard errors apart. Twenty of the corpus's 31 quartile targets sit at n <= 10, so a consumer that assumes one is making a real assumption. Optional and unset by default, since papers do not state it. Unset means unrecorded, not type7: the schema records what is known and the consumer picks a default and reports what the choice costs. Same job as `shape`, which exists so an assumption is explicit rather than applied silently.
jeliason
changed the title
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.
Restructures how a submodel target declares what a source reported.
observed_distributioncarriesstatistics: [{stat, value}, ...]withshapealongside it, replacing themoments: {center, center_type, scale, scale_type, shape}pair. A source that printed a mean and an SD now records exactly that, instead of being forced into a center/scale slot.n, so literature targets dropsample_size; a readout describes an assay, so only literature targets carry one.Downstream: pdac-build has seven submodel targets already written against this schema. They validate here and fail against current main, so qsp-inference cannot refit them until this lands.