Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
joshwlambert opened this issue Jun 7, 2023 · 3 comments · Fixed by #75
Closed
Labels

Comments

@joshwlambert
Copy link

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
Copy link
Collaborator

sbfnk commented Jun 7, 2023

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).

@jamesmbaazam
Copy link
Collaborator

Thanks for catching this, Josh.

Seb has basically said what's on my mind.

@jamesmbaazam
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants