Mixlen refactoring: Remove dependency on ModelFactoryMixlen and cppoptlib, add support for the -ft option - #182
Conversation
|
The remaining problems: |
…ame getMixlen() to getNMixlen()
|
On the new changes:
|
|
@bqminh, |
This pull request simplifies the GHOST rate model implementation and allows using mixture models with GHOST rates for PMSF profile estimation on a guide tree. Specifically, it introduces the following changes:
ModelFactoryMixlenclass into theModelFactoryclass. The former class was just adding unnecessary complexity, its functionality is now seamlessly included into theModelFactoryclass.PhyloTreeMixlenclass on thecppoptliblibrary and simplify initialization ofPhyloTreeMixlenobjects. The library has never been compiled and used, its version has long been deprecated, and if needed someday (very unlikely), it can be reimplemented without inheriting fromPhyloTreeMixlen. The library required the number of heterotachy categories being set by thePhyloTreeMixlen()constructor (and used the ill-defined-mixlenoption for this), this largely contradicted the general logic that implies that the number of categories should be passed within the model string (as-m "...+Hn").Now the logic is respected: the
PhyloTreeMixlen()constructor takes only the alignment object as an argument, the number of heterotachy categories is set exclusively by thePhyloTreeMixlen::setModelFactory()function as given in the model string, and if a tree is read from a file with not enough or with extra sets of branch lengths, it is corrected by thePhyloTreeMixlen::initializeMixBranches()function as before.PhyloTreeMixlenclass initialization rules (see above),mixlenis always 1 for a newly constructed object and is always higher than 1 after setting aRateHeterotachysite rate withsetModelFactory(), it can never be 0. Also ASSERTs are added to ensure that a model factory provides aRateHeterotachysite rate (or a nullptr) and that aRateHeterotachysite rate uses aPhyloTreeMixlentree.PhyloTree::getCurMixture()to return -1 instead of 0. The old output likely didn't affect anything for now, but clearly was a latent bug.PhyloTreeMixleninAliSimulator::initializeIQTreeFromTreeFile(). The fix comes from declaringIQTree *¤t_treeas a reference to a pointer, not just a pointer in the supertree case -- this way the subtrees are reset, not just deleted in the supertree vector.IQTreeorPhyloTreeMixlenobject instead of thePhyloTreeobject for the guide tree in thecomputeSiteFrequencyModel()function ofphyloanalysis.cppto extend the-ftoption to the mixture models that use heterotachy rates. This makes the fix for the Segmentation Fault when using PMSF with GHOST models iqtree2#459 issue complete.Note
params.min_branch_lengthbeing temporarily set in the function: this is needed in the general case, but the need is especially pronounced for the heterotachy rate model, which can be using very short branch lengths (without the minimal length, this results in numerical underflows).runStandardBootstrap()function ofphyloanalysis.cpp, change the indices of the.bootlhfile lines from always zero to usingsample. This is the output of the-wtloption; it looks pretty useless, for the likelihoods are not phylogenetic, but polynomial, but let it at least have the correct indexing now.These changes are intended not to affect tree and model optimization. The optimized likelihoods should be the same as in the upstream version wherever they can be compared.