Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,27 @@ jobs:
# `logger=csv`. One extra entry turns both on: `test_every_rendered_logger_composes` is then the
# job that would catch `logger=wandb` raising MissingConfigException again.
loggers: [false]
data_backend: [mtd]
include:
- profile: supervised
loggers: true
data_backend: mtd
# The custom-featurization lane: its venv deliberately omits meds-torch-data, so the lane
# itself proves nothing imports it (including `meds-model commands` — the import-guard test).
- profile: supervised
loggers: false
data_backend: custom_featurization
steps:
- uses: actions/checkout@v4
- uses: astral-sh/setup-uv@v6
with:
enable-cache: true
- name: Render the ${{ matrix.profile }} DAG (loggers=${{ matrix.loggers }})
- name: Render the ${{ matrix.profile }} DAG (loggers=${{ matrix.loggers }}, backend=${{ matrix.data_backend }})
run: |
uvx copier copy --vcs-ref=HEAD --defaults --trust \
--data model_slug=demo_model --data model_name="Demo Model" \
--data profile=${{ matrix.profile }} \
--data data_backend=${{ matrix.data_backend }} \
--data use_wandb=${{ matrix.loggers }} --data use_mlflow=${{ matrix.loggers }} \
. /tmp/demo_model
- name: Install torch (CPU) + run the generated repo's tests
Expand Down
22 changes: 18 additions & 4 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,14 @@ cd /tmp/demo && uv venv \
&& uv run pytest -m "not slow" -q -rs
```

Expect ~37 passed and 2 skipped, and both skips are the correct result — everything that does not need a
model runs, including real MTD tensorization and task materialization. One skip is `skip_if_stub`; the
other is `test_unsupported_command_fails_clearly`, whose parameter set is empty for `probe` because that
DAG registers all five commands.
Expect ~75 passed and 1–3 skipped (supervised; `probe` also skips `test_unsupported_command_fails_clearly`,
whose parameter set is empty because that DAG registers all five commands), and every skip is the correct
result — everything that does not need a model runs, including real MTD tensorization, a real predicates
featurization (both workspaces are built, and the equivalence guard compares their label partitioning),
and task materialization. One skip is `skip_if_stub`; with `'data_backend': 'custom_featurization'` in
the `data` dict the repo installs **without meds-torch-data**, the two MTD-only tests skip via
`importorskip`, and the CLI smoke test doubles as the regression test for the import guard in
`lightning/__init__.py`.

Add `'use_wandb': True, 'use_mlflow': True` to that `data` dict to render the optional logger configs —
otherwise `logger=csv` is the only thing the generated suite ever composes.
Expand Down Expand Up @@ -224,6 +228,16 @@ class exists that no profile registers — which is how `MaterializedPredictComm
`commands.py.jinja` builds an `entries` list in Jinja and derives its import block from it, so a profile
can never import a class it does not register (an F401 in the generated repo).

