pharmaverse / admiralpeds

Admiral Package Extension for Pediatric Clinical Trials
https://pharmaverse.github.io/admiralpeds/
Apache License 2.0
13 stars 3 forks source link

Closes #59 derive params growth age add interpolation #60

Closed zdz2101 closed 2 months ago

zdz2101 commented 3 months ago

Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral codebase remains robust and consistent.

Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the main branch until you have checked off each task.

github-actions[bot] commented 3 months ago

Code Coverage

Package Line Rate Health
admiralpeds 78%
Summary 78% (119 / 152)
CDC-DNPAO commented 3 months ago

Zelos - I think the WHO weight for length file gives lengths in intervals of 0.5 cm. If you want, I can generate an interpolated file that has lengths in intervals of 0.1 cm. Just let me know. And I might be wrong about the 0.5 cm intervals.

david

On Tue, Jul 2, 2024 at 3:55 PM Zelos Zhu @.***> wrote:

@.**** commented on this pull request.

@rossfarrugia https://github.com/rossfarrugia I think this should fix some things, need to check with @CDC-DNPAO https://github.com/CDC-DNPAO's files to be absolutely sure

In R/derive_params_growth_age.R https://github.com/pharmaverse/admiralpeds/pull/60#discussion_r1663084031 :

@@ -236,7 +238,10 @@ derive_params_growth_age <- function(dataset, by = c("sex_join", "ageu_join"), relationship = "many-to-many" ) %>%

  • filter(prev_age <= {{ age }} & {{ age }} < next_age)
  • mutate(age_diff := abs(metadata_age - {{ age }})) %>%

@rossfarrugia https://github.com/rossfarrugia I think this should do the trick, logic is as follows:

  1. calculate difference between current age provided, and the age provided in metadata (absolute value)
  2. calculate which record contains the smallest/minimum difference so in this case 25.49 should map to 25.5 not 24.5
  3. filter by that record only

In tests/testthat/test-derive_params_growth_age.R https://github.com/pharmaverse/admiralpeds/pull/60#discussion_r1663084591 :

