Fix static_inclusion_mode=INCLUDE against a cohort with no statics (fixes #16) - #18
Closed
florian6973 wants to merge 2 commits into
Closed
Fix static_inclusion_mode=INCLUDE against a cohort with no statics (fixes #16)#18florian6973 wants to merge 2 commits into
florian6973 wants to merge 2 commits into
Conversation
`build_signal_dataset` emitted nothing but timestamped events, so the cohort it
produced had **no static measurements at all**. meds-torch-data cannot collate that
under `static_inclusion_mode: INCLUDE`:
File ".../meds_torchdata/pytorch_dataset.py", line 1822, in collate
static_data = JointNestedRaggedTensorDict(
ValueError: Cannot infer dtype from empty values; provide an explicit `schema=`.
The model is never called, so the traceback names the collator rather than the fixture
that caused it. The consequence is that the flagship learnability tier could not
exercise *any* model that reads baseline data — it failed before reaching it — and the
fast tier could not see the problem at all, because the meds-testing-helpers fixture
does have statics.
Found porting TECO, which concatenates age, sex, ethnicity and race onto every
15-minute interval. Dropping them to get a green suite is what
docs/PORTING-A-MODEL.md step 4 exists to forbid, so the fixture is what has to change.
Both builders now give every subject one value-less code (`BASELINE//GROUP_{A,B}`) and
one numeric one (`BASELINE//AGE`), covering both branches of a model's static handling.
`with_statics=False` reproduces the old cohort.
The assignment comes from a generator seeded independently of the one that decides the
labels. That is load-bearing rather than tidy: a static correlated with the outcome
would be a second signal, which would let a model score well without ever reading
SIGNAL_CODE and would give the negative control something real to learn — the vacuous
pass the control exists to rule out.
`template/tests/test_synthetic_statics.py` is the regression, in the payload rather
than here because the template's own suite installs no torch and so cannot collate
anything. Four checks: every subject has both kinds of static; the codes reach
`metadata/codes.parquet`; neither group is perfectly separable by label; and a real
tensorized batch collates under INCLUDE with populated static tensors. Unmarked, so it
runs in the fast tier — where this should have been caught.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit stopped the *fixture* tripping the collate crash. It did nothing
for the crash itself: a user whose MEDS dataset genuinely has no baseline variables,
running a model whose datamodule sets `static_inclusion_mode: INCLUDE`, still gets
ValueError: Cannot infer dtype from empty values; provide an explicit `schema=`
from inside the dataloader — naming neither the config key that asked for static data
nor the cohort that lacks it, and arriving once training has already started.
The mismatch is knowable before any work happens: the tokenized cohort records each
subject's statics in `tokenization/schemas/*/*.parquet` as a `static_code` list, so
"no subject has any static measurement" is one scan of one column.
`require_statics_if_requested` does that scan and raises with the cause and both ways
out — set OMIT, or check that preprocessing preserved the null-time rows. It runs in
`build_datamodule`, which is the single place every command constructs a datamodule,
so no command can skip it. This is the same shape as `_require_split_sharded`: a
precondition that is cheap to check and expensive to discover later.
A cohort with no statics is legitimate MEDS, so this is a mismatch between the cohort
and the request rather than a broken dataset, and the message says so. Whether
meds-torch-data should also collate an empty static set is a separate question,
upstream of here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #16.
Two commits, and the split matters — the first alone would have been a fixture change wearing a fix's clothes.
1. The guard (
c1e5613) — the actual fixA model whose datamodule sets
static_inclusion_mode: INCLUDE, run against a MEDS dataset with no baseline variables, dies inside the dataloader with:meds-torch-data builds its static tensors from whatever the cohort holds; when that is nothing at all,
JointNestedRaggedTensorDicthas no values to infer a dtype from. The error names neither the config key that asked for static data nor the cohort that lacks it, and it arrives once training has started.The mismatch is knowable before any work happens. The tokenized cohort already records each subject's statics in
tokenization/schemas/*/*.parquetas astatic_codelist, so "no subject has any static measurement" is one scan of one column.require_statics_if_requesteddoes that scan and raises with the cause and both ways out; it runs insidebuild_datamodule, the single place every command constructs a datamodule, so no command can skip it.Same shape as
_require_split_sharded: cheap to check, expensive to discover later.A cohort with no statics is legitimate MEDS, so this is a mismatch between cohort and request, not a broken dataset. Whether meds-torch-data should additionally collate an empty static set is a separate question, upstream of here — this makes the failure actionable, it does not make the case work.
2. The fixture (
009f9ba) — why this was invisiblebuild_signal_datasetemitted nothing but timestamped events, so the cohort it produced had no static measurements at all. That is why the learnability tier could not exercise any model that reads baseline data: it failed before reaching it. The fast tier could not see the problem either, because themeds_testing_helpersfixture does have statics.Both builders now give every subject one value-less code (
BASELINE//GROUP_{A,B}) and one numeric one (BASELINE//AGE).with_statics=Falsereproduces the old cohort — which is what the guard's test uses.The independent RNG is load-bearing, not tidiness. The statics come from
random.Random(seed + 9973), separate from the generator that decides the labels. A static correlated with the outcome would be a second signal: a model could score well on the designed-signal test without ever readingSIGNAL_CODE, and the negative control would have something real to learn — the vacuous pass the control exists to rule out.Tests
template/tests/test_synthetic_statics.py, in the payload rather than in the template's own suite — the template's tests install no torch, so they cannot collate anything, which is exactly why this was invisible here. Unmarked, so it runs in the fast tier. Five checks:metadata/codes.parquet, so they land in the vocabulary;build_datamodule, andOMITstill works — the guard;INCLUDEwith populated static tensors — the crash itself.Verification
ruff check/ruff format --checkclean, payload included.copier updatewill do: fast tier 76 passed, 0 skipped, lint clean. Its localadd_baseline_variablesworkaround — ~40 lines of exactly commit 2, written because a generated repo cannot fix the fixture from the inside — was deleted, and the learnability tier still passes: designed signal AUROC 1.0000, negative control 0.3158 against a 0.70 ceiling, identical across two runs. (The control moves from the 0.3885 that repo measured with its own workaround: same fixture shape, different draw.)🤖 Generated with Claude Code