Real fairness, calibration, and baseline-gap artifacts from the canonical run - #74
Real fairness, calibration, and baseline-gap artifacts from the canonical run#74duckyquang wants to merge 13 commits into
Conversation
E09 requires the calibration scaler to never touch test, but run_calibration fit and applied on the same test predictions, which gives an optimistic ECE. The missing-modality eval now persists per-condition val predictions alongside the test ones (predictions_val), and run_calibration fits there when the field is present, recording scaler_fit_on in the output. Old results JSONs without val predictions keep the in-sample behavior and say so. Also pins the #72 mechanism while in the test file: with one pooled ECG vector tiled into identical tokens, echo_to_ecg attention output is independent of the echo query -- identical keys force uniform weights and identical values make the weighted sum equal that value -- so that direction is a fixed linear image of the pooled ECG vector until token-level ECG exists.
…ition E08 needs fairness numbers for full and echo_dropped, but run_fairness only read the three-table layout and only scored the full condition. Both layouts now route through prepare_fused_probe_data, so non-finite rows are dropped the same way as everywhere else -- the old three-table path scored NaN-embedding rows the canonical rerun excludes, so the two invocation styles disagreed on n. Per-condition results land in results/fairness/<condition>/ with the shared provenance; the combined JSON keeps the full-condition overall/by at top level because aggregate.py reads them there. The drop count is renamed n_dropped_nonfinite_all_splits since it counts across train/val/test, not just the reported test rows. Strata with missing demographics fall back to 'unknown' instead of crashing sorted() on NaN, and the MIMIC gender/race curation-bias note rides along in every output. load_and_merge and prepare_tokens had no callers left and are gone.
The published baselines (EchoJEPA 5.97 MAE, ECG-FM 0.929 AUROC) are from different cohorts, so they cannot say whether fusion is broken. This scores all four trained probes on the identical test frame with bootstrap CIs, paired fused-vs-solo deltas, an LVEF label-source error breakdown, and a fused train/val/test overfit check, writing results/baseline_gap.json. On the canonical checkpoints: fused 10.42 MAE / 0.771 AUROC beats ECG-only (11.60 / 0.671) with paired significance and is never behind echo-only or concat, so the gap to published numbers is cohort and label regime, not the fusion. The label breakdown puts 43.0 MAE on the six lvef_upper test rows -- 18 of that fallback's 24 cohort rows carry an implausible LVEF of 100.0.
The real-data path needed the cohort plus two separate embedding tables, which no longer exist as such -- the joined manifest is the canonical table. --manifest builds the per-modality matrices straight from it, trains the ridge harness on the train split and reports strictly on test (the old path lumped val into ~train), guarding against empty splits and clip-level input. This replaces the synthetic-demo-only situation: the taxonomy, complementarity, and dropout-profile artifacts can now come from the real 1,208-row cohort instead of results/failure_demo.
…he docs All numbers from real seed-42 CPU runs on the 1,208-row manifest (Apple M1 Pro, 16 GB; commands in the respective scripts): - README gains the in-cohort four-probe baseline table with CIs, the fairness stratification for full and echo_dropped, and val-fit calibration ECEs (0.072 / 0.043 / 0.015), with the caveat that ecg_dropped's low ECE reflects predictions collapsing toward the mean, not a usable degraded model. - TECHNICAL 6.4 now states what the cross-attention block can actually attend to: with pooled inputs both directions provably degenerate and the fused probe is concat-equivalent; echo_to_ecg stays dead until token-level ECG is re-extracted (#72). The README results note says the same. - TECHNICAL 8 places the published solo baselines next to the in-cohort ones and names the measured contributing factors, including the lvef_upper label artifact. - TECHNICAL 3.1/11 fix the pairing window to the +/-24h the cohort was actually built with (config said 48; the manifest's max |delta_hours| is 23.98) and record the within-manifest tightening curve; widening needs the BigQuery rebuild scripts/run_cohort_sensitivity.py wraps. - TECHNICAL 9 carries the fresh measured wall-clocks (47.7s four-probe train, 11.8s missing-modality eval, 8.3s fairness) and the storage footprint. - Config placeholders are either real repo-relative paths now (hubert_ecg_parquet) or explicitly documented as local overrides; the ECG and echo pooling decisions from E11 are recorded where they are consumed.
kevzho
left a comment
kevzho
left a comment
There was a problem hiding this comment.
I checked this against the current PR head (035a258) and reran the real-data pipeline independently. My manifest is 14,278,192 bytes with SHA-256:
81694c9ba20fbf4af596c942c0fa3acf76275248142204e02324681f7c7a982d
The cohort properties and reported probe, bootstrap, fairness, calibration, label-source, and failure-analysis results reproduce.
One blocking reproducibility issue remains: M10 writes the canonical fused checkpoint to probes/fused/cross_attn_fused.pt, but the README and evaluation examples use probes/cross_attn_fused/cross_attn_fused.pt. In my checkout these are different checkpoints. The documented command loads the older checkpoint and produces 10.28 MAE / 0.766 AUROC rather than the reported 10.42 / 0.771. Let me know when an update has been initiated so that everything is consistent within the README, script defaults, and examples.
Also, record the manifest and checkpoint SHA-256 hashes in run provenance;
include checkpoint, seed, and prediction-source provenance in calibration output;
keep #69 open, or move the still-pending pooled-vs-clip echo comparison to a linked follow-up; provide sanitized aggregate result artifacts or a checksummed result bundle, since results/ and probes/ are gitignored.
Everything checks out otherwise; the rebased commit is practically identical to 507e802. I'll be happy to approve once more once these issues are fixed.
Review on #74 caught a real reproducibility hole: train_probes.py writes the canonical fused checkpoint to probes/fused/, but the README command and the eval-script defaults pointed at probes/cross_attn_fused/ -- a directory only the old M09-only runner populates. On a checkout that had run both, the documented command silently loaded the stale checkpoint and reported the old 10.28/0.766 instead of the canonical 10.42/0.771. Every reference now agrees on probes/fused/, including cross_attn.run's default out_dir. Path agreement alone cannot prove two machines scored the same model, so provenance now carries content hashes: sha256_file in probes/common.py, recorded for the manifest and checkpoints in run_metadata.json, the missing-modality payload, the fairness output, and the gap-diagnosis JSON. The calibration artifact was only traceable through its input file, so it now embeds predictions_path plus the source checkpoint/seed/hash block outright. Verified against the reviewer's independent reproduction: manifest sha256 81694c9b... matches, and retraining here reproduces the fused checkpoint bit-identically (bac18bb8...).
results/ and probes/ are gitignored on purpose (per-example predictions and checkpoints stay local), which left reviewers nothing committed to diff reported numbers against. export_result_bundle.py copies the aggregate-only result JSONs into docs/results/ -- stripping the per-example prediction blocks from the missing-modality payload -- and writes a SHA256SUMS covering both the bundle and the local raw artifacts, so an independent reproduction verifies with one shasum -c. The checksum list names only the canonical M10 checkpoint locations; stale directories from older layouts stay out so they can't masquerade as canonical. All seven artifacts here were regenerated in this run against data/processed/echo_hubert_manifest.parquet (sha256 81694c9b...), and the README provenance note now cites the manifest and checkpoint hashes directly.
|
All five items are in, pushed as two commits ( Checkpoint path (the blocking one). Confirmed the mechanism you hit: Hashes in provenance. Calibration provenance. #69. Opened #77 (E12) carrying the still-pending pooled-vs-clip echo comparison, with the concrete unblock path (the Result bundle. Everything was regenerated in this run against |
sebasmos
left a comment
sebasmos
left a comment
There was a problem hiding this comment.
Read the diff at a4e7b80 against a local clone; 95 tests pass. @kevzho's five items all land. Fairness alignment is correct: fused_probe_loader defaults shuffle=False, so strata match predictions.
Three asks before these reach the paper:
-
ecg_droppedAUROC 0.383, CI 0.279 to 0.493, excludes 0.5. Bins show near-collapse, but inverted ranking is not "nearly uninformative". Say below chance. -
_bootstrap_ciand_paired_deltadiscard non-finite-AUROC replicates uncounted, so each CI rests on an unknown n. Record the discards. -
failure_report.jsoncomes from the ridge harness, not the fused checkpoint. Label it, since #76 wants it in the paper.
The 0.383 EF<=40 AUROC has a 95% interval of 0.279-0.493 that excludes 0.5, so the ranking is inverted rather than uninformative, and MAE barely moves in that condition. Calling it 'nearly uninformative' undersold it: the regression output looks healthy while the gate it feeds is wrong, which is the silent-failure case this repo is about. Say so in the results table, the calibration caveat, and the degradation section.
Both bootstrap paths drop replicates whose resample draws a single EF class, because AUROC is undefined there, but neither said how many -- so every reported AUROC interval had an unknown n behind it. Carry n_bootstrap alongside the kept and discarded counts in the missing-modality and baseline-gap artifacts. The paired delta gets its own count: a replicate only survives when both models have a defined AUROC on it, so it can rest on fewer than the per-model ones. _ci95 now returns None on an empty sample instead of raising inside np.quantile, which is what happened if every replicate went.
The failure report comes from the harness's own Ridge on concatenated embeddings, not from the fused checkpoint the rest of the results score, and the two disagree badly -- drop-echo MAE 117.5 against the checkpoint's 20.55. Nothing in the file or its name said which model produced it, and #76 wants these views in the paper next to Table 4. Emit a provenance block naming the producing model, with the manifest hash and git sha where a real run supplies them, and export the bundle copy as failure_report.ridge.json so the name carries the label too. The committed copy predates the provenance block; #79 replaces it with a fused-checkpoint report.
Two more instances of the layout drift that made the documented command score a stale checkpoint. evaluate_calibration.py wrote calibration.json straight into --out, so running the three conditions overwrote one file, and the bundler reads results/calibration/<cond>/ and found nothing; it nests per condition now. train_ecg_probe.py defaulted to probes/ecg_only while train_probes.py and the eval scripts use probes/ecg, so a checkout that used the standalone runner had the diagnosis script fail to find the probe.
SHA256SUMS was rebuilt from whatever sources happened to exist, so a missing
source dropped the line for a docs/results file that stays committed either way
-- shasum -c then passed over an artifact no run had verified. Those files keep
their line, and the run says which ones it did not regenerate.
Fairness mirrored the full condition to the top level with .get('full', {}),
which no-ops when full is not among the scored conditions: aggregate.py then
rendered an empty fairness table rather than failing. Say why instead.
|
All three are in, plus four fixes I turned up reviewing my own diff. Pushed as five commits ( 1. Below chance. You're right that "nearly uninformative" was the wrong word — the interval excludes 0.5, so the ranking is inverted, not absent. That reading is now in four places: the results table note, the calibration paragraph, and TECHNICAL §7.1 and §7.3. I also took your point about the bins rather than just asserting collapse. 234 of 245 test cases sit in the single 0.1–0.2 bin, where mean confidence Worth stating plainly since it changes the story: 2. Discards. While in there I hit a latent crash — 3. Ridge label. Every failure report carries a #79 is the real fix and it's built: a Four extras — heads up, the diff grew. Two are the same path-drift class @kevzho blocked on, so I didn't want to ship them:
One caveat. I don't have the manifest on this machine, so nothing under Tests 103 → 116, |
3 participants
Summary
Picks up the whole eval backlog against the canonical pooled checkpoint. Fairness now reads the joined manifest and stratifies per missing-modality condition; the Platt scaler fits on val predictions (which the missing-modality eval now persists per condition) instead of in-sample; a new
scripts/diagnose_baseline_gap.pydoes the in-cohort fused-vs-solo comparison with paired bootstrap CIs; the failure harness gained a--manifestroute so its taxonomy/complementarity/dropout artifacts come from real data instead of the synthetic demo; docs restate everything from measured runs and record the pooling + pairing-window decisions where they're consumed (configs, TECHNICAL 11). Also carries the507e802results commit that was pushed after #73 merged and never reached main.Closes #63, closes #64, closes #65, closes #66, closes #67, closes #68, closes #69, closes #71, closes #72.
Result
All from real seed-42 CPU runs on the 1,208-row manifest (Apple M1 Pro, 16 GB; commands in the scripts, artifacts under gitignored
results/):lvef_upperfallback sit at 43.0 MAE — 18 of that fallback's 24 cohort rows carry LVEF = 100.0, which is not a real ejection fraction.Notes
run_cohort_sensitivity.pyis ready for whoever has it — the manifest was built at +/-24h, and within it +/-12h keeps 767 rows, +/-6h keeps 515), and the access checklist needs confirmations from the people themselves.lvef_upperlabels inbuild_cohort.py(excluding them moves test MAE from ~10.4 to ~9.6), and rebuild the paper's failure figures from the realresults/failureartifacts — the case-report.texisn't in this repo so I couldn't fix the "representative seed" caption that currently sits over synthetic demo data.data/private/pr73_kevin— worth a quick confirm that it matches what you have, sinceresults/andprobes/are gitignored and reproduction goes through that file.