fix(mimic-iv): add missing temperature itemids to vitalsign concept - #2042
fix(mimic-iv): add missing temperature itemids to vitalsign concept#2042acnimma wants to merge 1 commit into
Conversation
The vitalsign concept omitted blood/CCO and Arctic Sun/Alsius temperature itemids documented in MIT-LCP#1358. Include 226329, 227632, and 227634 in the Celsius temperature CASE, and 227630/227631 in temperature_site alongside 224642. Regenerate postgres and duckdb dialect copies from the BigQuery source. Fixes MIT-LCP#1358
|
missing temp itemids are easy to miss. can you list the added itemids in the pr body for reviewers? |
|
Listed the added itemids in the PR body for reviewers:
All five are in the BigQuery source and the regenerated postgres/duckdb copies. |
Chessing234
left a comment
There was a problem hiding this comment.
the two site itemids look wrong to me. 227630/227631 are arctic sun probe locations for the cooling device, so folding them into MAX(CASE WHEN itemid IN (224642, 227630, 227631) THEN value END) AS temperature_site puts two different meanings in one column. at a charttime where both are charted the MAX just picks whichever string sorts higher, so temperature_site can end up naming a device probe rather than the site of the 223761/223762 reading sitting next to it. that also silently changes what #2046's LOWER(temperature_site) LIKE '%axillary%' matches, and that's your PR too. could you keep temperature_site on 224642 and give the device probes their own column?
226329 was deliberately commented out on main, so uncommenting it needs a reason in the description. blood/CCO is a continuous core temperature on a small cardiac subset, and averaging it into the same per-charttime AVG as oral/axillary moves vitalsign.temperature for those stays. that propagates through first_day_vitalsign.temperature_min/max into apsiii, sofa, sapsii, oasis and sirs. can you show the demo numbers — how many stays change and how far min/max move?
separately the postgres copy doesn't reproduce. regenerating from this branch's bigquery source with the pinned sqlglot==30.11.0 from requirements-lock.txt gives a 6-line diff against what's committed — the header comment block is three lines here and one line from the pinned transpiler. main reproduces byte-identically and the duckdb copy matches, so it looks like it was generated with a different sqlglot.
Chessing234
left a comment
There was a problem hiding this comment.
thanks for listing the itemids — that made this much easier to check. a few things came out of it.
first the mechanical one: the postgres copy isn't transpiler output. running the repo's transpiler (sqlglot 30.11.0 per requirements-lock) over your bigquery source and diffing against mimic-iv/concepts_postgres/measurement/vitalsign.sql on this branch, sqlglot emits the header comment as one line and the branch has it split across three. duckdb matches exactly; the same check against main is identical, so the drift is from this branch. that folder is checked by the generated-up-to-date job and the file header says not to edit directly.
the substantive ones:
226329 was not missing — it's present on main at line 94 of the bigquery source, deliberately commented out (-- 226329 -- Blood Temperature CCO (C)). uncommenting an exclusion someone made on purpose needs a reason in the pr body; blood temperature off a CCO/PA catheter is a different measurement site and modality from a nursing-charted temperature, which is presumably why it was left out.
merging the probes into one column via MAX(...) worries me more. previously temperature had a single source itemid so there was no conflict to resolve. a patient on an Arctic Sun can now have 223762, 227632 and 227634 at the same charttime, and MAX silently reports the warmest. for targeted temperature management that's the opposite of what you'd want — the whole point is tracking the cooling probe. same question for temperature_site: 224642 is "Temperature Site" but 227630/227631 are "Arctic Sun Temp #1/#2 Location", and MAX over text just picks whichever sorts last, so a charttime with both a nursing site and a probe location returns one arbitrarily.
could you show the counts — how many charttimes end up with more than one of these temperature itemids populated, and how many with both a 224642 value and a 227630/227631 value? if that's common then i think these need to be separate columns rather than folded into temperature/temperature_site.
Summary
vitalsign.sql(Celsius CASE + site)Added itemids (for reviewers)
temperature_sitetemperature_siteExisting
223762/223761(C/F) and224642(site) are unchanged.Why
The vitalsign concept omitted blood/CCO and Arctic Sun/Alsius temperature itemids documented in #1358.
Test plan
pytest tests/test_transpile.pypasses locallyFixes #1358