ML4H merge: tranche 1 - #1185
Open
will-pang wants to merge 16 commits into
Open
Conversation
Empty notes, labs, CXR, and ICD are now zero-length sequences instead of a constant [MISSING_TEXT] row, black image, or pad visit. The tokenizer crash on empty batches is handled by constructing empty tensors, so BERT cannot treat note presence as a free mortality feature. Co-authored-by: Cursor <cursoragent@cursor.com>
nn.Module.train() re-enables dropout inside Bio_ClinicalBERT even when every weight has requires_grad=False. Pin those encoders back to eval so a frozen note embedding is deterministic across steps. Co-authored-by: Cursor <cursoragent@cursor.com>
A frozen BERT forward is identical across epochs, but batch padding width changes every shuffle, so a key over the full padded row never hits. Hash only the attended tokens so the same note reuses its [CLS] vector. Co-authored-by: Cursor <cursoragent@cursor.com>
a0f1422 deleted _scan_table/_scan_parquet while MEDSDataset._subset_patient_ids still calls _scan_parquet, so MEDS loads crashed. load_table routes through _scan_table again, and resolve_table_path keeps absolute cache paths. The test reads a real two-row parquet file. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
Pushed four proven repairs onto this branch (fast-forward from
Still local, not on this PR yet: pad-mask / fp16 fill, nested |
The explicit path filled padded scores with -1e9, which is outside the fp16 range, so AMP overflowed. Ordinary forwards now use fused scaled_dot_product_attention; the explicit path stays behind register_hook for interpretability and fills with finfo(dtype).min. The test checks a padded fp16 pass stays finite and that -1e9 still overflows. Co-authored-by: Cursor <cursoragent@cursor.com>
The collator padded short samples with 0.0 and nothing marked the extra slots, so they looked like real measurements at admission time. It now emits {field}__pad_mask, the unified heads thread it through, and RNN packed lengths clamp at 1 so an all-pad sample does not crash. Tests check the collate mask, pad-last sort, and an all-pad RNN step.
Co-authored-by: Cursor <cursoragent@cursor.com>
NestedSequenceProcessor used padding_idx=None so a fake empty visit could have a non-zero vector. Index 0 then received gradients. Empty visits are now zero events, so the pad row stays frozen zeros. The test checks both the zeros and a zero gradient on that row. Co-authored-by: Cursor <cursoragent@cursor.com>
The sunlab loader required a directory named images and wrote the derived CSV into the PhysioNet root, which is read-only on the cluster. Both images and resized_images are accepted, cache is tried first, and the generated YAML points at the absolute CSV. The test chmods the root to 555 and checks the CSV lands in cache. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
Pushed four more proven repairs (fast-forward
Repro for all four: 12 passed on this checkout. |
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
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.
A first pass at consolidating all the work we did so that we can merge to main. I'll probably need everyone's help in reviewing this (@joshuasteier, @Rian354, @joshuxchn, as well as @jhnwu3), but in short the updates here add in unified embedding support and tested across the different backbones (RNN, transformer, bottleneck
transformer, EHRMamba, JambaEHR) that we're interested in presenting results. In terms of modality, this focuses only on labs, labs + notes, labs + notes + cxr.
Three big updates made
What to remove
pyhealth/scripts/which I've used to submit to condor and can be removed.