When reviewing {cdcanthro} and {whoanthro} what could we re-use and where should we make adaptations for {admiralpeds}?

[rossfarrugia]

Links: https://github.com/CDC-DNPAO/CDCAnthro & https://github.com/CDC-DNPAO/WHOanthro

Examples to consider:

• Admiral programming conventions – such as function naming and use of tidyverse​
• Modularising the functions into separate R scripts with roxygen2 headers & pkgdown site​
• Ensuring robust testing (code coverage) and CI/CD​
• CRAN submission (and companies only needing to adopt a single package)

[rossfarrugia]

Idea from team call: standardize the input/output of functions

[santarabantoosoo]

Thoughts from the package WHOanthro

General thoughts

1. Input validation:
   I think more validation checks may be needed(eg. weight within acceptable range)

2. Specify unit of measurement:
   Instead of asking the user to abide to Weight is in kg, and lenhei is length or height in cm. BMI is kg/m^2.

3. Autoconversion:

Instead of: If age is in completed number of months, multiply by 30.4375 and add 15.219 (1/2 of 30.4375). Then take the floor of this value. Age in the WHO reference data is given as 0 to 1826 days.

Apply the conversion automatically within the package. The user will choose from months and days (days would be the default value)

4. Adding labels to variables in output:

For example, waz: weight-for-sex/age z-score

we can add labels for the columns using labelled package

5. Required columns:

need to have all variables in data; agedays,wt,lenhei,headc, and bmi

why is this a must ? I thought that age and sex only were necessary.

what is nice to keep?

1. sex can be coded almost any way

  # sex can be coded almost any way
   data[,sexn:=toupper(substr(sex,1,1))]
   data[,sexn:=fcase(
      sexn %in% c(1,'B','M'), 1L,
      sexn %in% c(2,'G','F'), 2L
   )]

However, I am afraid that 1 and 2 encodings might be misinterpreted

2. w_zscore function:

w_zscore=function(var, l, m, s){ 
   invl=1/l
   z = (((var/m) ^ l) -1) / (l*s) # z-score formula
   sdp2 = (m * (1 + 2*l*s) ^ (invl)) 
   sdp3 = (m * (1 + 3*l*s) ^ (invl))
   sdm2 = (m * (1 - 2*l*s) ^ (invl)) 
   sdm3 = (m * (1 - 3*l*s) ^ (invl))
   z=fcase(
      z >= -3 & z < 3, z,
      z > 3, 3 + (var - sdp3)/(sdp3 - sdp2),
      z < (-3), -3 - abs((var - sdm3)/(sdm2 - sdm3))
   )
   list(z, sdp2, sdp3, sdm2,sdm3)
}

3. Getting reference data:

dref1 <- who_ref_data[(denom=='forage' & agedays<1857) | denom=='forlen']
   # setkey(data,sexn,agedays); setkey(dref1,sexn,agedays)
   dt1 <- dref1[data, on=c('sexn','agedays'),nomatch=0];

what to refactor?:

1. first 5 helper functions:

I think these would be converted to tidyverse conventions

.c , fnames, fdel, fvalid, set_cols_first

2. whoanthro main function:

It has the validation, filtering and joining. I think this will be refactored to use tidyverse and Adam

Please let me know if any of these suggestions make sense

[rossfarrugia]

Thanks Mahmoud (@santarabantoosoo) for the thorough review and comments - much appreciated! 👍

Some replies that might help on the general thoughts above:

