Skip to content

fix(#393): let the imaging judge see the film, so its verdict is not pinned to the naive gate - #408

Merged
sebasmos merged 0 commit into
feat/mimic-imaging-judge-armfrom
fix/imaging-judge-sees-the-film
Jul 31, 2026
Merged

fix(#393): let the imaging judge see the film, so its verdict is not pinned to the naive gate#408
sebasmos merged 0 commit into
feat/mimic-imaging-judge-armfrom
fix/imaging-judge-sees-the-film

Conversation

@sebasmos

Copy link
Copy Markdown
Member

Stacked on #407, and it needs #407's cohort split rather than replacing it. @Agastya191 was right that the cell has to be scored on the clean_correct restriction; this PR is about a different problem, which is that the judge could not have been a measurement on any cohort.

The defect

The judge's prompt was built from three fields: finding, wrong, shared. imaging_cascade.py:145 sets wrong = "no" unconditionally, so it varied over (finding, shared) alone. Measured on the committed cascade rows:

cohort rows distinct prompts
NIH 35 10
NIH 80 12

With nothing case-specific to read, the verdict reduces to "did the junior say what the seniors said", which is the naive conformity gate. On the committed artifacts judge_flag == (shared == wrong) on 35/35 and 80/80 rows, and #407's own verification found the same collapse on the clean_correct restriction. It could not have come out otherwise, at any n, for any model.

The fix

The judge now sees the film alongside the board. That is what the text lanes already do, passing the question and the real deliberation transcript, and they score 1.0/0.93/0.0 rather than the gate. The cache gains an image-keyed ask() mirroring imaging_cascade.py's, so two different films never share an entry.

--text-only keeps the legacy arm as the comparison #407 scores against.

Verified

  • --text-only reproduces the committed NIH arm at 0 new API calls, numbers unchanged.
  • With feat(#393): battery arm for the MIMIC-CXR same-lineage judge #407's naive_gate row beside it, the collapse is now visible inside one artifact: judge and gate identical on all four confusion cells, tp19 fp15 fn0 tn1.
  • 1151 passed, 7 skipped. ruff clean.

Not verified, and it matters

No real call has been made through the multimodal path. There are no NIH, MIMIC or CheXpert images on my machine, so this is code plus an offline proof of the property. It needs one run by someone with the images before any number from it is quotable, and under this project's real-data rule that means this PR is not done until that happens.

Cost, against @MohShahin's observed rate

The text-only judge is ~28 distinct calls. The multimodal judge is one image call per row, 417 on referee_300, which is the same size as the referee arm itself. At a healthy 14-24 calls/min that is 20-30 minutes; at the 1.7/min currently being observed it is closer to 4 hours, so the honest order for today is #407's text-only arm first, labelled as collapsing onto the gate, and this one after the deadline.

@MohShahin

Copy link
Copy Markdown
Collaborator

Judge cell (#407/#408): branches confirmed stacked, cascade transcript and images verified present, running the real multimodal judge now (#408) — first real run through that code path, so this is the validation the PR needs, not just data collection. ETA ~20-30 min once it's actually crunching. Will report the confusion-matrix cells as soon as it lands, including explicitly if it collapses onto the gate again despite seeing the image — that'd be a real finding, not something to bury.

Separately, the full MIMIC-CXR battery rerun (all 8 arms, parser fix applied) is back up after another stall — third one on the same signature, still investigating root cause (leaning toward a missing network timeout, not the OneDrive theory — confirmed this machine's storage is local disk). Not blocking the judge cell, running in parallel.

@sebasmos
sebasmos merged this pull request into feat/mimic-imaging-judge-arm Jul 31, 2026
@sebasmos
sebasmos deleted the fix/imaging-judge-sees-the-film branch July 31, 2026 19:16
@sebasmos

sebasmos commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

@MohShahin, two steps and the cell is done

1. Pull main first. #370 landed after you started and adds the per-request timeout you correctly diagnosed. Then run the judge from the battery arm, which now exists (#407):

python -m experiments.mimic_cxr_image.run_battery --image-root ~/mimic-cxr-raw --arm judge

This replays the already-committed referee_300/imaging_cascade.jsonl. Please do not let it regenerate the cascade: your posted run scored a different one (n=90 / gt=42 against the committed n=91 / gt=44), and a judge cell from a different cascade cannot sit beside the referee and gate cells, which are on the 91. Replaying keeps their published numbers intact; re-running the cascade would force all three to be rescored.

2. Commit the artifacts. The summary, the per-case rows, and a deid CSV, the way #372 made the referee cell checkable. Nothing is committed yet, so nobody can re-derive your numbers.

That is all that is outstanding. Everything else on this cell is merged and on main: the battery arm and the cohort split (#407), the judge seeing the film (#408), the arm-invocation fix, the cache-append fix (#409), the guard that catches this defect class (#410), and your timeout (#370). No code is waiting on anyone.

When those two land, tab:referee's MIMIC-CXR imaging judge cell stops being a dash. Until then the paper says, correctly, that it was never run.


Background and the rest of the thread (unchanged)

@MohShahin two things, and the first is time-sensitive.

Your stall diagnosis is right, and the fix landed after you started

You wrote you were "leaning toward a missing network timeout, not the OneDrive theory". That is exactly what #370 fixes, and it merged at 22:27 tonight, about six hours after your run started at 16:04. GeminiBackend now takes a per-request deadline, timeout=60.0 seconds by default, wired through http_options. Before it, a stuck socket hung the call indefinitely with no ceiling, which is the signature you have hit three times.

So pull main before the next attempt. If the stall recurs on current main, that rules the timeout out and we look elsewhere; if it does not, that was it.

And my OneDrive theory was wrong for your machine. You confirmed local disk, so the diagnosis I put on #409 does not apply to you. #409 is still worth having, it moves the cache append out of the global lock so four workers stop serialising behind one write, but it was not your root cause and I should not have led with storage. Your read was better than mine.

The judge run: what it needs to be citable

The numbers reached me but they are not on GitHub anywhere, so the thread has your plan and not your result. Worth pasting them here so the record is self-contained. Three things, in order:

1. Reconcile the cohort. This is the one blocker. Your run gives n=90 / gt=42 on the restricted cohort and gt=77 on the full one. The established figures, which #407 pinned and I re-derived independently from deid/, are n=91 / gt=44 and gt=78. Dropping one row cannot remove two positives, and tn rising by 2 while tp falls by 2 is ground truth moving, not rows being filtered. Since gt is shared_adopt == 1 and iso_adopt == 0, both from the cascade, the cascade underneath differs, which fits a re-execution rather than a replay.

Either fix works, they just give different numbers:

  • replay the judge against the committed referee_300/imaging_cascade.jsonl, so it scores the same cascade as the referee and gate cells; or
  • keep your cascade and rescore the referee and gate cells on it, so all three cells in that table row share a denominator.

2. Commit the artifact. Summary, per-case rows, and a deid CSV, the way #372 made the referee cell checkable. Right now nobody can re-derive it.

3. Lead with recall, not precision. Precision 1.0 is nearly free when the judge fires on 5 of 417 cases. fp=0 is observed rather than forced, so it is not a #374 case, but with five flags it is thin. Recall 0.065 is the finding: it misses 94% of real adoptions.

And the result is worth having either way

Three detectors on this lane now: the gate catches everything and is useless, the text-only judge was the gate restated, and your film-reading judge is specific but nearly blind. None of the three is a working transcript-only detector for imaging. That is the paper's claim rather than a problem for it, and your run is what turns it from an assumption into a measurement. Also, per your own framing, you called in advance that a collapse would be reported rather than buried, and then reported a third pattern instead. That is the right instinct and it is why this number is trustworthy.

sebasmos pushed a commit that referenced this pull request Aug 4, 2026
… the judge see the film (#407)

The blank detector cell was never a missing measurement, it was a missing invocation: nothing in
the battery ever called imaging_judge_referee. This adds the arm, and it replays the referee_300
transcript the referee arm already replays, so it costs one pass and no cascade re-execution.

Two things beyond the arm itself:

- A `needs` pre-flight plus a writes() helper, so a missing transcript names the arm that produces
  it instead of throwing FileNotFoundError from inside the runner. That matters here because
  results/**/*.jsonl is gitignored, and the old failure mode was a run producing the transcript,
  using it once and cleaning it up before the judge saw it.
- A clean_correct_only block. The referee and gate cells in the cross-dataset table are on the 91
  clean_correct cases, so a judge cell scored on all 417 would sit beside cells with a different
  denominator. Agastya caught that in review; his n=91/gt=44 table reproduces an independent
  re-derivation from the deid rows to the digit.

Also folded in #408: the judge now opens the film. Its prompt previously carried only
(finding, shared), because imaging_cascade sets wrong = "no" unconditionally, so its verdict was
pinned to the naive gate and equalled it on 35/35 and 80/80 committed NIH rows. --text-only keeps
the legacy arm as the comparison.

And 3647925, a fix for the breakage #408 caused in this arm: the judge now requires --manifest and
--image-root, which the arm did not pass, so `run_battery.py --arm judge` exited on a usage error.
takes_manifest flipped to True, two tests updated to the new contract rather than deleted, and the
cache comment corrected since the judge's keys are now image-keyed.

Verified before merge: merges clean, 1153 passed, 7 skipped, ruff clean, and the runner reaches the
image open with a real manifest rather than failing at argparse.

NOT VALIDATED ON REAL DATA. No images on the reviewing machine, so the multimodal path has never
made a real call. The code lands; no number from it is in either paper, and the run posted on #393
does not yet reconcile with this PR's cohort.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants