Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AUMC difftime calculation #21

Closed
wants to merge 8 commits into from

Conversation

prockenschaub
Copy link
Collaborator

This is a fix for the issue described in #20.

The current solution is mainly copy-paste of load_mihi with some small changes. This could/should be refactored to avoid code duplication, perhaps with a single function that can deal with all databases.

Note: this currently needs to remove a warning from load_concepts.itm, which may not be desired. Maybe there is another way to prevent unnecessary warnings in the case of additional arguments to load_concepts?
load_mihi, load_au, and load_ei only differ in the rounding function they apply (and the fact that eicu strictly speaking doesn't require merging of origin). They can thus all be replaced with a single function that receives the rounding function as a parameter.
@prockenschaub prockenschaub marked this pull request as ready for review October 11, 2023 20:40
@prockenschaub
Copy link
Collaborator Author

After closer inspection of load_mihi and load_eiau (split into load_ei and load_au by one of the commits), it turned out that they only differ in the rounding function they apply. They can therefore be replace by a single function do_load_difftime that receives the rounding function as a paramter.

Technically, do_load_difftime unnecessarily merges the origin for eICU. However, the time overhead does not seem too bad and the simplifcation is worth it IMO.

@prockenschaub prockenschaub requested a review from nbenn October 11, 2023 20:43
@prockenschaub
Copy link
Collaborator Author

Note that this PR branches of #19 , which should be merged before this one.

@prockenschaub
Copy link
Collaborator Author

Closed in favour of #51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AUMC] Error in difftimes for second ICU stays
1 participant