• Input validation: Here we have the strategy of checking for sensible user inputs via assertion checks (https://pharmaverse.github.io/admiraldev/articles/programming_strategy.html#input-checking) rather than checking the input data itself. There's another pharmaverse package (https://pharmaverse.github.io/sdtmchecks/) that checks plausibility of input data.
• Units: We have the benefit in ADaM creation of having well standardized input data via CDISC SDTM data model, so there will be a unit variable that comes in the input data along with the value that we could rely on.
• Autoconversion: Agree we can cover conversion of units within the package. There's an admiral function we can use for this: https://pharmaverse.github.io/admiral/reference/derive_vars_duration.html.
• Labels: These are covered by another pharmaverse package (xportr) which takes them direct from user specifications - see this ADSL example: https://pharmaverse.github.io/examples/adam/adsl.html.
• Required columns: I defer to @CDC-DNPAO to answer your question above.

[santarabantoosoo]

Thanks a lot for the thorough reply.

I will look into these resources and hopefully I would be able to work on one of these issues when the programming part starts.

[CDC-DNPAO]

Mahmoud (@santarabantoosoo), I agree that the 0/1 and 1/2 coding for sex could be confusing. The whoanthro program contains this because a pediatrician, who was new at R, asked me to include both codings so she wouldn't have to change her programs. A user even asked me to let the 'sex' variable be named 'sex_code' and (I think) I accommodated this request. I'm a pushover.

About the required columns: These functions calculate z-scores and percentiles for weight, length, etc, relative to a standard (CDC or WHO) population. If only sex and age are in the input dataset, none of the z-scores can be calculated - they will all be NA. There could easily be some rows that are missing weight, height, or head circumference - the calculated z-scores and percentiles will be NA. But if the entire data frame contains only values for sex and age, there's no point in using whoanthro. That's what the clumsily worded phrase "need to have all variables in data; agedays, wt, lenhei, headc, and bmi" means.

[santarabantoosoo]

@CDC-DNPAO Thank you so much for clarifying this. Much clearer now :)

[Pierre-Wallet]

A few comments to start this review. Other comments will be added.

• .c() seems to be used to convert a vector of objects into a vector of characters. Maybe use as.character(alist(...)) instead, easier to read. Or maybe {dplyr} style will make it useless.
• fnames() and fdel() are not used anywhere.
• set_col_first() seems to be used to reorder columns. Again, to see if it is worth to keep it with {dplyr} style
• Where does who_ref_data dataset come from? Is it playing the role of the metadataset that we just created from WHO and CDC websites?

[rossfarrugia]

thanks Pierre! for the last bullet, that's my interpretation too and we'd use instead what you and Zelos are preparing here for our package metadata

[CDC-DNPAO]

Piere, About 'who_ref_data' - it's just the various WHO reference datasets made into 1 'complete' dataset - the columns are the LMS values for wt, ht, headcirc, bmi, etc. Boys and girls are rbinded together set_col_first reorders column names and I think it comes from the hutils package, .c() is to get rid of having to put variable names in quotation marks; I think this is the 'Cs' function in the Hmisc package. I'm not surprised that I have functions (fnames and fdel) that are not used anywhere david

…

On Thu, Mar 14, 2024 at 7:07 AM Pierre-Wallet ***@***.***> wrote: A few comments to start this review. Other comments will be added. - .c() seems to be used to convert a vector of objects into a vector of characters. Maybe use as.character(alist(...)) instead, easier to read. Or maybe dplyr style will make it useless. - fnames() and fdel() are not used anywhere. - set_col_first seems to be used to reorder columns. Again, to see if it is worth to keep it with {dplyr} style - Where does who_ref_data dataset come from? Is it playing the role of the metadataset that we just created from WHO and CDC websites? — Reply to this email directly, view it on GitHub <#5 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AY2XGPW4FRIXJRC5YY22DHDYYGAHJAVCNFSM6AAAAABDEQCPPKVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4DOOBUHAYTM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[zdz2101]

It may be time to revisit this discussion once again, and begin thinking about how we want to design our functions in a way that are user-friendly/the "admiral" way with some of the above thoughts take into consideration

The repo that @ngiangre shared in the slack can certainly provide some inspiration: https://lshtm-gigs.github.io/gigs/index.html

I am still gathering/looking through myself and will have thoughts on this matter soon

[ngiangre]

Here's my notes looking at {gigs}:

For a function ig_nbs_value2zscore that generates a number as in:

2 things are happening: 1) The function body first calls validate_ig_nbs with all the arguments, returning a list that verified the values are valid according to standards implemented in other functions 2) The function body's second call is to ig_nbs_v2c_internal which calls a) ig_nbs_wlrfga_v2p which calls many internal functions that I can't trace easily...

