Skip to content

Towards Unified Sequence Representation V: Add MIMIC-IV YAIB example for RNN_V2 - #1354

Merged
Logiquo merged 2 commits into
masterfrom
feat/seq-v2-5
Nov 2, 2026
Merged

Logiquo merged 2 commits into
masterfrom
feat/seq-v2-5

Conversation

@Logiquo

@Logiquo Logiquo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Fifth PR in the stack toward a unified sequence representation, on top of #1353. Example only; no library changes.

examples/yaib_mimic4_rnn_v2.py trains a separate RNN_V2 on each of the five YAIB MIMIC-IV tasks (mortality, kidney function, length of stay, AKI, sepsis). It writes checkpoints, logs, the configuration and test metrics. --check verifies the data files and the device without preprocessing or training.

This is a training example using PyHealth's YAIB task definitions, not a reproduction of the original benchmark's tuning and cross-validation protocol.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Test Coverage

Total: 75.14% (21431 / 28522 lines)

No modified files under pyhealth/.

Workflow run · 988b7c4

@Logiquo
Logiquo removed this pull request from stack #1339 November 1, 2026 09:45
@Logiquo
Logiquo added this pull request to stack #1359 November 1, 2026 09:45
@Logiquo
Logiquo force-pushed the feat/seq-v2-5 branch 3 times, most recently from b78d7f1 to e1e57f4 Compare November 1, 2026 11:09
@Logiquo Logiquo changed the title Towards Unified Sequence Representation V: Add MIMIC-IV YAIB example for Unified_RNN Towards Unified Sequence Representation V: Add MIMIC-IV YAIB example for RNN_V2 Nov 1, 2026
@Logiquo
Logiquo force-pushed the feat/seq-v2-5 branch 2 times, most recently from 17a525d to 1ae0e3d Compare November 1, 2026 21:52
Base automatically changed from feat/seq-v2-4 to master November 1, 2026 23:59
examples/yaib_mimic4_unified_rnn.py trains a separate Unified_RNN on each
of the five YAIB MIMIC-IV tasks (mortality, kidney function, length of
stay, AKI and sepsis) and writes checkpoints, logs, the configuration and
test metrics. --check verifies the data files and the device without
preprocessing or training.
@Logiquo
Logiquo marked this pull request as ready for review November 1, 2026 23:59
@Logiquo Logiquo added the status: automatic The PR will go through automatic review process. label Nov 1, 2026
@Logiquo

Logiquo commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 988b7c4

}
REPO_ROOT = Path(__file__).resolve().parents[1]
TABLES = {
"hosp": ["patients", "admissions", "labevents", "transfers", "microbiologyevents"],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] --check does not verify hosp/d_labitems.csv.gz, which preprocessing requires

The labevents config in pyhealth/datasets/configs/mimic4_ehr.yaml joins hosp/d_labitems.csv.gz on itemid with an inner join. BaseDataset loads that join file through _scan_table (pyhealth/datasets/base_dataset.py ~line 881), which raises FileNotFoundError when it is missing. preflight checks only the tables listed here, so --check reports success on a root that will then fail during preprocessing. The docstring says --check verifies the data files. Fix: add "d_labitems" to the "hosp" list. Also consider adding d_items for chartevents if that join is added to the config later.

parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument(
"--root",
default="D:/physionet.org/files/mimiciv/3.1",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] Default paths are one developer's Windows layout

The default --root (and --cache-dir at line 180, and the .venv/Scripts/python.exe invocation in the module docstring) hard-code D:/physionet.org/... and a Windows venv path. On other machines the script only fails later with a missing-file error. Consider making --root required, or documenting it in the docstring without a personal default.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review

suggestion: approve
decision: approve

Thank you for this contribution. The new example script trains a separate RNN_V2 on each of the five YAIB MIMIC-IV tasks. It includes a --check preflight and writes checkpoints, logs, config and test metrics. It only adds an example and changes no library code. The split is done before fitting the preprocessing statistics, which is the right order. It looks ready to merge.

Coverage: the PR changes no files under pyhealth/, so there is no per-file coverage to judge. Whole-repo coverage is 75.14%, which is above the 60% bar. Example scripts that need real MIMIC data are not normally unit-tested, so I don't see this as a problem.

Optional suggestions:

  • --check is documented as verifying the data files, but TABLES doesn't include hosp/d_labitems.csv.gz. The labevents config joins that file, so a machine missing it would pass --check and then fail during preprocessing. Adding it to the hosp list would close the gap.
  • The default --root and --cache-dir and the docstring invocation (D:/..., .venv/Scripts/python.exe) assume one Windows layout. Platform-neutral defaults, or a required --root, and a plain python examples/... invocation in the docstring would be easier for other users.

Reviewed commit 988b7c4

@Logiquo
Logiquo merged commit 2ea75c2 into master Nov 2, 2026
2 checks passed
@Logiquo
Logiquo deleted the feat/seq-v2-5 branch November 2, 2026 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: automatic The PR will go through automatic review process.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant