Refactor fi_pool_cluster()

[fredjaya]

Low priority enhancements to improve code readability and unit testing such as:

• Breaking up components into separate functions for:
  • form == "beta"
  • form == "discrete"
  • form %in% c("logitnorm", "cloglognorm")
  • real_scale == FALSE
• Remove avoidable nesting of if (form %in% c("discrete", "beta")) { (comment on why beta and discrete have to be calculated similarly still addresses this)

Thoughts?

[AngusMcLure]

I agree the function needs restructuring in some way. I think the real_scale TRUE and FALSE cases should be handled by the same function.

We could also definitely split up into separate (non_exported) functions which fi_cluster calls in the backend. Perhaps named something like:

• fi_beta
• fi_discrete
• fi_linknorm

To do this, we'd move phi() and log_one_minus_phi() out as separate utility functions which would be called by all three. These two utility functions would also need additional arguments for sensitivity, specificity, and pool size passed to them. This will make some of the more math-heavy code harder to read as we'll be replaced phi(p) with phi(p, s, varphi, psi), but I think this is pretty minor, compared to the benefits.

Will do this, but low priority for now