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

Upgrade treetime to v0.7 #431

Merged
merged 7 commits into from
Feb 13, 2020
Merged

Upgrade treetime to v0.7 #431

merged 7 commits into from
Feb 13, 2020

Conversation

rneher
Copy link
Member

@rneher rneher commented Jan 4, 2020

creating this PR not for immediate merging but for discussion and testing.

TreeTime underwent a number of changes from 0.6 to 0.7. Among other things

  • you can't mess with node.sequence anymore as these are now protected
  • tip states are not reconstructed by default anymore
  • traits inference is followed by numerical optimization of the overall switching rate. This doesn't require the 'short branch length' assumption that was made previously and that is violated for rapidly changing states.
  • numerical date conversion is now handled more consistently

Our traits code messes with node.sequences, so this needed changing. Other than that, I have not noticed any problems but it will be good to check more thoroughly, also test treetime.

…f the parallel implementation that existed in augur traits.
@rneher
Copy link
Member Author

rneher commented Jan 4, 2020

Validation is failing, however, since augur is now exporting mutations like A564W which are not valid nucleotide mutations according to the schema. We need to decide how we want to report those. Some data sets like Ebola also contain other illegal characters like ? in sequences. those would now show up as

image

rneher added 3 commits January 8, 2020 23:25
…ous with the latter being default (as it was for vcf, now also for fasta input).
@rneher
Copy link
Member Author

rneher commented Jan 9, 2020

This should implement the issues we discussed last night. The Ebola ancestral reconstruction now looks proper in auspice:
image

Augur now goes through TTs map of characters to likelihood profiles and identifies fully ambiguous characters which are all mapped to 'N'

@rneher
Copy link
Member Author

rneher commented Jan 9, 2020

With the updated v1 schema, this passes tests now. Should be ready to merge and address #429

@rneher rneher requested review from emmahodcroft and trvrb January 9, 2020 10:03
This keeps default augur behavior unchanged despite the TreeTime upgrade. By adding the --keep-ambiguous flag, new behavior can be requested. This seemed safer in terms of not causing unexpected behavior for users.
This was referenced Feb 12, 2020
@trvrb trvrb merged commit 7d4a10d into master Feb 13, 2020
@trvrb trvrb deleted the tt07 branch February 13, 2020 23:22
@emmahodcroft
Copy link
Member

Hi @rjabhinav, it's best to open a new issue if you need help. Please don't reply to this PR anymore, but open an issue if you need further help.

We apologise that the instructions for the Zika tutorial have gone out of date somewhat. augur ancestral now has two output options that must be specified: --output-node-data and --output-sequences. Please try setting both of these and see if this works. If not, please open a new issue!

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.

3 participants