Repository navigation
Key the random local clock's rates prior to its clock partition - #222
Merged
Merged
Conversation
RRatesPrior.c:s$(n) does not match the .c:$(n) pattern, so BEAUti fills in $(n) with the data partition's name instead of the clock's. After the data partition is renamed, the rule looks for RRatesPrior.c:s<newname>, so switching the clock away from the random local clock leaves the old prior in the analysis, on clock rates that are no longer in the model. The same fix is in CompEvol/BeastFX#114 for BEAST 2.7.
This file contains hidden or 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
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.
In
beast-fx/.../fxtemplates/ClockModels.xmlthe random local clock's rates prior has idRRatesPrior.c:s$(n). Its connect rule is generated from the same id, so it is added correctly at first. But.c:s$(n)doesn't match the.c:$(n)pattern inBeautiDoc.translatePartitionNames, so$(n)becomes the data partition's name rather than the clock's.After renaming the data partition, the rule looks for
RRatesPrior.c:s<newname>. Switching the clock away from the random local clock then leaves the old prior in the analysis, onclockratesthat are no longer in the model. The posterior is unaffected (the parameter is fixed), but the logged prior is off and the XML carries a dangling prior.This PR drops the stray
s, and updates the id expected bySimpleClockModelTest(currently@Disabled).Tested in BEAUti 3 (master + this change): with the change, after a random local clock, renaming the data partition and switching to a strict clock, no random local clock prior is left behind; without it,
RRatesPrior.c:s<oldname>remains. BEAST 3 accepts the XML either way.mvn -pl beast-fx testpasses.The same fix (plus the mean-rate prior id, already correct here) is CompEvol/BeastFX#114 for BEAST 2.7. Found while comparing BEAST 2.7.9 and BEAST 3 analyses built in BEAUti (CompEvol/beast3-test, C15).