epiforecasts/bpmodels

Make clear whether `chain_ll()` function calculates a likelihood or loglikelihood

joshwlambert opened this issue · 3 comments

The current documentation of the chain_ll() function says it calculates the likelihood. But from what I can tell it is actually the loglikelihood. Indications of this also come from the function name (I assume _ll() stands for loglikelihood) and return value in the documentation of nbinom_size_ll() is "log-likelihood values".

I would recommend making it clear in the documentation of chain_ll() whether it is the likelihood or loglikelihood being returned.

Related to this could be a log argument in the function which accepts boolean values to turn the log transformation on or off.

sbfnk commented

Excellent point - as we're deprecating the package perhaps only fixing the documentation is the best approach here. In epichains we can then add a log argument as you suggest (which would, if set to FALSE, add an additional call to exp at the end) to a function that has a more sensible name (see epiverse-trace/epichains#26).

Thanks for catching this, Josh.

Seb has basically said what's on my mind.

@joshwlambert This issue has been addressed in #75. Please review it and approve it if it fixes the issue and follow up conversation.