search

National-COVID-Cohort-Collaborative / Phenotype_Data_Acquisition

The repository for code and documentation produced by the N3C Phenotype and Data Acquisition workstream
60 stars 35 forks source link

Phenotype 3.1: OMOP birth_datetime breaks phenotype script #185

Closed mgkahn closed 3 years ago

mgkahn commented 3 years ago

As I predicted, the change from using CDM-required OMOP fields (month_of_birth, year_of_birth) to an optional field (birth_datetime) breaks OMOP instances that do not fill in this optional field. Put Colorado into that bucket.

If you feel you must allow for the use of an optional field rather than helping OMOP submissions conform to OMOP CDM requirements, consider first attempting to use a required field and only if the required field is null attempt to use the optional field. I do not know if SQLRender supports the coalesce() function which would be the easiest SQL construct to use.

kmkostka commented 3 years ago

Thank you for reporting this issue. Our release unit test was inadvertently biased to an organization that does populate both fields. We appreciate you catching this. We will patch this and make a modification this week.

mgkahn commented 3 years ago

We do not populate this field because we do not feel our source system provides meaningful TIMESTAMP information, which is the only additional information one gets from a datetime field versus the Y-M-D breakdown in the required fields. We try not to hallucinate data or imply false accuracy when it does not exist (feeds into the angst about DT fields in CDM V6.x discussions). For the moment, I have violated this in the N3C-specific data mart by implying a D=1; H=M=S=0 in creating a pseudo-fictitious birth_datetime so I can move forward on testing the new script.

empfff commented 3 years ago

Michael--we just patched this. We had actually done this as a coalesce, but neglected to SQLRender it to the GBQ version. (The coalesce was only in the MSSQL version.) The new version of the GBQ script should be good to go, let us know if you find other issues.

mgkahn commented 3 years ago

Pulled down the new commit and see the change. IFNULL() was a much superior construct! Thank you for making this change. I'll take down the fake birth_datetime to give the conformant CDM a test drive.