Distance based shelters

[dstuck]

Refactoring the find shelters functionality to use distance from a given point. This is currently going to reduce efficiency but will enable using geocoding to get the closest shelters to any given point, not just the center of a zip code.

This code is definitely less performant as number of shelters goes up but we can add in some optimizations in the future so we're only checking a subset of zip codes first, but didn't include that in the initial version.

Currently posting as a draft because I know there are some changes to the overall functionality that haven't been discussed yet and so am gonna hold off on tests until I see people are happy with this proposed change in interface.

[dstuck]

Also, I'm not a js coder so I welcome any style clean up comments; get as pedantic as you want!

[nihonjinrxs]

@dstuck thanks for this work, and thanks for posting it as a draft for discussion. Very helpful.

I'm not opposed to your change, since I think the experience could possibly be better.

However, I'm not sure about this direction because I think the change from "return a max of 3 shelters within n miles radius" to "return m shelters" is also a rather large change to the experience that I'd probably want to test before just deciding to run with it. That could be solvable by adding a mile radius check into your approach, though, which would make them very similar in experience.

In addition, I'm concerned that the need to geocode the inputs would end up meaning reliance on an external service that may end up costing money.

(That's in addition to the performance hit you've already noted, which I think we might be able to get around with a bit of creative JS coding.)

To clarify what the current process does, it:

1. extracts zip codes from each incoming message (possibly more than one)
2. augments those zip codes with their geocenter lat and long (from a local datafile in a library, not a lookup) and computes all neighboring zip codes whose geocenter is within the mile radius specified
3. filters to retain only zip codes with shelters present in them from the total list of zip codes within the mile radius of each incoming extracted zip
4. computes geodistance between the shelter lat/long and incoming extracted zip code lat/long for each shelter and incoming extracted zip code pair
5. sorts found shelters by increasing distance
6. truncates the sorted list of shelters for each incoming extracted zip to a maximum of 3
7. builds the shelter messages array and returns it for formatting by the TwilioFormatter

Note that if there are less than 3 within the radius, the user will get less, rather than getting 3 shelters even if they're miles away. Because of that, I think we need to maintain the mile radius in the mix.

I'm happy to talk through how this process can be made better (and I'm sure it can be), but I'm not sure I'm ready to run with this approach yet.

Is my understanding of what you're doing incorrect? I'd appreciate if you could walk me through your approach in a bit more detail. What is your goal with this approach, and could you walk through the algorithm/steps to get there?

Thanks again.

[dstuck]

@nihonjinrxs Yeah the proposed new flow would be:

1. Extract location information from text (right now just pull out zip code)
2. Convert location information to LatLon (currently using zipcodes library, but also enables geocoding)
3. Find closest shelters to LatLon
   1. Compute distance to each shelter
   2. Sort and filter down to those within X miles (or N closest later)
4. Create message from list of shelters

I think this flow separates responsibilities a little cleaner and enables us to improve our location extraction in the future to possibly use geocoding.

I agree that we should investigate the cost of geocoding and discuss the value of specifying specific locations. I think it would really improve the usability but also it will make extracting location data and converting to latlon significantly more complicated

[dstuck]

Added back in support for multiple zip codes and for max radius which we can discuss changing later. If this looks reasonable, I'll finish up by adding test coverage on the refactored version and set as ready for review!

[nihonjinrxs]

Based on discussion at Project Night, we'll migrate this toward the following:

• @dstuck Move zip code based input to a separate class & file, keeping the LocationsFinder stuff all Lat/Lon based.
• @nihonjinrxs Refactor to have all base functionality not related to Shelters specifically be applied to Location in a base class, with Shelter specific stuff in a Shelter subclass
• @nihonjinrxs Optimize Location search by calculating a bounding box of Lat/Lon for the mile radius and filtering using that, instead of filtering using the zip codes, then do distance calculation only on those locations within the bounding box
• @dstuck Explore mechanisms for, as a later step, using reverse geocoding in a new input class to find Lat/Lon from user input.

[nihonjinrxs]

@dstuck I added you to the hurricane-response team. Let's push a new branch on this repo, and work together there. That'll mean closing this PR and creating a new one on that new branch.

[nihonjinrxs]

@dstuck Closing this one in favor of #11.