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

Parameterize size of clades to estimate by diffusion frequency likelihood #383

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

huddlej
Copy link
Contributor

@huddlej huddlej commented Oct 17, 2019

This PR exposes a previously hardcoded parameter for the diffusion tree frequency
estimator that determines how many tips a clade must have to be considered for
frequency estimation by the diffusion likelihood calculation. Clades that are
smaller than the specified threshold inherit their parent's frequencies
proportionally to their contribution to the parent clade.

This commit exposes the min_clades parameter through a keyword argument to the
diffusion frequencies class and also through the augur frequencies command
with the new --minimal-clade-size-to-estimate argument. This commit modifies
the description of the existing --minimal-clade-size argument to clarify its
use for filtering output in contrast to the new argument's use for controlling
frequency estimation itself. The previously hardcoded default value of 10 is now
the default for both the frequencies class and the command line argument.

…hood

Exposes a previously hardcoded parameter for the diffusion tree frequency
estimator that determines how many tips a clade must have to be considered for
frequency estimation by the diffusion likelihood calculation. Clades that are
smaller than the specified threshold inherit their parent's frequencies
proportionally to their contribution to the parent clade.

This commit exposes the `min_clades` parameter through a keyword argument to the
diffusion frequencies class and also through the `augur frequencies` command
with the new `--minimal-clade-size-to-estimate` argument. This commit modifies
the description of the existing `--minimal-clade-size` argument to clarify its
use for filtering output in contrast to the new argument's use for controlling
frequency estimation itself. The previously hardcoded default value of 10 is now
the default for both the frequencies class and the command line argument.
@huddlej huddlej requested a review from rneher October 17, 2019 17:49
Copy link
Member

@rneher rneher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good as well. My only comment is on the multiline string as help. As written, the string will include line breaks and spaces. But it seems to be rendered ok by the line wrap engine...

@huddlej huddlej merged commit 390b907 into master Oct 17, 2019
@rneher rneher deleted the min-clades-for-tree-frequencies branch October 27, 2019 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants