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

Map improvements including transmission ribbons #712

Merged
merged 7 commits into from
Mar 11, 2019
Merged

Conversation

trvrb
Copy link
Member

@trvrb trvrb commented Mar 11, 2019

This PR includes a few separate map improvements.

  1. Slightly jitter each separate transmission line so that if there are multiple events they appear as a ribbon rather than a narrow line. I think this is a significant improvement. And it has the nice effect where the transmission ribbon thickness properly interacts with visibility adjustments made through date slider or tree zoom.

Here's current Ebola:

ebola

Here's an animation of flu spread:

flu

This just adds an additional extend parameter that is propagated down through the map code to the underlying bezier math that pushes the arc out by extend number of pixels. extend for a particular pair of demes starts at 0 and increments with each transmission event.

This was mainly motivated by the ugliness of current regional flu view, but I think made a big difference overall. Compare before:

Screen Shot 2019-03-10 at 5 13 41 PM

To after:

Screen Shot 2019-03-10 at 5 14 52 PM

  1. Improve deme circle sizing. Previously, deme circle area was proportional to the number of tips belonging to this deme. This resulted in large datasets like flu having circles that were too big and small datasets like Ebola in North Kivu having circles that were too small. This PR adds a normalization by the square root of the total number of nodes in the tree. I kept this as square root so that smaller datasets still have slightly smaller circles than larger datasets. Within a single auspice view, we maintain circle area as proportional to tip count (as I believe is most expected behavior).

This mostly addresses #280. Initial deme radii look good. Zooming out too far on a narrow set of regions (like Ebola in West Africa or H7N9 in China) doesn't look great, but it's really not bad.

  1. Improve initial map bounds logic. This is a small fix to make initial zoom bounds better fit to the lat/longs that are present. Essentially we just needed to set Leaflet's zoomSnap to something more granular than the default 1.

  2. Don't show error for missing lat/longs if deme is labeled "Unknown" or "unknown". This was apparent in https://nextstrain.org/community/inrb-drc/ebola-nord-kivu, where the samples that are labeled as "unknown" really should not have an associated lat/long.

  3. Add very slight border to deme circles. I think this looks stronger and more grounded.

Resolves #425.
Resolves #369.

This jitters the height of the Bezier curve of each transmission line based on the the specific count of deme A to deme B. This causes lines to become ribbons with thickness proportional to the number of transmission events.
Previously, deme circle area was directly proportional to tip count. This caused bigger datasets to have too large circles and smaller datasets to have too small circles. This commit adds a factor based on total node count to equilibrate. Testing across existing datasets suggests an improvement.
This improves initial fit of map bounds, mainly by setting "zoomSnap" parameter to 0.5, rather than the 1.0 default.
@rneher
Copy link
Member

rneher commented Mar 11, 2019

I like it. looks much better IMO!

@trvrb
Copy link
Member Author

trvrb commented Mar 11, 2019

I'm going to go ahead and merge this as well.

@trvrb trvrb merged commit e21bffa into master Mar 11, 2019
@trvrb trvrb deleted the map-improvements branch March 11, 2019 15:09
@rneher
Copy link
Member

rneher commented Mar 11, 2019

There is one aspect that looks a little strange:
image
The circles around the disk have sometimes odd colors. This is from
https://nextstrain.org/flu/seasonal/h3n2/ha/6m?c=gt-HA1_91&p=grid

@trvrb
Copy link
Member Author

trvrb commented Mar 11, 2019

Thanks for catching this Richard. I know what this is (stroke color not updating on colorBy change). Edit: Fixed in: cefef4a

@jameshadfield
Copy link
Member

Looks great!

@ghost
Copy link

ghost commented Mar 26, 2019

cool!

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.

Bundle transmissions together rather than overlaying them Remove commented code from map where appropriate
3 participants