From 003e32808b9a258a159dc7226771041e0bab9ffb Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 10:58:09 -0400 Subject: [PATCH 1/9] Use enumerate to avoid separate counter --- NetworkFlowAnalyzer.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/NetworkFlowAnalyzer.py b/NetworkFlowAnalyzer.py index e30a2e6..3832e08 100644 --- a/NetworkFlowAnalyzer.py +++ b/NetworkFlowAnalyzer.py @@ -41,15 +41,13 @@ vertex = {} # maps stationName to vertex number # store data -row = 0 -for station in input["stationBeanList"]: +for row, station in enumerate(input["stationBeanList"]): # store data in numpy arrays totalDocks[row] = station["totalDocks"] stations[row][0] = station["latitude"] stations[row][1] = station["longitude"] vertex[station["stationName"]] = row names[row] = station["stationName"] - row += 1 # process optional args if args.stations: From 90a05346f13d99f73b5cec12fc04983b1af04786 Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:09:13 -0400 Subject: [PATCH 2/9] Seems like there should be a station object We're manipulating vectors of length num_stations and using indexing to get the right station. Each vector has one attribute of a station. This feels like there should be a station object that contains all the attributes you need. --- MaxFlowUtils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/MaxFlowUtils.py b/MaxFlowUtils.py index 7b41c63..f1b1f78 100644 --- a/MaxFlowUtils.py +++ b/MaxFlowUtils.py @@ -34,6 +34,7 @@ def findFlowEdges(stations, totalDocks, source, capacity_func): num_stations = len(stations) marked_v = np.ones((num_stations,), dtype=bool) # marks all from vertices marked_w = np.ones((num_stations,), dtype=bool) # marks all to vertices + # this feels like there should be an object q = collections.deque() q.append(source) From a8538ea27dcfc659a0df12f13f2c9a4474b85166 Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:18:52 -0400 Subject: [PATCH 3/9] Initialize to zeros instead of random numbers --- NetworkFlowAnalyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NetworkFlowAnalyzer.py b/NetworkFlowAnalyzer.py index 3832e08..cee3fc9 100644 --- a/NetworkFlowAnalyzer.py +++ b/NetworkFlowAnalyzer.py @@ -34,7 +34,7 @@ input = json.load(f) num_stations = len(input["stationBeanList"]) -stations = np.random.random((num_stations, 2)) # 2 cols for latitude, longitude +stations = np.zeros(shape=(num_stations, 2)) # 2 cols for latitude, longitude #totalDocks = np.zeros((num_stations,), dtype=np.uint8) totalDocks = np.zeros((num_stations,), dtype=np.int16) names = {} # maps vertex number to stationNam From 0cbe1debdf50af7425447098c2b45ba5189dc157 Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:25:33 -0400 Subject: [PATCH 4/9] Remove matplotlib requirement for code review I'm having installation issues with matplotlib, so ignoring that for now. --- MaxFlowUtils.py | 2 +- NetworkFlowAnalyzer.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/MaxFlowUtils.py b/MaxFlowUtils.py index f1b1f78..a62bdb8 100644 --- a/MaxFlowUtils.py +++ b/MaxFlowUtils.py @@ -1,6 +1,6 @@ import math, collections from MaxFlowLib import FlowEdge -import matplotlib.pyplot as plt +# import matplotlib.pyplot as plt import numpy as np import copy _DISTANCE = .4 # km diff --git a/NetworkFlowAnalyzer.py b/NetworkFlowAnalyzer.py index cee3fc9..0beedd7 100644 --- a/NetworkFlowAnalyzer.py +++ b/NetworkFlowAnalyzer.py @@ -11,7 +11,7 @@ ''' import json import argparse -import matplotlib.pyplot as plt +# import matplotlib.pyplot as plt import numpy as np import random import MaxFlowUtils @@ -76,15 +76,15 @@ # calculate maxflow using Ford-Fulkerson algorithm maxflow = FF(flownet, source, target) # plot flow network - MaxFlowUtils.plotFlowNetwork(stations, source, target, flow_edges) - plt.show() + # MaxFlowUtils.plotFlowNetwork(stations, source, target, flow_edges) + # plt.show() # flow_edges.append(FlowEdge(329, 330, totalDocks[329] + totalDocks[330])) # plot flow flowpath = MaxFlowUtils.flowPath(stations, flownet, source) flowpath2 = copy.copy(flowpath) - MaxFlowUtils.plotFlow(stations, source, target, flowpath) - plt.show() + # MaxFlowUtils.plotFlow(stations, source, target, flowpath) + # plt.show() # convert vertices in flow path to station names station_path = MaxFlowUtils.toStationNames(flowpath2, names) @@ -103,8 +103,8 @@ print e # plot edges in st-cut - MaxFlowUtils.plotSTcut(stations, source, target, flowpath, stcut_v) - plt.show() + # MaxFlowUtils.plotSTcut(stations, source, target, flowpath, stcut_v) + # plt.show() else: print '{} and {} are not connected.'.format(start_station, end_station) From 1e02a615424298e7ab22e0975d52492e0823199c Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:33:01 -0400 Subject: [PATCH 5/9] simplify syntax --- MaxFlowUtils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MaxFlowUtils.py b/MaxFlowUtils.py index a62bdb8..ed41990 100644 --- a/MaxFlowUtils.py +++ b/MaxFlowUtils.py @@ -9,7 +9,7 @@ def distance(v, stations): '''find distance from v to all other vertices''' R = 6371 - dLat_dLong = np.radians(stations[:, [0, 1]] - stations[v, [0, 1]]) + dLat_dLong = np.radians(stations - stations[v]) dLat = dLat_dLong[:, 0] dLong = dLat_dLong[:, 1] lat = np.radians(stations[:, 0]) # col vector of lat values for all stations; num_stations x 1 From 5bd6324fb2c3e29b66992e4ac9710a18c91ebd09 Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:40:48 -0400 Subject: [PATCH 6/9] fix to readme to indicate actual usage --- README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README b/README index 96dc9f3..aa6ade4 100644 --- a/README +++ b/README @@ -10,7 +10,7 @@ Capacity on an edge is the max of the bike docks at each edge vertex. usage: run NetworkFlowAnalyzer.py "citybike.json" Example: -run NetworkFlowAnalyzer.py "citybike.json" "Broadway & Battery Pl" "E 47 St & 1 Ave" +run NetworkFlowAnalyzer.py --start_station "Broadway & Battery Pl" --end_station "E 47 St & 1 Ave" "citybike.json" run NetworkFlowAnalyzer.py [-h] [--stations] [--start_station START_STATION] [--end_station END_STATION] filename-json default start_station: "Broadway & Battery Pl" From 6ac38aaf5fdd7c910e7175212c3a6c732996118e Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:43:21 -0400 Subject: [PATCH 7/9] Condense distance function We can make this a lot more readable by only calling np.radians once per function call. We'd also appreciate more comments of the function being implemented & especially the constant R - what is that? --- MaxFlowUtils.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/MaxFlowUtils.py b/MaxFlowUtils.py index ed41990..798af18 100644 --- a/MaxFlowUtils.py +++ b/MaxFlowUtils.py @@ -8,15 +8,14 @@ # vectorized implementation def distance(v, stations): '''find distance from v to all other vertices''' - R = 6371 - dLat_dLong = np.radians(stations - stations[v]) - dLat = dLat_dLong[:, 0] - dLong = dLat_dLong[:, 1] - lat = np.radians(stations[:, 0]) # col vector of lat values for all stations; num_stations x 1 - lat_v = np.radians(stations[v][0]) # scalar for this station - - dLatH = np.divide(dLat, 2) - dLongH = np.divide(dLong, 2) + stations = np.radians(stations) + R = 6371 # ? + dLat_dLong = stations - stations[v] + dLatH = dLat_dLong[:, 0] / 2 + dLongH = dLat_dLong[:, 1] / 2 + lat = stations[:, 0] # col vector of lat values for all stations; num_stations x 1 + lat_v = stations[v][0] # scalar for this station + a = np.sin(dLatH) ** 2 + np.sin(dLongH) ** 2 * np.cos(lat) * np.cos(lat_v) c = np.arctan2(np.sqrt(a), np.sqrt(1-a)) * 2 d = c * R From 2c3bbfd78acb7493f738011dbdc00aa8d452a330 Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:45:52 -0400 Subject: [PATCH 8/9] This simpler syntax appears to have the same result --- MaxFlowUtils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MaxFlowUtils.py b/MaxFlowUtils.py index 798af18..8f23d1d 100644 --- a/MaxFlowUtils.py +++ b/MaxFlowUtils.py @@ -31,8 +31,8 @@ def findFlowEdges(stations, totalDocks, source, capacity_func): ''' flow_edges = collections.deque() num_stations = len(stations) - marked_v = np.ones((num_stations,), dtype=bool) # marks all from vertices - marked_w = np.ones((num_stations,), dtype=bool) # marks all to vertices + marked_v = np.ones(num_stations, dtype=bool) # marks all from vertices + marked_w = np.ones(num_stations, dtype=bool) # marks all to vertices # this feels like there should be an object q = collections.deque() q.append(source) From 7b3354a8e68b77c21b90c5bced5ef2a26dd1c08e Mon Sep 17 00:00:00 2001 From: Allison Kaptur Date: Mon, 21 Oct 2013 11:58:12 -0400 Subject: [PATCH 9/9] beef up docstring --- MaxFlowUtils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/MaxFlowUtils.py b/MaxFlowUtils.py index 8f23d1d..04a9a51 100644 --- a/MaxFlowUtils.py +++ b/MaxFlowUtils.py @@ -7,7 +7,11 @@ # vectorized implementation def distance(v, stations): - '''find distance from v to all other vertices''' + ''' + find distance from v to all other vertices + input: stations array of [(lat, long), (lat, long) ...] + output: distance array of [dist, dist, ... ] + ''' stations = np.radians(stations) R = 6371 # ? dLat_dLong = stations - stations[v]