-
Notifications
You must be signed in to change notification settings - Fork 10
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
Remove the topology-only clock #73
Comments
This is pretty old code I added to check the accuracy of the likelihood calculations vs a naive topology-only metric that basically just counts the number of edges beneath a node.
I can’t think of a time where we’d actually want to use this anymore.
… On Dec 6, 2019, at 9:03 PM, Yan Wong ***@***.***> wrote:
At the moment in the forwards, the section labelled # Topology-only clock assigns into the vv variable, but then nothing is done with that variable. It looks like there's a mistake there to me.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
I guess you meant to do the same as the previous lines, and assign |
Yes, another bit of code that used to be in there but got lost because we never used/tested the topology-only clock!
… On Dec 6, 2019, at 9:59 PM, Yan Wong ***@***.***> wrote:
I guess you meant to do the same as the previous lines, and assign val *= vv and probably g_val *= vv too in that branch of the if statement?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#73>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACRLT2BSWSTVL2Z7XYXJCSDQXLDKTANCNFSM4JXBIXLQ>.
|
Tests have show that basing node times on a naive expectation under the coalescent based on each marginal tree (what we call the "topology-only" clock) does not give good results (see e.g. #292), so I think we should simply remove the ability to do this, and raise a NotImplementedError |
At the moment in the forwards_algorithm, the section labelled
# Topology-only clock
assigns into the vv variable, but then nothing is done with that variable. It looks like there's a mistake there to me.The text was updated successfully, but these errors were encountered: