Add federated EHR T2DM risk-prediction tutorial (MLP on OMOP, both backends) - #1068
Draft
atriaybagur wants to merge 6 commits into
Draft
Add federated EHR T2DM risk-prediction tutorial (MLP on OMOP, both backends)#1068atriaybagur wants to merge 6 commits into
atriaybagur wants to merge 6 commits into
Conversation
A tabular, OMOP-only FLIP tutorial: a small MLP that predicts type-2-diabetes onset from OMOP CDM features (demographics + pre-diagnosis condition/visit history), for both NVFLARE (Client API) and Flower. No imaging — no Orthanc, XNAT, PACS pull or data-enrichment; the labels live in OMOP and are projected by query.sql, mirroring the xray classification tutorial's data path. Data: the public 1k-person Synthea-in-OMOP dataset (AWS Open Data Registry, anonymous HTTPS, fetched at run time, never committed). Two paths: - local sim: utils/build_synthea_dataframe.py derives per-site CSVs (make -C fl-tutorials download-synthea-data); - on-platform: a new omop_db_tools.synthea_ehr loader (make -C trust load-synthea-ehr) loads person/condition/visit into a running trust OMOP, since the shipped mock OMOP carries no condition rows. Synthea ids are shifted into a reserved band to coexist with the imaging cohorts, and query.sql scopes to persons with a recorded condition (valid on real trusts, and excludes the mock's imaging-only persons). FK-safe, idempotent, unit-tested. Model tuned for real performance: a 32-unit hidden layer trained 8 local epochs x 5 rounds (LR 0.02->0.001) reaches ~0.85-0.92 held-out AUROC on the Synthea cohort. Tests: CPU transform-chain + model coverage in fl-tutorials/tests; the Flower min_clients guard auto-covers the new app; loader transforms unit-tested in trust/omop-db. Comprehensive per-backend READMEs. Wires download-synthea-data / load-synthea-ehr make targets, run-tutorial.sh, the sync-check pairs + workflow path, and the CLAUDE/AGENTS docs. Removes the broken nvflare/data symlink that blocked data/ downloads. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The Flower app files descend from Flower's quickstart-monai example and kept its name in their module docstrings — misleading now that the standard template also backs non-MONAI apps (the EHR tabular tutorial). Rename each docstring to the app it actually is: the sync-pinned server_app copies take their template's name (standard-app / evaluation-app, model-agnostic), the spleen client/__init__ docstrings take their tutorial's name, and the docs' pyproject excerpt now matches the file it quotes (name = "standard-app"). Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
This was referenced Aug 27, 2026
The loader projected only the columns the tutorial's query.sql reads, but data-access-api's cohort statistics compute the age distribution from person.birth_datetime — with it NULL every cohort submission failed with 'Statistics aggregation failed' and the project could not be staged. Carry birth_datetime through from the source CSV (which always provides it) and require it in the schema-drift guard. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
7 tasks
Six conflicts, all from #1070 (the shared fl-tutorials/datasets/ tree) landing on develop mid-flight; resolved by moving the synthea tooling into that tree, as agreed when the two PRs were sequenced. - fl-tutorials/Makefile: develop's TUTORIAL_TARGETS/DATASET_TARGETS split, with download-synthea-data added to the forwarded dataset targets. - fl-tutorials/{flower,nvflare}/Makefile: develop's copies — the per-backend dataset targets are gone, so the flower→nvflare download-synthea-data delegate and the nvflare builder target (EHR_DIR / SYNTHEA_OUTPUT_DIR) go with them. - fl-tutorials/datasets/Makefile + README: download-synthea-data lives here now, single copy, backend-agnostic, writing to the shared data/synthea/; build_synthea_dataframe.py moves from the NVFLARE tutorial's utils/ to datasets/synthea/ (the tutorial's .env.app and README follow the path). - fl-tutorials/flower/run-tutorial.sh: develop's DATA_ROOT layout, with the ehr_risk_prediction case re-added on it. - CLAUDE.md/AGENTS.md: the fl-tutorials tree line carries both the EHR tutorial and the datasets-tree sentence; AGENTS.md regenerated from CLAUDE.md. Both sides had already deleted the fl-tutorials/nvflare/data self-symlink. Verified: make -C fl-tutorials test (134 passed), list-tutorials on both backends shows ehr_risk_prediction, a real download-synthea-data lands under fl-tutorials/data/synthea/ (1097 persons, site splits 553/544). Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…g cohorts clean_reserved_band deleted every row with person_id >= PERSON_ID_OFFSET (900_000_000) to make `make load-synthea-ehr` idempotent. That band is not reserved: the imaging cohorts' person_id is nhs_number_to_integer(PatientID), the first 9 digits of a real (effectively random) NHS number, so it scatters across the whole 9-digit range rather than staying small. Measured against the published mock data, 2/41 spleen and 825/8332 cxr person_ids already land >= 900_000_000. Confirmed live on trust1: 4187 imaging persons expected (21 spleen + 4166 cxr), 422 with person_id >= 900_000_000, 4187 - 422 = 3765 survivors — exactly what the DB held. fpk_image_occurrence_person_id is ON DELETE CASCADE, so each deleted person took their image_occurrence rows with them too (procedure_occurrence likewise); every reload was silently destroying ~10% of a trust's imaging cohort and its imaging rows. Fix: delete by provenance, not by id range. build_person_rows already wrote `synthea-<id>` into person_source_value; clean_reserved_band now deletes persons by that marker (LIKE 'synthea-%') and scopes the condition_occurrence / visit_occurrence deletes by joining back to those owned persons, rather than trusting condition_occurrence_id/visit_occurrence_id to stay outside whatever range the imaging loader used for them (unverified, unlike person_id). This is the actual ownership guarantee the module's docstring claimed but didn't have. PERSON_ID_OFFSET is retained (still needed to keep newly-inserted Synthea person_ids clear of the imaging range) but raised from 900_000_000 to 1_000_000_000 and documented: imaging person_id is bounded above by 999_999_999 by construction of nhs_number_to_integer (first 9 digits only), so any offset >= 1_000_000_000 cannot collide with an imaging person_id on insert. That bound, not the offset's mere size, is what makes the shift insert-safe — stated explicitly since a future change to NHS-number generation would invalidate it. The offset no longer has any role in the delete's correctness. Also updates the stale "reserved band" design-note claims in the module docstring, README.md, and the CLAUDE.md/AGENTS.md pair. Tests: existing test_synthea_ehr.py suite kept green (person_source_value assertions tightened to check the actual synthea- prefix; the offset test's threshold and rationale updated for the new value/invariant). Added TestCleanReservedBandProvenance — a genuine regression test using an in-memory SQLite "omop" schema (via ATTACH DATABASE, no Postgres required) seeded with an imaging person at person_id=923226025 (a real spleen-export value, in the old retired danger band): asserts it and its visit_occurrence row survive a load + reload cycle, and that Synthea's own rows are replaced rather than duplicated on reload. Independently confirmed the old `DELETE WHERE person_id >= 900_000_000` logic does remove that row. ruff and mypy clean. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
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.
What
A new FLIP tutorial: a federated EHR risk-prediction model — a small MLP predicting Type-2-Diabetes (T2DM) onset risk from OMOP tabular features — for both NVFLARE and Flower. The tabular, neural-network sibling of the chest-X-ray classification tutorial and of #635.
No imaging: no Orthanc, no XNAT, no PACS pull, no data-enrichment. The label lives in OMOP (like the xray tutorial), projected by the cohort SQL — the
query.sqlselects the cohort with noaccession_idcolumn, so imaging is a no-op for this project.Data source
Public Synthea-in-OMOP 1k dataset on the AWS Open Data Registry (
s3://synthea-omop/synthea1k/{person,condition_occurrence,visit_occurrence}.csv, anonymous HTTPS, ~5 MB, static 2023 export). Verified live: 1130 persons, T2DM prevalence 6.1%, all 6 SNOMED feature codes present. Never committed — fetched at run time.OMOP populate path
The shipped mock OMOP has zero
condition_occurrencerows, so the cohort query returns nothing on the dev stack. Newmake -C trust load-synthea-ehr(backed bytrust/omop-db/src/omop_db_tools/synthea_ehr.py) downloads Synthea from S3 and loads person/condition/visit into a running trust OMOP: per-trustperson_id-modulo split, ids shifted into a reserved band to coexist with the imaging persons, FK-safe, idempotent, unit-tested.Model & performance
Small MLP (hidden 32, 8 local epochs × 5 rounds, LR 0.02→0.001). Measured across 7 seeds: val AUROC ~0.90 ± 0.06, test ~0.92 ± 0.05. READMEs quote honest measured numbers.
Verification (all green in CI-equivalent local runs)
make -C fl-tutorials pytest→ 126 passed, 12 skipped (16 new EHR feature-engineering tests + themin_clientswiring guard auto-covering the new Flower app).make -C trust/omop-db local_test→ 54 passed (incl. 8 loader tests); mypy + ruff clean.ruff checkoverfl-tutorials→ clean.make export(CPU) → job builds cleanly with the tuned config.scripts/check_tutorial_sync.sh).Also in this PR
quickstart-monaidocstrings out of every Flower app file — leftovers from the Flower quickstart the template descends from, misleading now that the standard template also backs this non-MONAI tabular app. Docstring-only: the sync-pinnedserver_app.pycopies change together with theirfl-apps/flower/{standard,evaluation}templates (nowstandard-app/evaluation-app), the spleen client/__init__docstrings take their tutorial's name, and the docs' pyproject excerpt now matches the file it quotes.Draft — remaining before ready
Live dual-backendDone (2026-08-27): full lifecycle PASSED on both backends against the dev stack (NVFLARE then Flower, one shared approved project; modelsmake e2e_smokeEHR T2DM Risk MLP (NVFLARE)/(Flower), bothRESULTS_UPLOADEDwith genuine global-model artefacts). Two fixes fell out and are in this PR: the loader now populatesperson.birth_datetime(data-access-api's cohort statistics require it — without it staging fails on every trust), and the READMEs now documentEXTRA_ARGS="--image-pull-threshold 0"(the imaging stage is not a no-op: the pseudo-accessions produce an empty XNAT project and all-failed pulls per trust, which trips the smoke's pull guard).S3 version pin for the registry dataset— decided against (2026-08-27): the objects are static 2023 uploads, a pin adds nothing.Merge Consolidate fl-tutorials dataset tooling into a shared datasets/ tree #1070 first, then move the synthea tooling into the sharedDone (2026-08-28,fl-tutorials/datasets/tree3759aef1): develop merged;build_synthea_dataframe.pynow lives atfl-tutorials/datasets/synthea/,download-synthea-datais a single backend-agnostic target indatasets/Makefile(forwarded from the fl-tutorials root, no FL_BACKEND variant), output under the sharedfl-tutorials/data/synthea/; the NVFLARE.env.appand both READMEs follow the path, and the per-backend delegate targets are gone.Closes part of the example-coverage work; supersedes the earlier
claude/flip-example-coverage-sfvhmcspike.