Adds NW build scripts. - #1935
Conversation
|
nw build scripts — docs on required env vars / dump layout would save people a round trip. |
Chessing234
left a comment
There was a problem hiding this comment.
two things, one small and one worth changing.
small: the first quickstart block has -f nw/buildnw postgres/load_gz.sql (missing slash) — it's correct in the longer walkthrough further down, but that first one is the copy-pasteable version and it fails.
the one worth changing: postgres_env.sh documents source postgres_env.sh myuser mypassword nw localhost 5432. that puts a database password into shell history and into the process arguments of anything watching. these are PhysioNet-credentialled users, so it's a real credential. read -s -p 'password: ' inside the script, or pointing people at ~/.pgpass, avoids it without costing convenience. the defaults are also worth a thought — sourcing with no arguments exports PGPASSWORD=your_password, which fails with a confusing auth error rather than prompting.
i could not check the expected row counts in validate.sql; i don't have NWICU access, so someone who does should confirm those.
Chessing234
left a comment
There was a problem hiding this comment.
went through these properly against mimic-iv/buildmimic/postgres/, which is what they're modelled on. most of it tracks line for line; three places diverge in ways i think are bugs rather than deliberate adaptation.
prescriptions_pk drops drug_type. mimic-iv has:
ADD CONSTRAINT prescriptions_pk PRIMARY KEY (pharmacy_id, drug_type, drug);this branch has PRIMARY KEY (pharmacy_id, drug). drug_type is in the nw prescriptions table (VARCHAR(100)) and it's in the key upstream for a reason — a single pharmacy order routinely carries a MAIN row and a BASE/ADDITIVE row naming the same drug. if that pattern exists in NWICU at all, constraint.sql aborts after load_gz.sql has already inserted 1.85M prescription rows, and the operator has to work out what happened from a bare unique-violation. i can't check whether NWICU has such rows (no access), but there's no upside to narrowing the key relative to the source it was copied from — i'd restore drug_type.
d_items_pk is widened to (itemid, label), and that costs two foreign keys. mimic-iv uses PRIMARY KEY (itemid). adding label means the constraint no longer guarantees itemid is unique, which is the one thing anything joining d_items actually relies on — including the chartevents_to_* and procedureevents_to_* maps in #1930, which key on itemid alone.
it also explains a gap i'd otherwise have read as an oversight: mimic-iv declares chartevents_d_items_fk and procedureevents_d_items_fk, and neither exists here. they can't — a single-column FK on itemid has nothing to reference once the PK is a pair. so the widened PK and the two missing FKs are the same decision. if there really are duplicate itemids in nw_icu.d_items that's worth stating explicitly in the readme, because it's a meaningful difference from MIMIC; if there aren't, narrowing the PK back to itemid gets both FKs back.
the readme's quickstart only sets ON_ERROR_STOP=1 on the load step. mimic-iv's readme sets it on load, constraint and index:
psql -d mimiciv -v ON_ERROR_STOP=1 ... -f constraint.sql
psql -d mimiciv -v ON_ERROR_STOP=1 ... -f index.sqlhere create.sql, constraint.sql, index.sql and validate.sql all run without it, so psql prints the error, keeps going, and exits 0. combined with the first point that's the difference between "the build failed on the prescriptions key" and "the build looked fine and the key silently isn't there". this one's a one-line fix and worth doing regardless of the other two.
smaller things:
constraint.sqladdsdiagnoses_icd_patients_fkanddiagnoses_icd_admissions_fktwice — once in the primary-key section (lines 34/40) and again in the foreign-key section (112/118). theDROP CONSTRAINT IF EXISTSin front makes it harmless, but it reads as an editing leftover.create.sqlmakesnw_hospandnw_icubut nonw_derived, where mimic-iv's makesmimiciv_derivedalongside the two. #2000 addsnw/concepts_postgres/, so something is going to need that schema.- 28
CREATE INDEXhere against mimic-iv's 43. mostly that's fewer tables, butlabeventsgains anitemidindex that mimic doesn't have whilecharteventsis(charttime, itemid)where mimic is(charttime, storetime). if those are deliberate tuning choices for NWICU's size, a comment saying so would stop the next person "fixing" them back. validate.sqljoins expected to observed withINNER JOIN, same as mimic-iv, so a table missing from either side silently drops out of the report instead of failing. inherited rather than introduced, but a build script whose failure mode is a shorter result set is worth anORDER BYsanity count at least.
my two earlier notes still stand: the -f nw/buildnw postgres/load_gz.sql typo in the first quickstart block (missing slash), and postgres_env.sh taking the password as a positional argument.
as before, i have no NWICU access, so the expected row counts in validate.sql and whether the data actually violates the prescriptions key both need someone who does.
This pull request adds PostgreSQL build scripts and documentation for the Northwestern ICU (NWICU) dataset on PhysioNet.
The following files are added under
nw/buildnw/postgres/:README.md– setup and usage instructionscreate.sql– schema creation scriptload_gz.sql– load compressed CSV dataconstraint.sql– primary/foreign key constraintsindex.sql– indexes for performancevalidate.sql– validation checkspostgres_env.sh– environment variable helper