tree: RAxML bootstrapping #1127
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
…tree builder The default parameters actually used are in DEFAULT_ARGS and the parameters actually used may be overridden by tree_builder_args. More confusing than helpful to include these.
…provided by the user Don't warn when the default --substitution-model is left unchanged. Related-to: <https://support.nextstrain.org/agent/nextstrain/nextstrain/tickets/details/668036000002953001>
Instead of using variants like IQTREE or IQtree. IQ-TREE is the form used by the project itself.
Otherwise it's swallowed whole, never to be seen again. This doesn't print the exception's stack trace in order to keep error messaging more readable for users, but we could consider printing that too when under "debug mode" (e.g. AUGUR_DEBUG=1) or something. Related-to: <https://support.nextstrain.org/agent/nextstrain/nextstrain/tickets/details/668036000002953001>
Use the output file which includes bootstrap support values when RAxML's bootstrapping is enabled by user-provided --tree-builder-args. This also avoids errors caused when trying to clean up RAxML output files that aren't present when bootstrapping is enabled, such as "RAxML_parsimonyTree" and "RAxML_result". Related-to: <https://support.nextstrain.org/agent/nextstrain/nextstrain/tickets/details/668036000002953001>
Codecov ReportBase: 67.98% // Head: 67.94% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1127 +/- ##
==========================================
- Coverage 67.98% 67.94% -0.05%
==========================================
Files 57 57
Lines 6656 6663 +7
Branches 1637 1640 +3
==========================================
+ Hits 4525 4527 +2
- Misses 1825 1830 +5
Partials 306 306
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
victorlin
approved these changes
Jan 12, 2023
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
Resolves several inter-related issues in
augur tree --method raxmlwhen a user-provided--tree-builder-argsenables bootstrapping.Related issue(s)
https://support.nextstrain.org/agent/nextstrain/nextstrain/tickets/details/668036000002953001
Testing
Manually tested various invocations of
augur treeexercising the changes. (The existingaugur treetests seem focused on basic functionality.)I also modified zika-tutorial with this patch to reproduce the problem reported via support:
and tested it completes successfully now with these changes by running:
It does.
Checklist