Allow users to specify arbitrary IQ-TREE models #776
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #776 +/- ##
==========================================
+ Coverage 33.75% 33.76% +0.01%
==========================================
Files 41 41
Lines 5898 5896 -2
Branches 1427 1426 -1
==========================================
Hits 1991 1991
+ Misses 3821 3819 -2
Partials 86 86
Continue to review full report at Codecov.
|
|
Looks good to me. the only point I am not sure about is the argument
|
|
Good call, @rneher! It wouldn't hurt to change the API here since it hasn't worked for years anyway. I like "auto", since it aligns with our existing "auto" option for the |
IQ-TREE supports hundreds of models through its `-m` argument. When users omit this argument, IQ-TREE will run a model test and use the best model to build the tree. Users can refer to the IQ-TREE log file to find the best model and use that in future runs of the command. Unfortunately, augur tree hardcodes only a handful of possible substitution models as viable choices for the `--substitution-model` argument. Although the help text for `--substitution-model` says users can provide "none" as an argument to run the IQ-TREE model test, "none" is not one of the hardcoded choices. This means the model test functionality of IQ-TREE hasn't been available for years. Additionally, the message we print to stdout when users can select "none" refers to the wrong log file, and once users find the best model for their data, they cannot provide it to augur tree unless it happens to be one of the predefined choices. To address these issues, this commit removes the list of hardcoded substitution model choices from augur tree, allowing users to provide any valid IQ-TREE model or "none", as described in the help text. This commit also adds a functional test of this "model test" interface that revealed a typo in the code that calls IQ-TREE, fixes the typo, and prints the path of the actual IQ-TREE log file to check for model test results. Fixes #619
Renames the value to pass to augur tree's `--substitution-model` argument from "none" to "auto" to more clearly communicate what this option does.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
IQ-TREE supports hundreds of models through its
-margument. Whenusers omit this argument, IQ-TREE will run a model test and use the best
model to build the tree. Users can refer to the IQ-TREE log file to find
the best model and use that in future runs of the command.
Unfortunately, augur tree hardcodes only a handful of possible
substitution models as viable choices for the
--substitution-modelargument. Although the help text for
--substitution-modelsays userscan provide "none" as an argument to run the IQ-TREE model test, "none"
is not one of the hardcoded choices. This means the model test
functionality of IQ-TREE hasn't been available for years. Additionally,
the message we print to stdout when users can select "none" refers to
the wrong log file, and once users find the best model for their data,
they cannot provide it to augur tree unless it happens to be one of the
predefined choices.
To address these issues, this commit removes the list of hardcoded
substitution model choices from augur tree, allowing users to provide
any valid IQ-TREE model or "none", as described in the help text. This
commit also adds a functional test of this "model test" interface that
revealed a typo in the code that calls IQ-TREE, fixes the typo, and
prints the path of the actual IQ-TREE log file to check for model test
results.
Related issue(s)
Fixes #619
Testing
Adds a functional test to
tests/functional/tree.tto test for the model test functionality of IQ-TREE.