I think the main takeaway is that there is a russian nesting doll technique in use, where inner functions verify accurate inputs per the standards/data model and then for running mathematical operations like z scores. There's a lot of hard coded variables within these, just to note.

Instead of this technique, we may want to consider OOP like using an R6 object or S3 methods. Especially if there's a large nesting structure for possible inputs and outputs. Just a thought.

I hope this is useful for takeaways for us to ingest and adapt our strategy!

[rossfarrugia]

thanks Nick for investigating! i think we can definitely keep it simpler in our package in line with the admiral manifesto: https://pharmaverse.github.io/admiral/#admiral-manifesto. in our admiral parameter derivation functions we do have assertion checks we use for verifying the input data and user argument selections are appropriate, but we generally use a functional workflow with clear inputs and outputs. we do use OOP sometimes in admiral such as when you need to construct parameters based on some rule sets such as for time to event: https://pharmaverse.github.io/admiral/articles/bds_tte.html#pre-defined-time-to-event-source-objects. i'm always conscious of making things easy for our users to trace though, which sounds like something you struggled with in that package and definitely something we need to concentrate on in admiralpeds.

[ngiangre]

I think the designing will be much easier if we have a tree diagram of all possible inputs and outputs. Or at least good estimates. This could change over time, but it would help guide development in following the manifesto's principles.

[rossfarrugia]

yes absolutely, hoping @zdz2101 can share with us a first draft of the initial outline for our ADVS with the growth development parameters, and from there we all can review and consider whether we see any different inputs/outputs/functions/arguments needed. always easier if someone from admiral core dev team does this first as they know the admiral dev principles best.

[CDC-DNPAO]

My 2 cents: I think the ig_nbs_value2zscore function is the wrong approach - it seems that the user has to type in values for 1 person and then gets a z-score back. As a user of the CDC and WHO functions (and based on talks with other users), over 99.9% of the time users are going to want to feed a dataset / dataframe to the function and get back the same dataset with some additional columns for z-scores and percentiles. If you do want something like what is shown in the screenshot, the user could make a 1-row dataframe and feed it to the function.

[ngiangre]

Sounds great, thank you! Cheers, Nick Giangreco, PhD nickg.bio

…

On Mon, Apr 8, 2024 at 7:20 AM Ross Farrugia ***@***.***> wrote: yes absolutely, hoping @zdz2101 <https://github.com/zdz2101> can share with us a first draft of the initial outline for our ADVS with the growth development parameters, and from there we all can review and consider whether we see any different inputs/outputs/functions/arguments needed. always easier if someone from admiral core dev team does this first as they know the admiral dev principles best. — Reply to this email directly, view it on GitHub <#5 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3TDS4MFJJPPCXCOU23R4DY4J4Q3AVCNFSM6AAAAABDEQCPPKVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4TANBUHAYTS> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ngiangre]

Thank you David, I totally agree! I screenshotted directly from the README of {gigs} Cheers, Nick Giangreco, PhD nickg.bio

…

On Mon, Apr 8, 2024 at 8:29 AM David Freedman ***@***.***> wrote: My 2 cents: I think it is the wrong approach to have a user type in values for 1 person and then to get a z-score back. As a user of the function and based on talks with other users, > 99% of the time, people are going to want to feed a dataset / dataframe to the function and get back the same dataset with some additional columns for z-scores and percentiles. If you do want something like what is shown in the screenshot, the user could make a 1-row dataframe and feed it to the function. In my 20 years at CDC using and maintaining the CDC and WHO functions, I never had a user ask us for a function that works like the one in the screenshot. Everyone was happy to feed a dataset to the function and get back the dataset with additional columns for the z-scores and percentile. — Reply to this email directly, view it on GitHub <#5 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3TDS5JQILFFCV6C5OCOCDY4KETRAVCNFSM6AAAAABDEQCPPKVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4TANBVGU3DQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[rossfarrugia]

Closing out this discussion now and we should take the above considerations into effect when developing the admiralpeds functions, but also we have the benefit of using David's packages as essentially a QC check to verify consistency of our results.