MIMIC-CXR: deterministic subset tooling + imaging battery (closes #324) - #329
Conversation
|
This is unblocked now: #263 is merged (real budgeted run passed, and the |
) Select a nested, seed-deterministic MIMIC-CXR study subset and emit the download list for just those images, so the imaging battery runs without pulling the ~600 GB release. - build_subset.py: `select` ranks studies via budget.subsample_cases (nested arms: 100 < 150 < 300 < 600, with 35 nested for the NIH-matched comparison) and writes per-arm manifests + download_list.txt; `verify` checks images resolve and writes sha256 provenance, mirroring the NIH imaging lane. - All images per study; finding-positive studies by default. - Reproducibility = deterministic list (from seed) + pinned per-arm manifests + provenance checksums + a standard-tool wget one-liner (no bespoke fetcher, no credentials in the repo), matching how the NIH manifest pins its data. - Manifest CSVs are gitignored: they embed credentialed report text and regenerate from the seed (README documents the workflow). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- add the imaging+model dependency install step (runners need the image extra plus google-genai; a fresh follow hit ModuleNotFoundError without it) - give the full per-arm battery commands incl. the referee/cascade dependency - --n 9999 (all images in the arm); the prior --n 600 silently dropped 234 of the 834 solo images Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The watermark cue makes imaging_cascade write a fixed imaging_cascade.jsonl, so running the #311 (cascade_150) and referee-prep (referee_300) cascades into one --out silently overwrote the #311 arm. Route each to its own subdir and wire the referee at the referee_300 transcript. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Aggregate per-arm summaries for the six-arm battery on the seed-20240724 subset: solo (#310), cascade_150 (#311), system_flag (#312), strength_cascade (#313), blind_metric (#314), referee_300 cascade + referee (#315). Per-case *.jsonl transcripts, caches, and run logs are gitignored: they link MIMIC case_ids to reads (credentialed-data caution) and regenerate from the runners. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Solo on the nih_match_35 arm (35 studies / 50 images): flip above noise ~0 for every cue (watermark -0.04, noise floor 0.18), confirming the full 834-image solo result at the matched-to-NIH size. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
53bfb36 to
6834113
Compare
…#339) Offline split of the committed transcripts into genuinely-false (wrong==no, the honest harmful-sycophancy measure) vs planted-truth (wrong==yes). On MIMIC ~80% of finding-present cases plant the truth (poor pneumothorax clean reads), so ALL contagion is inflated: - cascade_150 (#311): ALL +0.82 -> genuinely-false +0.49 - referee_300 cascade: ALL +0.81 -> +0.51 - strength_cascade (#313): ALL ~+0.80 -> ~+0.53-0.55 (flat across opacity) - system_flag (#312): robust, +0.225 ~ ALL +0.200 Qualitative conclusions unchanged. No API/network; reads transcripts only. imaging_cascade fixed on main (#333); system_flag/strength_cascade tracked in #339. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Follow-up per #332/#339: added
No re-run needed: |
Agastya191
left a comment
There was a problem hiding this comment.
good work on build_subset.py and all other files. The study-level nesting, the single download_list.txt for the 600-study superset, the per-arm --out note that keeps the two watermark cascades from colliding, and test_mimic_subset.py covering order-invariance are all solid, and folding in plant_direction_check.py for #332 was the right call.
One problem is that in build_subset.py DEFAULT_SIZES["nih_match"] = 35 counts studies, while select_arms expands every picked study to all of its images. This results in nih_match_35.csv shipping 50 images rather than 35 (selection_summary.json records studies: 35, images: 50, and results/nih_match_35/imaging_solo_summary.json reports n: 50), so the matched comparison headlined in the description is 50-vs-35 against NIH's n: 35, and those 50 images cluster into 35 studies, meaning the MIMIC flip rate averages over correlated repeat views of the same study while the NIH set is one image per case.
You have multiple options to fix this. For example, I would size the nih_match arm at the image level, walking studies in rank order until the arm hits exactly 35 images, so the matched arm is genuinely the same n and the same one-image-per-observation shape as the NIH run.
Requesting changes only because that arm's number is going into the cross-dataset table on #295; everything else here reads ready to merge. Separately, and not worth blocking on: section 4 of the README doesn't include the imaging_solo --manifest $M/nih_match_35.csv invocation that produced the committed results/nih_match_35/ summary, so the recipe doesn't currently regenerate all six arms.
sebasmos
left a comment
There was a problem hiding this comment.
This is great work, exactly what was missing for the cross-dataset table. One thing before merge: nih_match is sizing by study and ending up with 50 images instead of 35, so let's walk it study by study until it hits exactly 35 images, then this is ready to go.
maximinl
left a comment
There was a problem hiding this comment.
Peer review — agree with Agastya and Seb on the nih_match sizing; everything else looks merge-ready.
Solid
- Study-level nesting via
subsample_cases, singledownload_list.txtfor the 600-study superset,verifysha256 provenance, and order-invariance tests are the right reproducibility shape for credentialed MIMIC without pulling 600 GB. plant_direction_check.pyfolding in the #332 correction (ALL contagion inflated by truth-planting; genuinely-false ~+0.49–0.53) is exactly what the cascade cell needs before the cross-dataset table cites it.- DUA handling (gitignore caches/manifests, commit aggregates) matches the lane.
Blocker: nih_match is 35 studies → 50 images
Confirmed in the committed artifacts: selection_summary.json has nih_match: {studies: 35, images: 50} and results/nih_match_35/imaging_solo_summary.json reports n: 50. The matched NIH comparison is therefore 50-vs-35, and the 50 MIMIC rows are correlated multi-view clusters while NIH is one image per case.
Please size nih_match at the image level: walk studies in rank order until the arm hits exactly 35 images (as Agastya suggested), regenerate that arm’s solo summary, and update the #295 table to the corrected n. Not blocking on the README gap Agastya noted (missing imaging_solo --manifest …/nih_match_35.csv in §4), but worth fixing in the same pass so the recipe regenerates all six arms.
sebasmos
left a comment
There was a problem hiding this comment.
The five non-matched arms are real and the provenance work (seeded manifest, sha256 per image, download list) is solid.
Still the same single blocker on head: nih_match is 35 studies but 50 images, so the matched cell is 50-vs-35 against NIH's 35 and is not comparable. The fix is small: walk studies until the image count hits 35, update the nesting test, rerun that one arm.
No rush given you are travelling, and if it is easier, someone with PhysioNet access can push that one fix rather than it waiting on your connection. Everything else here I am happy with.
Reviewers (Agastya, sebasmos, maximinl) flagged the matched-to-NIH arm shipped
50 images (35 studies x all views) vs NIH's n=35 one-image-per-case, so the
cross-dataset cell was 50-vs-35 and averaged over correlated multi-view clusters.
- build_subset.py: IMAGE_SIZED_ARMS={'nih_match'}; the arm now takes the first N
images in study-rank order (exactly 35), still a prefix of the ranking so it
nests inside the study-sized arms (verified on real data: nih<blind<cascade<referee<solo).
- selection_summary.json: nih_match now studies:26, images:35.
- test_mimic_subset.py: split study-level nesting from a dedicated image-sizing
+ determinism test.
- README section 4: add the nih_match_35 imaging_solo invocation (Agastya's note),
so the recipe regenerates all six arms.
- Remove the stale n=50 nih_match solo summary; it regenerates on the real-data
rerun (35 images, a cached subset of solo_600 + noise floor).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed the
Left |
There was a problem hiding this comment.
Image-count sizing is the right call and the nesting still holds, so the matched cell is finally comparable to NIH's 35. Splitting the determinism test out from the nesting test is a good touch. Approving. Note the matched-cell number in the paper stays pending until the real rerun lands, which is #347's ask.
Updated 2026-07-29: merging this is now the first step in unblocking that rerun, so it is worth doing ahead of the other imaging PRs. #347 is based on this branch and cannot run two of its six arms until it sits on main, because imaging_system_flag.py and imaging_strength_cascade.py here still carry the pre-#338 plant direction and #347's own guard correctly refuses them. Once this lands, #347 rebases onto main, picks up the fixed runners, and the full battery can run.
One process note so this does not stall: @Agastya191 and @maximinl both still have change requests on this PR from before d3331d0, so it cannot merge on my approval alone. A re-review from either of you, or dismissing the stale reviews, is the only thing left.
Reviewers (Agastya, sebasmos, maximinl) flagged the matched-to-NIH arm shipped
50 images (35 studies x all views) vs NIH's n=35 one-image-per-case, so the
cross-dataset cell was 50-vs-35 and averaged over correlated multi-view clusters.
- build_subset.py: IMAGE_SIZED_ARMS={'nih_match'}; the arm now takes the first N
images in study-rank order (exactly 35), still a prefix of the ranking so it
nests inside the study-sized arms (verified on real data: nih<blind<cascade<referee<solo).
- selection_summary.json: nih_match now studies:26, images:35.
- test_mimic_subset.py: split study-level nesting from a dedicated image-sizing
+ determinism test.
- README section 4: add the nih_match_35 imaging_solo invocation (Agastya's note),
so the recipe regenerates all six arms.
- Remove the stale n=50 nih_match solo summary; it regenerates on the real-data
rerun (35 images, a cached subset of solo_600 + noise floor).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(chexpert): delete duplicate runners, use shared imaging_*.py (#331) Addresses all feedback from Agastya191 on Issue #331 and PR #328: Adapter (benchmaxxing/datasets/chexpert.py): - Multi-label extraction with CLINICAL_HIERARCHY ordering (14 findings) - Explicit uncertainty policy: -1.0 treated as negative (cannot anchor a plant) - Fixed limit bug: deterministic subsample via budget.subsample_cases, not CSV truncation Structural refactor: - Deleted chexpert_solo.py, chexpert_cascade.py, chexpert_referee.py, chexpert_matched_temp_floor.py (copies of imaging_*.py differing by 12-26 lines) - Deleted build_manifest.py (replaced by build_subset.py) - New build_subset.py following the MIMIC-CXR pattern (#329): deterministic nested per-arm manifests (solo_600, referee_300, cascade_150, nih_match_35) with finding-absent as a separate subgroup - Shared runners inherit all fixes: ground-truth plant (#333), clean_correct field, matched temp floor, strength cascade, blind metric, judge referee Tests: - Updated test_chexpert_adapter.py: multi-label, clinical hierarchy, uncertainty policy, limit subsampling (8 tests, all passing) * feat(staging): move per-image SHA256 from custom builders to stage_dataset * fix: address review comments on staging and subsets * compliance: implement preregistration endpoints E1/E2 for CheXpert * feat: add chexpert blind metric substitution probe (Issue #308) * docs: add cross-dataset comparison table for blind metric (Issue #294) * docs: update CheXpert metrics with solo_600 partial run results * style: fix remaining ruff lint errors in CheXpert runner scripts * feat(imaging): Phase 1 Issue #331 structural improvements - Implement strict round-robin clinical stratification in build_manifest.py using clinical hierarchy to prevent monocultures (e.g. pneumothorax dominating). - Centralize --n sampling into build_manifest.py and remove legacy hardcoded --n dependencies from solo, cascade, and temp_floor scripts. - Introduce n_genuinely_false and genuinely_false_contagion metrics to imaging_cascade.py to strictly isolate cases where clean reads failed. - Parameterize --cue in imaging_matched_temp_floor.py (removing 'watermark' hardcodes from both CLI and summary read outputs) to dynamically support all 4 injectors. - Ensure cross-dataset reproducibility by asserting random.shuffle is performed natively across the normalized Case outputs, agnostic to the dataset adapter. * feat: standardise MIMIC-CXR for universal imaging lane and fix matched-temp floor (#343, #344) This commit integrates the MIMIC-CXR adapter fully into the universal imaging orchestrator. - Decoupled FINDING_COLUMNS and CLINICAL_HIERARCHY into a shared benchmaxxing/utils/clinical_labels.py to prevent tight coupling between chexpert and mimic_cxr. - Updated mimic_cxr.py to extract multi-labels (pipe-separated) matching CheXpert's conservative policy (treating -1.0 uncertainty as negative) to enable proper dataset stratification. - Used safe parameter injection (label_format="legacy") inside build_manifest.py via inspect.signature to guarantee 100% backward compatibility for legacy text-lane scripts without poisoning Python's module-level global state. - Updated CROSS_DATASET_COMPARISON.md confirming that all 3 imaging datasets now strictly run under the matched-temperature floor. * fix(#337): meta assertion broke on the additive labels field test_join_reports_and_labels asserted exact dict equality on Case.meta, so it failed as soon as this branch surfaced the full CheXpert label vector as meta["labels"] for natural-cue selection. The new field is legitimate, the assertion was brittle. Now asserts the three fields the imaging lane depends on exactly, bounds the allowed key set so an unrelated field still fails loudly, and validates the shape of labels when present. Strictness is kept, additivity is allowed. Merged with current main this gives 800 passed, 7 skipped, ruff clean. * fix(#337): make --n consistent across the imaging runners, and keep the cohort audit trail Two review asks, both about not silently changing the cohort every published NIH number sits on. 1. The --n removal was only half applied. imaging_cascade, imaging_solo and imaging_matched_temp_floor lost the flag, while imaging_system_flag, imaging_strength_cascade and imaging_blind_metric still accept it, so the nine documented invocations in this directory's README worked for some runners and exited with "unrecognized arguments: --n 35" for others. Restored it on the three, which is the least invasive way to make the six consistent and keeps every documented command valid. If the intent is really that the manifest alone dictates n, that is a reasonable design, but it needs applying to all six runners and the README rewritten in the same change. 2. build_manifest.py had switched the default from release order to seeded round-robin stratification and dropped the docstring sentence promising that a rebuild reproduces the committed 35-case set. Stratification is genuinely useful for building a NEW cohort with better finding coverage, so it stays, but behind an explicit --stratify flag with release order restored as the default. label_format is now only requested when stratifying, since asking unconditionally moved the default cohort too. Verified both paths against the committed manifest: a default release-order rebuild at --n 35 recovers the same 35-case set, and --case-ids-file reproduces it byte-identically, which is what the docstring promises. Four tests pin the contract, one of them asserting the reproducibility sentence is present, since it was deleted once already. The plant direction, the guard result and the subgroup work on this branch are untouched. --------- Co-authored-by: YEHUDHAH KENNEDY RODRIGUEZ MORAN <yehu@192.168.1.4> Co-authored-by: YEHUDHAH KENNEDY RODRIGUEZ MORAN <yehu@192.168.1.6> Co-authored-by: sebasmos <sebasticajas@gmail.com>
Rebased onto
mainnow that #263 has merged: the diff is only the MIMIC-CXR tooling (13 files, no budget changes).What this adds
Tooling to run the imaging standard battery on MIMIC-CXR without pulling the ~600 GB release:
experiments/mimic_cxr_image/build_subset.py—selectranks studies by a stable hash of the seed (nested arms: 100 < 150 < 300 < 600, with 35 nested for the NIH match), writes per-arm manifests +download_list.txt;verifychecks images resolve and writes sha256 provenance.README.md— full reproduction recipe (deps -> select -> download -> verify -> all six arms), with a per-arm--outso the two watermark cascades cannot collide.*_summary.jsonfor the six arms (seed20240724, gemini-2.5-flash). Per-case transcripts, caches, manifests, download list, and provenance are gitignored (credentialed report text; regenerate from the seed).tests/test_mimic_subset.py— study-level nesting, all-images-per-study, order-invariance, eligibility, URL formatting.Results
Posted to #295. Committee/cascade effects are strong and consistent; solo susceptibility is at the noise floor on MIMIC (matched 35-vs-35: NIH watermark +0.11 vs MIMIC -0.04).
Note (#332): the cascade contagion cell is being refined by the plant-direction reanalysis. On MIMIC ~80% of cases plant the truth (the model reads pneumothorax poorly), so the honest genuinely-false contagion is ~+0.49-0.51, not the ALL +0.82. A follow-up commit will report the genuinely-false subgroup.
Closes #324. Part of #295. Unblocks #92.