search

CompEvol / beast2

Bayesian Evolutionary Analysis by Sampling Trees
www.beast2.org
GNU Lesser General Public License v2.1
233 stars 82 forks source link

Tree logger should not sort tree in state #923

Open Rong419 opened 3 years ago

Rong419 commented 3 years ago

For likelihoods that depend on tree orientation, loggers that sort the tree in state break likelihood comparisons in store/restore.

Solution: copy the tree as its being logged, sort the copy and log that. Keep the original tree in state unchanged.

tgvaughan commented 3 years ago

Hi @Rong419, your commit doesn't seem to follow your proposed solution. Also, mightn't a better approach be to do the sorting during construction of the Newick string? Then you wouldn't have to copy anything.

alexeid commented 3 years ago

I like the idea of sorting while constructing the newick string.

On 18/09/2020, at 7:22 PM, Tim Vaughan notifications@github.com wrote:

Hi @Rong419 https://github.com/Rong419, your commit doesn't seem to follow your proposed solution. Also, mightn't a better approach be to do the sorting during construction of the Newick string? Then you wouldn't have to copy anything.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CompEvol/beast2/issues/923#issuecomment-694703890, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG5MSLTR6K5JHUI7IEHK53SGMDFFANCNFSM4RRHV2SA.