`data_backend` is a second, orthogonal copier axis (`mtd` | `custom_featurization`) — a representation
choice, not a DAG choice. **[`docs/design-featurization.md`](docs/design-featurization.md) is its spec.**
It gates only dependencies (meds-torch-data), rendered configs (`datamodule/*.yaml`,
`preprocess_data.yaml`'s `featurization` default), stubs (`datamodule.py`) and the `model.yaml`
predicates wiring; `src/meds_model_base/` ships both representations unconditionally and tolerates
either dependency set (import guards in `lightning/__init__.py` and `lightning/modules.py` — do not
"clean up" those try/excepts; a custom_featurization repo has no meds_torchdata to import). The model's
`predicates.yaml` is user-owned and is the ONE predicates file: generated tests, MEDS-DEV runs and
production all read it.

`_templates_suffix: .jinja` means **only** `.jinja` files are rendered; everything else under `template/`
is copied byte-for-byte (path segments like `{{ model_slug }}` are still substituted). A file whose
rendered *name* is empty is skipped — that is how `predict.py` is made conditional.
Expand Down
14 changes: 14 additions & 0 deletions copier.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ _skip_if_exists:
- "src/{{ model_slug }}/model.py"
- "src/{{ model_slug }}/commands.py"
- "src/{{ model_slug }}/predict.py"
- "src/{{ model_slug }}/datamodule.py"
- "src/{{ model_slug }}/configs/model/**"
- "src/{{ model_slug }}/configs/paths/**"
- "src/{{ model_slug }}/configs/profile/**"
- "predicates.yaml"

_exclude:
- "*.pyc"
Expand Down Expand Up @@ -113,6 +115,18 @@ profile:
"custom — choose each command yourself": "custom"
default: "supervised"

# The data backend decides how `preprocess_data` materializes patient data — a representation choice,
# orthogonal to the profile (which is a DAG-shape choice). It gates only dependencies, configs and
# stubs; `src/meds_model_base/` ships both paths unconditionally, so flipping the answer later and
# running `copier update` backfills what the new backend needs.
data_backend:
type: str
help: "How preprocess_data materializes patient data"
choices:
"mtd — MEDS-transforms + meds-torch-data tensorization (the standard sequence-model path)": "mtd"
"custom_featurization — 0/1 predicate columns on MEDS parquet; you write the datamodule": "custom_featurization"
default: "mtd"

# Command toggles, only *asked* for the custom profile. `preprocess_data` is always supported: every
# chain starts by materializing patient data. Tasks are not a stage — commands take external_labels_dir.
implements_pretrain:
Expand Down
40 changes: 29 additions & 11 deletions docs/CONFIG-AUDIT.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ Three keys look inert and are not:
- **`callbacks`** — `instantiate_group` branches on the node being empty, not on its value. Benign, but
note that dropping `model_checkpoint` silently changes which weights get published (see
`_persist_checkpoint`, §2).
- **`trainer.deterministic` / `trainer.benchmark`** — the exception to "nothing here can be quietly
- **~~`trainer.deterministic` / `trainer.benchmark`~~ Fixed; both now ship in `trainer/default.yaml`
alongside an explicit `precision`.** The exception to "nothing here can be quietly
wrong", and the reason `trainer.*` is qualified above. Neither appears in
`configs/trainer/default.yaml`, so Lightning runs with `deterministic=False` and cuDNN autotunes its
algorithms by timing. Nothing raises; the run simply is not reproducible, which makes every number it
Expand Down Expand Up @@ -96,9 +97,19 @@ These are the ones that matter. "Branch" column gives the exact site.
The complete fix is to record a config hash in the work directory and refuse to resume when it differs;
flipping the default buys most of that safety for one line, and the hash can follow if resume is ever
used in anger.
2. **Determinism settings** (`trainer.deterministic` / `trainer.benchmark`, §1, plus the hardcoded matmul
precision below). Equally silent, and it degrades every other result: two runs at the same seed can
disagree, so no measurement in the repository is reproducible evidence of anything.
2. **~~Determinism settings~~ (`trainer.deterministic` / `trainer.benchmark`, plus the hardcoded matmul
precision below). Fixed.** Equally silent, and it degraded every other result: two runs at the same
seed could disagree, so no measurement in the repository was reproducible evidence of anything.

`configs/trainer/default.yaml` now ships `deterministic: warn`, `benchmark: false` and an explicit
`precision: 32-true`, and `configure_cublas_workspace()` sets `CUBLAS_WORKSPACE_CONFIG` from the
dispatcher so `trainer.deterministic=true` is a usable strict mode on CUDA instead of an error at the
first matmul. `precision` is listed rather than left implicit because configs are struct mode: without
the key, `trainer.precision=bf16-mixed` needs Hydra's `+` and nobody guesses that.

What is now claimed, in the generated README and in the porting procedure, is *same seed + same config
+ same environment → same metric* — not "reproducible". The claim can only be **shipped** here, not
**proven**: proving it needs two real training runs, which needs a model (#7).
3. **`pipeline` / `pipeline_overrides`.** Known broken until 2026-08-01; see §3.
4. **`attach_labels=false`.** Produces a file `meds_evaluation` rejects. Loud, but nothing tells a user
the key exists or why they would want it.
Expand All @@ -116,13 +127,20 @@ Not config-driven, but in the same blind spot and worth inspecting together:
manifest, never asserted.
- `load_pretrained_weights`'s **zero-match `RuntimeError`** (`train.py:280`) — the load-bearing guard of
the `finetune` profile: it is what stops a renamed encoder from becoming a silent from-scratch run.
- **`torch.set_float32_matmul_precision("medium")`** (`train.py:60`, and again at `train.py:219`) — set
unconditionally, with no config key and no mention in any document. On Ampere and later this runs fp32
matmuls in bfloat16. It is stable run-to-run, so it does not break same-machine reproducibility, but it
silently diverges from any source implementation that ran at full precision — and because it lives in
the vendored contract, a user can only change it by editing a file `copier update` overwrites. For the
porting procedure in `template/docs/PORTING-A-MODEL.md.jinja`, whose deliverable is a ledger of
deviations that affect results, this is a deviation the template imposes invisibly.
- **~~`torch.set_float32_matmul_precision("medium")`~~ Removed.** Set unconditionally at `train.py:60` and
again at `train.py:219`, with no config key and no mention in any document. On Ampere and later this ran
fp32 matmuls in bfloat16 — stable run-to-run, so it did not break same-machine reproducibility, but it
silently diverged from any source implementation that ran at full precision, and living in the vendored
contract meant a user could only change it by editing a file `copier update` overwrites. For a porting
procedure whose deliverable is a ledger of deviations that affect results, that was a deviation the
template imposed invisibly.

Deleting the call was the whole fix: torch's own default is `highest` with `allow_tf32` off, so the
contract now simply does not take a position, and the environment matches the one every source
implementation ran in. No config key was needed — promoting it to one would have meant carrying it in
four command roots (training *and* inference; `zero_shot_direct` and `packaged` never train at all) and
keeping them agreed. `trainer.precision` is the sanctioned speed knob, and a port that specifically
needs TF32 sets it in `model.py`, which is the user's file and already requires a ledger row.
- **`CoverageError`** (`predict.py:175`) — reachable in normal use; nothing exercises it.
- **`InferenceKind.scores` / `MaterializedPredictCommand`** — reachable only through
`zero_shot_materialized`, which `skip_if_stub`s.
Expand Down
Loading
Loading