From 2e8c560c3f11bd51f1e2f49b533e717898eb83ea Mon Sep 17 00:00:00 2001 From: eric bryant Date: Sun, 21 Apr 2019 18:59:42 -0400 Subject: [PATCH 1/5] WaypointQuadtree.cpp FIXME comments * Cannot use w->colocated instead of waypoint_at_same_point(w), because singleton points would not increase the unique_locations count. * sprintf in WaypointQuadtree::str(): done. * WaypointQuadtree::size() used when printing stats @ and of program. Otherwise, commented-out debug text in NMP search. Speed is OK; leave as-is, optimized for RAM. --- .../classes/WaypointQuadtree/WaypointQuadtree.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp b/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp index 3c1bdc8c..8a622162 100644 --- a/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp +++ b/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp @@ -35,7 +35,7 @@ void WaypointQuadtree::insert(Waypoint *w) { // insert Waypoint *w into this quadtree node //std::cout << "QTDEBUG: " << str() << " insert " << w->str() << std::endl; if (!refined()) - { if (!waypoint_at_same_point(w)) //FIXME Try (!w->colocated) instead. Efficiency increase? + { if (!waypoint_at_same_point(w)) { //std::cout << "QTDEBUG: " << str() << " at " << unique_locations << " unique locations" << std::endl; unique_locations++; } @@ -114,20 +114,19 @@ std::forward_list WaypointQuadtree::near_miss_waypoints(Waypoint *w, return near_miss_points; } -std::string WaypointQuadtree::str() //FIXME use sprintf -{ std::string s = "WaypointQuadtree at (" + \ - std::to_string(min_lat) + "," + std::to_string(min_lng) + ") to (" + \ - std::to_string(max_lat) + "," + std::to_string(max_lng) + ")"; +std::string WaypointQuadtree::str() +{ char s[139]; + sprintf(s, "WaypointQuadtree at (%.15g,%.15g) to (%.15g,%.15g)", min_lat, min_lng, max_lat, max_lng); if (refined()) - return s + " REFINED"; - else return s + " contains " + std::to_string(std::distance(points.begin(), points.end())) + " waypoints"; + return std::string(s) + " REFINED"; + else return std::string(s) + " contains " + std::to_string(std::distance(points.begin(), points.end())) + " waypoints"; } unsigned int WaypointQuadtree::size() { // return the number of Waypoints in the tree if (refined()) return nw_child->size() + ne_child->size() + sw_child->size() + se_child->size(); - else return std::distance(points.begin(), points.end()); //FIXME std::list faster here? Where & how often is WaypointQuadtree::size() used? + else return std::distance(points.begin(), points.end()); } std::forward_list WaypointQuadtree::point_list() From 1e5901d00a4da4ec25c18317b3ee60498d41d269 Mon Sep 17 00:00:00 2001 From: eric bryant Date: Mon, 22 Apr 2019 02:04:32 -0400 Subject: [PATCH 2/5] region_hash --- siteupdate/cplusplus/classes/HighwaySystem.cpp | 4 ++-- siteupdate/cplusplus/classes/Region.cpp | 7 ------- siteupdate/cplusplus/classes/Route/Route.cpp | 8 +++++--- siteupdate/cplusplus/classes/Route/Route.h | 2 +- siteupdate/cplusplus/siteupdate.cpp | 4 +++- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/siteupdate/cplusplus/classes/HighwaySystem.cpp b/siteupdate/cplusplus/classes/HighwaySystem.cpp index d0423c85..72a4b4af 100644 --- a/siteupdate/cplusplus/classes/HighwaySystem.cpp +++ b/siteupdate/cplusplus/classes/HighwaySystem.cpp @@ -32,7 +32,7 @@ class HighwaySystem HighwaySystem(std::string &line, ErrorList &el, std::string path, std::string &systemsfile, std::list> &countries, - std::list &all_regions) + std::unordered_map ®ion_hash) { char *c_country = 0; std::string filename; std::ifstream file; @@ -63,7 +63,7 @@ class HighwaySystem else { getline(file, line); // ignore header line while(getline(file, line)) { if (line.back() == 0x0D) line.erase(line.end()-1); // trim DOS newlines - route_list.emplace_back(line, this, el, all_regions); + route_list.emplace_back(line, this, el, region_hash); if (!route_list.back().is_valid()) route_list.pop_back(); } } diff --git a/siteupdate/cplusplus/classes/Region.cpp b/siteupdate/cplusplus/classes/Region.cpp index b879c3e5..cd3307e0 100644 --- a/siteupdate/cplusplus/classes/Region.cpp +++ b/siteupdate/cplusplus/classes/Region.cpp @@ -93,13 +93,6 @@ class Region } }; -Region *region_by_code(std::string code, std::list &all_regions) -//FIXME replace with an unordered_map -{ for (std::list::iterator r = all_regions.begin(); r != all_regions.end(); r++) - if (r->code == code) return &*r; - return 0; -} - bool sort_regions_by_code(const Region *r1, const Region *r2) { return r1->code < r2->code; } diff --git a/siteupdate/cplusplus/classes/Route/Route.cpp b/siteupdate/cplusplus/classes/Route/Route.cpp index 7eaf704a..29354b9d 100644 --- a/siteupdate/cplusplus/classes/Route/Route.cpp +++ b/siteupdate/cplusplus/classes/Route/Route.cpp @@ -2,7 +2,7 @@ std::mutex Route::liu_mtx; std::mutex Route::ual_mtx; std::mutex Route::awf_mtx; -Route::Route(std::string &line, HighwaySystem *sys, ErrorList &el, std::list &all_regions) +Route::Route(std::string &line, HighwaySystem *sys, ErrorList &el, std::unordered_map ®ion_hash) { /* initialize object from a .csv file line, but do not yet read in waypoint file */ con_route = 0; @@ -28,9 +28,11 @@ Route::Route(std::string &line, HighwaySystem *sys, ErrorList &el, std::listsystemname + ".csv line: [" + line + "], expected 8 fields, found 2"); return; } - region = region_by_code(line.substr(left+1, right-left-1), all_regions); - if (!region) + try { region = region_hash.at(line.substr(left+1, right-left-1)); + } + catch (const std::out_of_range& oor) { el.add_error("Unrecognized region in " + system->systemname + ".csv line: [" + line + "], " + line.substr(left+1, right-left-1)); + region = 0; return; } diff --git a/siteupdate/cplusplus/classes/Route/Route.h b/siteupdate/cplusplus/classes/Route/Route.h index 006079cd..dc61608e 100644 --- a/siteupdate/cplusplus/classes/Route/Route.h +++ b/siteupdate/cplusplus/classes/Route/Route.h @@ -58,7 +58,7 @@ class Route double mileage; int rootOrder; - Route(std::string &, HighwaySystem *, ErrorList &, std::list &); + Route(std::string &, HighwaySystem *, ErrorList &, std::unordered_map &); std::string str(); void read_wpt(WaypointQuadtree *, ErrorList *, std::string, std::mutex *, DatacheckEntryList *, std::unordered_set *); diff --git a/siteupdate/cplusplus/siteupdate.cpp b/siteupdate/cplusplus/siteupdate.cpp index caa4539e..fc5ec386 100644 --- a/siteupdate/cplusplus/siteupdate.cpp +++ b/siteupdate/cplusplus/siteupdate.cpp @@ -173,6 +173,7 @@ int main(int argc, char *argv[]) //regions list all_regions; + unordered_map region_hash; filename = args.highwaydatapath+"/regions.csv"; file.open(filename.data()); if (!file) el.add_error("Could not open " + filename); @@ -180,6 +181,7 @@ int main(int argc, char *argv[]) while(getline(file, line)) { Region rg(line, countries, continents, el); if (rg.is_valid()) all_regions.push_back(rg); + region_hash[all_regions.back().code] = &all_regions.back(); } } file.close(); @@ -198,7 +200,7 @@ int main(int argc, char *argv[]) { ignoring.push_back("Ignored comment in " + args.systemsfile + ": " + line); continue; } - HighwaySystem *hs = new HighwaySystem(line, el, args.highwaydatapath+"/hwy_data/_systems", args.systemsfile, countries, all_regions); + HighwaySystem *hs = new HighwaySystem(line, el, args.highwaydatapath+"/hwy_data/_systems", args.systemsfile, countries, region_hash); // deleted on termination of program if (hs->is_valid()) highway_systems.push_back(hs); else delete hs; From b21fdef17c41a51ae01624e65927e24c5c910a10 Mon Sep 17 00:00:00 2001 From: eric bryant Date: Mon, 22 Apr 2019 13:18:45 -0400 Subject: [PATCH 3/5] remove concurrency_detection.cpp FIXME comment I think this referred to trying `concurrent` as an unordered_set. * That's actually slower * and results in non-deterministic segment_name()s. Nope! --- siteupdate/cplusplus/functions/concurrency_detection.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/siteupdate/cplusplus/functions/concurrency_detection.cpp b/siteupdate/cplusplus/functions/concurrency_detection.cpp index 907374bc..53cea378 100644 --- a/siteupdate/cplusplus/functions/concurrency_detection.cpp +++ b/siteupdate/cplusplus/functions/concurrency_detection.cpp @@ -26,8 +26,8 @@ for (HighwaySystem *h : highway_systems) } else { other->concurrent = s->concurrent; - std::list::iterator it = s->concurrent->begin(); //FIXME - while (it != s->concurrent->end() && *it != other) it++; //see HighwaySegment.h + std::list::iterator it = s->concurrent->begin(); + while (it != s->concurrent->end() && *it != other) it++; if (it == s->concurrent->end()) { s->concurrent->push_back(other); //concurrencyfile << "Added concurrency [" << s->str() << "]-[" \ From 88d12c5b0ac5a16c235354d383b04b10004c4322 Mon Sep 17 00:00:00 2001 From: eric bryant Date: Mon, 22 Apr 2019 15:28:56 -0400 Subject: [PATCH 4/5] remove canonical_waypoint_name.cpp FIXME comment Was reminder to change siteupdate.py. See 5635614284c35eeb5eb3cbfcca8ec9ec112e9e91 --- .../cplusplus/classes/Waypoint/canonical_waypoint_name.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/siteupdate/cplusplus/classes/Waypoint/canonical_waypoint_name.cpp b/siteupdate/cplusplus/classes/Waypoint/canonical_waypoint_name.cpp index 0643d430..014f6354 100644 --- a/siteupdate/cplusplus/classes/Waypoint/canonical_waypoint_name.cpp +++ b/siteupdate/cplusplus/classes/Waypoint/canonical_waypoint_name.cpp @@ -135,7 +135,7 @@ std::string Waypoint::canonical_waypoint_name(std::list &log) suffixes[other_index] = suffix; } } - else if (ap_coloc[check_index]->label.find(ap_coloc[other_index]->route->name_no_abbrev()) == 0) //FIXME -> elif in siteupdate.py + else if (ap_coloc[check_index]->label.find(ap_coloc[other_index]->route->name_no_abbrev()) == 0) { this_match = 1; if (strchr(ap_coloc[check_index]->label.data(), '_')) { std::string suffix = strchr(ap_coloc[check_index]->label.data(), '_'); From 95db2b6337be046167d066f1ba98e895dbae486f Mon Sep 17 00:00:00 2001 From: eric bryant Date: Mon, 22 Apr 2019 20:01:36 -0400 Subject: [PATCH 5/5] remove Waypoint.cpp FIXME comments Were reminders to change siteupdate.py. See b2de340757a2b1808e255640f733a1326330f20c --- siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp b/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp index 222ded1f..a52441d6 100644 --- a/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp +++ b/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp @@ -225,7 +225,7 @@ inline Waypoint* Waypoint::hashpoint() /* Datacheck */ inline void Waypoint::duplicate_label(DatacheckEntryList *datacheckerrors, std::unordered_set &all_route_labels) -{ // duplicate labels //FIXME *MAYBE* see if the list changes could help siteupdate.py a tiny bit? +{ // duplicate labels // first, check primary label std::string lower_label = lower(label); while (lower_label[0] == '+' || lower_label[0] == '*') lower_label.erase(lower_label.begin()); @@ -248,7 +248,7 @@ inline void Waypoint::duplicate_coords(DatacheckEntryList *datacheckerrors, std: if (!coords_used.insert(w).second) for (Waypoint *other_w : route->point_list) { if (this == other_w) break; - if (lat == other_w->lat && lng == other_w->lng /*&& label != other_w->label*/) //FIXME necessary? Try eliminating from siteupdate.py + if (lat == other_w->lat && lng == other_w->lng) { sprintf(fstr, "(%.15g,%.15g)", lat, lng); datacheckerrors->add(route, other_w->label, label, "", "DUPLICATE_COORDS", fstr); }