@@ -4,18 +4,18 @@ test_that("derive_params_growth_age Test 1: derive_params_growth_age works", { vs_data <- tibble::tribble( ~USUBJID, ~SEX, ~AGECUR, ~AGEU, ~VSTESTCD, ~VSSTRESN,

  • "1001", "M", 30, "days", "WEIGHT", 10,
  • "1002", "F", 25, "months", "WEIGHT", 100,
  • "1001", "F", 24.5, "months", "WEIGHT", 10,
  • "1002", "F", 25.49, "months", "WEIGHT", 11,

demo'd here with fake metadata but showcases that it does indeed work

In R/derive_params_growth_height.R https://github.com/pharmaverse/admiralpeds/pull/60#discussion_r1663085137 :

@@ -263,7 +269,11 @@ derive_params_growth_height <- function(dataset, by = c("sex_join", "heightu_join"), relationship = "many-to-many" ) %>%

  • filter(prev_height <= {{ height }} & {{ height }} < next_height)
  • mutate(ht_diff := abs(meta_height - {{ height }})) %>%

same thing needed to be done in case decimals are involved in height/lenghts

— Reply to this email directly, view it on GitHub https://github.com/pharmaverse/admiralpeds/pull/60#pullrequestreview-2154732889, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY2XGPUF44GDPLSYODZMAEDZKMAS3AVCNFSM6AAAAABKIFEVJ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNJUG4ZTEOBYHE . You are receiving this because you were mentioned.Message ID: @.***>

rossfarrugia commented 3 months ago

Zelos - I think the WHO weight for length file gives lengths in intervals of 0.5 cm. If you want, I can generate an interpolated file that has lengths in intervals of 0.1 cm. Just let me know. And I might be wrong about the 0.5 cm intervals. david

Looks to me like they're already by 0.1cm so we should be ok, see https://github.com/pharmaverse/admiralpeds/blob/main/data-raw/who_wt_for_ht_boys.R and https://github.com/pharmaverse/admiralpeds/blob/main/data-raw/who_wt_for_lgth_boys.R.

CDC-DNPAO commented 3 months ago

You're right. I'm less familiar with the WHO files and must have clicked on the wrong one. Thanks

On Wed, Jul 3, 2024 at 4:20 AM Ross Farrugia @.***> wrote:

Zelos - I think the WHO weight for length file gives lengths in intervals of 0.5 cm. If you want, I can generate an interpolated file that has lengths in intervals of 0.1 cm. Just let me know. And I might be wrong about the 0.5 cm intervals. david

Looks to me like they're already by 0.1cm so we should be ok, see https://github.com/pharmaverse/admiralpeds/blob/main/data-raw/who_wt_for_ht_boys.R and https://github.com/pharmaverse/admiralpeds/blob/main/data-raw/who_wt_for_lgth_boys.R .

— Reply to this email directly, view it on GitHub https://github.com/pharmaverse/admiralpeds/pull/60#issuecomment-2205392080, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY2XGPWLZGRFA6UODX4CA7DZKOX5LAVCNFSM6AAAAABKIFEVJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBVGM4TEMBYGA . You are receiving this because you were mentioned.Message ID: @.***>

CDC-DNPAO commented 3 months ago

What happens if you have an age in days of 1035 and you want the z (SD) score for BMI from the CDC reference? This child is 34.0 months, so are you converting this to 23.5 months to get the LMS parameters from the online CDC data files? If so, I think a better solution is to keep the age in days and use my interpolated agedays CSV file so that the LMS parameters match those for a child who is exactly 1035 days of age.

On Wed, Jul 3, 2024 at 12:56 PM Zelos Zhu @.***> wrote:

@.**** commented on this pull request.

In R/derive_params_growth_age.R https://github.com/pharmaverse/admiralpeds/pull/60#discussion_r1664495419 :

@@ -236,7 +238,10 @@ derive_params_growth_age <- function(dataset, by = c("sex_join", "ageu_join"), relationship = "many-to-many" ) %>%

  • filter(prev_age <= {{ age }} & {{ age }} < next_age)
  • mutate(age_diff := abs(metadata_age - {{ age }})) %>%

@rossfarrugia https://github.com/rossfarrugia The function was written to be unit-agnostic, as long as the input dataset's age units and the metadata's age units are matching , e.g. if you calculate current age is in months and you only have >= 2 yr olds in the study you don't need to modify the CDC data because it is already in months, if the current age calculation is in days and you are only working with the WHO criteria, or if you had some sort of hybridized columns of numeric of days and months as long as you have the appropriate metadata data with the correct range of days and months it should join fine

— Reply to this email directly, view it on GitHub https://github.com/pharmaverse/admiralpeds/pull/60#discussion_r1664495419, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY2XGPRZFSO7WNTZLBNOVH3ZKQUMHAVCNFSM6AAAAABKIFEVJ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNJWHE2DQNZSGQ . You are receiving this because you were mentioned.Message ID: @.***>

zdz2101 commented 3 months ago

What happens if you have an age in days of 1035 and you want the z (SD) score for BMI from the CDC reference? This child is 34.0 months, so are you converting this to 23.5 months to get the LMS parameters from the online CDC data files? If so, I think a better solution is to keep the age in days and use my interpolated agedays CSV file so that the LMS parameters match those for a child who is exactly 1035 days of age. On Wed, Jul 3, 2024 at 12:56 PM Zelos Zhu @.> wrote: @*.*** commented on this pull request. ------------------------------ In R/derive_params_growth_age.R <#60 (comment)> : > @@ -236,7 +238,10 @@ derive_params_growth_age <- function(dataset, by = c("sex_join", "ageu_join"), relationship = "many-to-many" ) %>% - filter(prev_age <= {{ age }} & {{ age }} < next_age) + mutate(age_diff := abs(metadata_age - {{ age }})) %>% @rossfarrugia https://github.com/rossfarrugia The function was written to be unit-agnostic, as long as the input dataset's age units and the metadata's age units are matching , e.g. if you calculate current age is in months and you only have >= 2 yr olds in the study you don't need to modify the CDC data because it is already in months, if the current age calculation is in days and you are only working with the WHO criteria, or if you had some sort of hybridized columns of numeric of days and months as long as you have the appropriate metadata data with the correct range of days and months it should join fine — Reply to this email directly, view it on GitHub <#60 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY2XGPRZFSO7WNTZLBNOVH3ZKQUMHAVCNFSM6AAAAABKIFEVJ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNJWHE2DQNZSGQ . You are receiving this because you were mentioned.Message ID: @.>

It would depend on the metadata file the user provides, either:

  1. User provides current age in days in their input dataset and modifies the CDC metadata to days instead of months: so input dataset would have 1035 days, cdc metadata would have L/M/S values available for 33.5/34.5 months respectively but transformed to, 33.5 x 30.4375 = 1019.656, 34.5 x 30.4375 = 1050.094 the "smaller" difference is the 34.5 value, so should match L/M/S values for 34.5 months record

  2. User provides current age in months, 1035/30.4375 = 34.00411 and leaves the CDC metadata as is in months and it should match with the L/M/S values for 34.5 months, same result as scenario 1

  3. User provides current age in days and leaves the CDC metadata as is in months and it should not match with any L/M/S values

CDC-DNPAO commented 3 months ago

1 and #2 are going to result in an age difference (exact age of child minus reference data age) of about 15 days, which for the height and weight of young children can make a substantial difference in the calculated z-score

zdz2101 commented 3 months ago

1 and #2 are going to result in an age difference (exact age of child minus reference data age) of about 15 days, which for the height and weight of young children can make a substantial difference in the calculated z-score

I believe this is what @rossfarrugia was mentioning about moving the interpolation/how to do it into the template/user modified metadata not necessarily the function core itself

rossfarrugia commented 2 months ago

@zdz2101 CI fixes needed for styler and lintr - adding by_vars also breaks the template and vignette but me and @Fanny-Gautier are already fixing that in our branches/PRs so we could ignore here the templates and CMD checks

zdz2101 commented 2 months ago

@rossfarrugia @Fanny-Gautier I've added the by_vars parts to the vignette and template in this PR to match what fanny already had in the templates from #62 to reduce merge conflicts and get the CI/CD going properly here and merged in the tests from #54