Refactor the OSRM/BirdDistance management #256
Conversation
include/euclideangeofilter.hpp
Outdated
| }; | ||
| } | ||
|
|
||
| #endif // TR_OSRM_GEO_FILTER |
| // Common utility functions | ||
| static std::tuple<float, float> calculateLengthOfOneDegree(const Point &point); | ||
| static float calculateMaxDistanceSquared(int maxWalkingTravelTime, float walkingSpeed); | ||
| //TODO WHy one point * and other & |
There was a problem hiding this comment.
Indeed good question, why? It was like that before?
There was a problem hiding this comment.
Yeah, that was just a copy paste from the previous code.
|
|
||
| std::string queryString = "/table/v1/" + mode + "/" + std::to_string(point.longitude) + "," + std::to_string(point.latitude); | ||
|
|
||
| for (auto &&[uuid,node] : nodes) |
There was a problem hiding this comment.
Could this part of the code actually use the euclidian distance? Instead of having to use the protected methods here to calculate something that really looks like the euclidian distance (I haven't compare the exact code).
There was a problem hiding this comment.
hum, I guess you are right, need to look at the code more, but it seems we are doing the euclidian first to filter the nodes and then filter those with OSRM.
(it hurts to know that the data is also in PostGIS usually, since that query would be so simple to do there. )
There was a problem hiding this comment.
Ok, re-read the code in more detail.
It does calculate the euclidian distance, but it does something different with the data. Using the common base functions seems the best way to go here. I'll try to add a comment
There was a problem hiding this comment.
@tahini Let me know if the comment I added explain the difference sufficiently.
The bird distance was renamed to euclidean. This allow to pass the desired object to the Calculator constructor and thus allowing us to select which method we want to use. This replace the global static variable. We don't use the cycling and driving mode so we instanciate an OsrmGeoFilter for those. The OSRM parameters (host/port) are now part of the OsrmGeoFilter object. AccessMode and EgressMode was always walking, we removed those variables from the Paramters object.
Setting the option --useEuclideanDistance=true will selection the EuclideanGeoFilter instead of the OsrmGeoFilter. This is mostly useful for testing and debugging, where you might not have an OSRM deamon running.
Validate that the server returned an 200 OK before attempting to parse the OSRM data. The function will return an empty array if the response was not valid
…er in the OSRMGeoFilter, in the first filter pass
greenscientist
force-pushed
the
birdseyeoption
branch
from
1235420 to
cf1956f
Compare
| protected: | ||
| // Common utility functions | ||
| static std::tuple<float, float> calculateLengthOfOneDegree(const Point &point); | ||
| static float calculateMaxDistanceSquared(int maxWalkingTravelTime, float walkingSpeed); |
There was a problem hiding this comment.
Eventually todo, don't refer to walking, it's just a travel time and speed
| Calculator calculator(transitData); | ||
|
|
||
| // Selection which geofilter to use. OSRM is the default one. Euclidean mostly used for debugging and testing | ||
| GeoFilter *geoFilter = 0; |
There was a problem hiding this comment.
This pointer never gets deleted. I guess it doesn't matter since it's constructed once in the main?
There was a problem hiding this comment.
Yeah, it should be cleaned, but it does not really matter as you said since it lives for the whole duration of the program.
This: