AngusMcLure / PoolPoweR

Power and sample size calculations for surveys using pool testing (AKA group testing)
GNU General Public License v3.0
0 stars 1 forks source link

Standardise input variable names and local aliases #1

Closed fredjaya closed 10 months ago

fredjaya commented 11 months ago

Amend input variable names so it is descriptive/verbose.

Then, these should be assigned within the functions so they match the math/latex notation as close as possible.

Variable name Current local alias(es)
pool_size s
pool_number N
prevalence p, theta
sensitivity varphi, sens
specificity psi, spec
correlation rho

Also amend cost.X variables to cost_X for styling.

@AngusMcLure can prevalence, sensitivity and specificity all have a single local alias? e.g. theta, varphi, psi respectively?

AngusMcLure commented 11 months ago

I like this plan, including fixing up the naming for cost.X etc.

Yes, I think changing those variables to have the same local alias is a good idea. It might be neater to have the local aliases consistently by the Greek letter names. (so correlation should be rho as well). The pool size and pool number are more fundamentally different so they can stay as s and N.

This is slightly more complicated, and perhaps could be its own issue for later, but it would be good if the order of the arguments could be consistent across functions too. Not every function has the same inputs obviously, but I would suggest the following order (omitting unused/irrelevant variables)

s, N, prevalence, cost_unit, cost_pool, cost_location, correlation, sensitivity, specificity, form, [any others]

Finally, when you make changes to parameters, if you wouldn't mind doing two things:

AngusMcLure commented 11 months ago

@AngusMcLure can prevalence, sensitivity and specificity all have a single local alias? e.g. theta, varphi, psi respectively?

Realised I perhaps didn't answer this clearly. Yes, I think a single greek letter alias (the ones you have given) would be appropriate

fredjaya commented 11 months ago

@AngusMcLure should these uses of p in (the renamed) fi_pool_cluster() remain as p, or be changed to theta (prevalence)?

They are mainly used in the integrand functions: https://github.com/AngusMcLure/PoolPoweR/blob/3e0cc1cf4c09c700386fddbdbdaaa28b8050233d/R/fisher_information.R#L140-L220 https://github.com/AngusMcLure/PoolPoweR/blob/3e0cc1cf4c09c700386fddbdbdaaa28b8050233d/R/fisher_information.R#L305-L313

AngusMcLure commented 11 months ago

Good catch. They should stay as p. As a parameter they are on the same scale as population prevalence (theta) but they are not the population prevalence per se. p is the variable of integration

fredjaya commented 11 months ago

Complete for fisher_information.R and optimise_prevalence.R

fredjaya commented 11 months ago

sorry not yet