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 IQ-TREE to 2.3.5 with CMAPLE support #228

Merged
merged 1 commit into from
Jul 30, 2024
Merged

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Jul 29, 2024

We want the CMAPLE support (first available with 2.3.4) for better and faster trees.

CMAPLE support is optional and not all release builds of IQ-TREE have it enabled. So far, it seems that the "main" releases are made without CMAPLE support (e.g. 2.3.5) and then parallel releases are made with it (e.g. 2.3.5.cmaple), but only for x86_64 on Linux and macOS. Thus, we're still not using an aarch64/arm64 binary even though the "main" releases (without CMAPLE support) provide them. If we keep waiting, maybe they'll start being provided, or, eventually, we can start compiling them ourselves. That's for another time, though.

Resolves: #226

Checklist

  • Checks pass

We want the CMAPLE support (first available with 2.3.4) for better and
faster trees.

CMAPLE support is optional and not all release builds of IQ-TREE have it
enabled.  So far, it seems that the "main" releases are made without
CMAPLE support (e.g. 2.3.5) and then parallel releases are made with it
(e.g. 2.3.5.cmaple), but only for x86_64 on Linux and macOS.  Thus,
we're still not using an aarch64/arm64 binary even though the "main"
releases (without CMAPLE support) provide them.  If we keep waiting,
maybe they'll start being provided, or, eventually, we can start
compiling them ourselves.  That's for another time, though.

Resolves: <#226>
@tsibley
Copy link
Member Author

tsibley commented Jul 29, 2024

This upgrades IQ-TREE from 2.1.2 to 2.3.5.cmaple. I scanned the release notes between those versions and didn't notice any major breaking changes.

@tsibley tsibley marked this pull request as ready for review July 30, 2024 05:20
@tsibley tsibley merged commit addb745 into master Jul 30, 2024
47 checks passed
@tsibley tsibley deleted the trs/iqtree-with-cmaple branch July 30, 2024 18:28
Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

12 ✅ for the pathogen-repo-ci jobs 🎉

Slightly tangential, does this warrant updating the citation in augur tree?

@tsibley
Copy link
Member Author

tsibley commented Jul 30, 2024

Uh… maybe!

Augur's not guaranteed to use an IQ-TREE with CMAPLE support, but it does us no harm to update the citation.

@tsibley
Copy link
Member Author

tsibley commented Jul 30, 2024

12 ✅ for the pathogen-repo-ci jobs 🎉

\o/

That said, my understanding here is that we'd not necessarily expect these to fail with CMAPLE support but get faster and/or better trees. And our pathogen CI isn't checking tree quality—that's still the sole domain of 👀 unfortunately—so potentially we've accidentally made the trees worse in some way with this change and wouldn't know via automated checks. But that would be contrary to expectations.

@huddlej
Copy link
Contributor

huddlej commented Jul 30, 2024

Thanks, @tsibley! I'm sure @bqminh will be happy to see this. He has previous said they would try to prioritize CMAPLE + ARM support.

That said, my understanding here is that we'd not necessarily expect these to fail with CMAPLE support but get faster and/or better trees.

We'd have to opt into the CMAPLE algorithm in our tree rules with the tree builder arg --pathogen-force (thanks to @corneliusroemer for confirming this!). It seems like mpox might be the first place we choose to opt for that algorithm but 🤷🏻

@bqminh
Copy link

bqminh commented Jul 31, 2024

Yes, @trongnhanuit and me are happy to see it happening ;-) thanks!

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.

Add IQtree version with cmaple feature
4 participants