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

Local Branching Index #491

Merged
merged 4 commits into from Feb 5, 2018
Merged

Local Branching Index #491

merged 4 commits into from Feb 5, 2018

Conversation

jameshadfield
Copy link
Member

@jameshadfield jameshadfield commented Feb 2, 2018

This adds LBI to nextstrain/auspice - code copied from nextflu/auspice. It runs when lbi is specified by augur as a color_option so it's backward compatible with all JSONs. Augur branch -
https://github.com/nextstrain/augur/tree/LBI-auspice - has this output for flu, and defines tau and time_window parameters.

Questions / observations (@rneher):

  • The values are over [0, 1.0] but the colour scale is bounded by [0, 0.7] (same as nextflu)
  • This takes around 18ms (2000 tip flu tree) to calculate, and does not use visibility (so filters / time selected doesn't affect it, and it's not recomputed when these things change). Is this OK, or should we aim to optimise things so it can be recomputed when the visibility changes?
  • The colour scales look different between nextflu & nextstrain because the latter has (a) slightly different colour ramps and (b) picks values uniformly through the range -- this shows the same data with nexflu (top) vs nextstrain (below):
    image

image

@rneher
Copy link
Member

rneher commented Feb 2, 2018

great! two quick suggestions:

  • if you change the range to [0, 0.72] you get pretty numbers. the reason for the range [0,0.7] is purely aesthetic. if the range is [0, 1], then you rarely see red anywhere bc the peaks are internal nodes without circles.
  • the calls to Math.exp could be the most costly bit. we could precompute node.ebl =Math.exp(-node.clock_length/LBI_tau). But I don't think we need to update this dynamically right now.

@jameshadfield
Copy link
Member Author

Great - changing to 0.72 doesn't work as expected - i'll work on the legend sometime soon as there's a few little things there (0 isn't displayed, numbers could be better displayed...)

image

@rneher
Copy link
Member

rneher commented Feb 3, 2018

that's some messed up rounding. should be 0, 0.08, 0.16, 0.32, 0.4, etc

@jameshadfield jameshadfield mentioned this pull request Feb 5, 2018
@jameshadfield
Copy link
Member Author

for referenct - this is an example of a valid color_option in the meta JSON to generate LBI in auspice:

"lbi": {
 "menuItem": "local branching index",
 "tau": 0.4,
 "legendTitle": "local branching index",
 "key": "LBI",
 "timeWindow": 0.6
},

@jameshadfield jameshadfield merged commit bfe964a into master Feb 5, 2018
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.

None yet

2 participants