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.

Sign up for GitHub

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

Jormungandr: fix reverse isochrons #1082

Merged
merged 5 commits into from
Jul 9, 2015
Merged

Jormungandr: fix reverse isochrons #1082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f267a09
jormungandar: to entrypoint tested in journeys api
azime Jul 1, 2015
f65d25d
kraken: isochrone at from or to
azime Jul 1, 2015
a2c1741
Jormungandr: enable reverse isochrone
antoine-de Jul 2, 2015
f72eb3d
Raptor: fix reverse isochrone
antoine-de Jul 6, 2015
3046ea8
Jormungandr: update the test
antoine-de Jul 7, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions source/disruption/tests/disruption_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,6 @@ using navitia::type::new_disruption::PtObj;
using navitia::type::new_disruption::Disruption;
using navitia::type::new_disruption::Severity;

inline u_int64_t operator"" _dt_time_stamp(const char* str, size_t s) {
return navitia::to_posix_timestamp(boost::posix_time::from_iso_string(std::string(str, s)));
}

struct DisruptionCreator {
std::string uri;
std::string object_uri;
Expand Down Expand Up @@ -140,7 +136,7 @@ class Params {
holder.disruptions.push_back(std::move(disruption));
}

Params(): b("20120614"), end_date("20130614T000000"_dt_time_stamp) {
Params(): b("20120614"), end_date("20130614T000000"_pts) {
std::vector<std::string> forbidden;
b.vj_with_network("network:R", "line:A", "11111111", "", true, "")("stop_area:stop1", 8*3600 +10*60, 8*3600 + 11 * 60)
("stop_area:stop2", 8*3600 + 20 * 60 ,8*3600 + 21*60);
Expand Down Expand Up @@ -195,7 +191,7 @@ class Params {

BOOST_FIXTURE_TEST_CASE(network_filter1, Params) {
std::vector<std::string> forbidden_uris;
auto dt = "20131219T155000"_dt_time_stamp;
auto dt = "20131219T155000"_pts;
pbnavitia::Response resp = navitia::disruption::disruptions(*(b.data),
dt, 1, 10, 0, "network.uri=network:R", forbidden_uris);

Expand All @@ -221,7 +217,7 @@ BOOST_FIXTURE_TEST_CASE(network_filter1, Params) {

BOOST_FIXTURE_TEST_CASE(network_filter2, Params) {
std::vector<std::string> forbidden_uris;
auto dt = "20131224T125000"_dt_time_stamp;
auto dt = "20131224T125000"_pts;
pbnavitia::Response resp = navitia::disruption::disruptions(*(b.data),
dt, 1, 10, 0, "network.uri=network:K", forbidden_uris);

Expand All @@ -242,7 +238,7 @@ BOOST_FIXTURE_TEST_CASE(network_filter2, Params) {

BOOST_FIXTURE_TEST_CASE(line_filter, Params) {
std::vector<std::string> forbidden_uris;
auto dt = "20131221T085000"_dt_time_stamp;
auto dt = "20131221T085000"_pts;
pbnavitia::Response resp = navitia::disruption::disruptions(*(b.data),
dt, 1 ,10 ,0 , "line.uri=line:S", forbidden_uris);

Expand All @@ -267,15 +263,15 @@ BOOST_FIXTURE_TEST_CASE(line_filter, Params) {

BOOST_FIXTURE_TEST_CASE(Test1, Params) {
std::vector<std::string> forbidden_uris;
auto dt = "20140101T0900"_dt_time_stamp;
auto dt = "20140101T0900"_pts;
pbnavitia::Response resp = navitia::disruption::disruptions(*(b.data),
dt, 1, 10, 0, "", forbidden_uris);
BOOST_REQUIRE_EQUAL(resp.response_type(), pbnavitia::ResponseType::NO_SOLUTION);
}

BOOST_FIXTURE_TEST_CASE(Test2, Params) {
std::vector<std::string> forbidden_uris;
auto dt = "20131226T0900"_dt_time_stamp;
auto dt = "20131226T0900"_pts;
pbnavitia::Response resp = navitia::disruption::disruptions(*(b.data),
dt, 1, 10, 0, "", forbidden_uris);
BOOST_REQUIRE_EQUAL(resp.disruptions_size(), 1);
Expand All @@ -294,15 +290,15 @@ BOOST_FIXTURE_TEST_CASE(Test2, Params) {
BOOST_FIXTURE_TEST_CASE(Test4, Params) {
std::cout << "bob 4?" << std::endl;
std::vector<std::string> forbidden_uris;
auto dt = "20130203T0900"_dt_time_stamp;
auto dt = "20130203T0900"_pts;
pbnavitia::Response resp = navitia::disruption::disruptions(*(b.data),
dt, 1 , 10, 0, "", forbidden_uris);
BOOST_REQUIRE_EQUAL(resp.response_type(), pbnavitia::ResponseType::NO_SOLUTION);
}

BOOST_FIXTURE_TEST_CASE(Test5, Params) {
std::vector<std::string> forbidden_uris;
auto dt = "20130212T0900"_dt_time_stamp;
auto dt = "20130212T0900"_pts;
pbnavitia::Response resp = navitia::disruption::disruptions(*(b.data),
dt, 1, 10, 0, "", forbidden_uris);

Expand Down
8 changes: 3 additions & 5 deletions source/jormungandr/jormungandr/interfaces/v1/Journeys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,9 +626,8 @@ def get(self, region=None, lon=None, lat=None, uri=None):
abort(503, message="Unable to compute journeys "
"from this object")

if not args["origin"]: #@vlara really ? I though we could do reverse isochrone ?
#shoudl be in my opinion if not args["origin"] and not args["destination"]:
abort(400, message="from argument is required")
if not (args['destination'] or args['origin']):
abort(400, message="you should at least provide either a 'from' or a 'to' argument")

#we transform the origin/destination url to add information
if args['origin']:
Expand All @@ -646,8 +645,7 @@ def get(self, region=None, lon=None, lat=None, uri=None):
else:
possible_regions = [region]

api = None
if args['destination']:
if args['destination'] and args['origin']:
api = 'journeys'
else:
api = 'isochrone'
Expand Down
2 changes: 1 addition & 1 deletion source/jormungandr/tests/authentication_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def test_unkown_region(self):
the authentication process must not mess if the region is not found
"""
with user_set(app, 'bob'):
r, status = self.query_no_assert('/v1/coverage/the_marvelous_unknown_region/stop_areas', display=True)
r, status = self.query_no_assert('/v1/coverage/the_marvelous_unknown_region/stop_areas')

assert status == 404
assert 'error' in r
Expand Down
43 changes: 29 additions & 14 deletions source/jormungandr/tests/isochrone_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,41 +43,56 @@ def test_from_isochrone_coord(self):
#not to use the jormungandr database
query = "v1/coverage/basic_routing_test/journeys?from={}&datetime={}"
query = query.format(s_coord, "20120614T080000")
self.query(query, display=False)
self.query(query)

def test_stop_point_isochrone_coord(self):
#NOTE: we query /v1/coverage/basic_routing_test/journeys and not directly /v1/journeys
#not to use the jormungandr database
query = "v1/coverage/basic_routing_test/stop_points/A/journeys?max_duration=400&datetime=20120614T080000"
response = self.query(query, display=False)
response = self.query(query)
assert len(response["journeys"]) == 1
assert response["journeys"][0]["duration"] == 300
assert response["journeys"][0]["to"]["stop_point"]["id"] == "B"
query = "v1/coverage/basic_routing_test/stop_points/A/journeys?max_duration=25500&datetime=20120614T080000"
response = self.query(query, display=False)
response = self.query(query)
assert len(response["journeys"]) == 2
assert response["journeys"][0]["duration"] == 300
assert response["journeys"][0]["to"]["stop_point"]["id"] == "B"
assert response["journeys"][1]["duration"] == 25200
assert response["journeys"][1]["to"]["stop_point"]["id"] == "D"

def test_to_isochrone_coord(self):
#NOTE: we query /v1/coverage/basic_routing_test/journeys and not directly /v1/journeys
#not to use the jormungandr database
query = "v1/coverage/basic_routing_test/journeys?from={}&datetime={}&datetime_represents=arrival"
query = "v1/coverage/basic_routing_test/journeys?from={}&datetime={}"
query = query.format(s_coord, "20120614T080000")
self.query(query, display=False)
self.query(query)

def test_from_isochrone_sa(self):
#NOTE: we query /v1/coverage/basic_routing_test/journeys and not directly /v1/journeys
#not to use the jormungandr database
query = "v1/coverage/basic_routing_test/journeys?from={}&datetime={}"
query = query.format("stopA", "20120614T080000")
self.query(query, display=False)
self.query(query)

def test_to_isochrone_sa(self):
#NOTE: we query /v1/coverage/basic_routing_test/journeys and not directly /v1/journeys
#not to use the jormungandr database
query = "v1/coverage/basic_routing_test/journeys?from={}&datetime={}&datetime_represents=arrival"
query = "v1/coverage/basic_routing_test/journeys?to={}&datetime={}&datetime_represents=arrival"
query = query.format("stopA", "20120614T080000")
self.query(query, display=False)
self.query(query)

def test_reverse_isochrone_coord(self):
q = "v1/coverage/basic_routing_test/journeys?max_duration=100000" \
"&datetime=20120615T200000&datetime_represents=arrival&to=D"
normal_response = self.query(q, display=True)
assert len(normal_response["journeys"]) == 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must test the datetimes somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is tested in routing_api_test


def test_reverse_isochrone_coord_clockwise(self):
q = "v1/coverage/basic_routing_test/journeys?datetime=20120614T080000&to=A"
normal_response, error_code = self.query_no_assert(q)

assert error_code == 404
assert normal_response['error']['message'] == 'reverse isochrone works only for anti-clockwise request'


def test_isochrone_non_clockwise(self):
q = "v1/coverage/basic_routing_test/journeys?datetime=20120614T080000&from=A&datetime_represents=arrival"
normal_response, error_code = self.query_no_assert(q)

assert error_code == 404
assert normal_response['error']['message'] == 'isochrone works only for clockwise request'
9 changes: 9 additions & 0 deletions source/jormungandr/tests/routing_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ def test_error_on_journeys(self):
#and no journey is to be provided
assert 'journeys' not in response or len(response['journeys']) == 0

def test_missing_params(self):
"""we should at least provide a from or a to on the /journeys api"""
query = "journeys?datetime=20120614T080000"

response, status = self.query_no_assert("v1/coverage/main_routing_test/" + query)

assert status == 400
get_not_null(response, 'message')

def test_best_filtering(self):
"""Filter to get the best journey, we should have only one journey, the best one"""
query = "{query}&type=best".format(query=journey_basic_query)
Expand Down
83 changes: 57 additions & 26 deletions source/kraken/worker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,18 +524,24 @@ pbnavitia::Response Worker::journeys(const pbnavitia::JourneysRequest &request,
}

std::vector<type::EntryPoint> destinations;
if(api != pbnavitia::ISOCHRONE) {
for (int i = 0; i < request.destination().size(); i++) {
Type_e destination_type = data->get_type_of_id(request.destination(i).place());
type::EntryPoint destination = type::EntryPoint(destination_type, request.destination(i).place(), request.destination(i).access_duration());

if (destination.type == type::Type_e::Address || destination.type == type::Type_e::Admin
|| destination.type == type::Type_e::StopArea || destination.type == type::Type_e::StopPoint
|| destination.type == type::Type_e::POI) {
destination.coordinates = this->coord_of_entry_point(destination, data);
}
destinations.push_back(destination);
for (int i = 0; i < request.destination().size(); i++) {
Type_e destination_type = data->get_type_of_id(request.destination(i).place());
type::EntryPoint destination = type::EntryPoint(destination_type, request.destination(i).place(), request.destination(i).access_duration());

if (destination.type == type::Type_e::Address || destination.type == type::Type_e::Admin
|| destination.type == type::Type_e::StopArea || destination.type == type::Type_e::StopPoint
|| destination.type == type::Type_e::POI) {
destination.coordinates = this->coord_of_entry_point(destination, data);
}
destinations.push_back(destination);
}

if (origins.empty() && destinations.empty()) {
//should never happen, jormungandr filters that, but it never hurts to double check
pbnavitia::Response response;
fill_pb_error(pbnavitia::Error::no_origin_nor_destination, "no origin point nor destination point given", response.mutable_error());
response.set_response_type(pbnavitia::NO_ORIGIN_NOR_DESTINATION_POINT);
return response;
}

std::vector<std::string> forbidden;
Expand All @@ -549,38 +555,63 @@ pbnavitia::Response Worker::journeys(const pbnavitia::JourneysRequest &request,
/// params for departure fallback
for(size_t i = 0; i < origins.size(); i++) {
type::EntryPoint &origin = origins[i];
if ((origin.type == type::Type_e::Address) || (origin.type == type::Type_e::Coord)
|| (origin.type == type::Type_e::Admin) || (origin.type == type::Type_e::POI) || (origin.type == type::Type_e::StopArea)){
origin.streetnetwork_params = this->streetnetwork_params_of_entry_point(request.streetnetwork_params(), data);
if ((origin.type == type::Type_e::Address)
|| (origin.type == type::Type_e::Coord) || (origin.type == type::Type_e::Admin)
|| (origin.type == type::Type_e::POI) || (origin.type == type::Type_e::StopArea)) {
origin.streetnetwork_params = this->streetnetwork_params_of_entry_point(
request.streetnetwork_params(), data);
}
}
/// params for arrival fallback
for(size_t i = 0; i < destinations.size(); i++) {
type::EntryPoint &destination = destinations[i];
if ((destination.type == type::Type_e::Address) || (destination.type == type::Type_e::Coord)
|| (destination.type == type::Type_e::Admin) || (destination.type == type::Type_e::POI) || (destination.type == type::Type_e::StopArea)){
destination.streetnetwork_params = this->streetnetwork_params_of_entry_point(request.streetnetwork_params(), data, false);
|| (destination.type == type::Type_e::Admin) || (destination.type == type::Type_e::POI)
|| (destination.type == type::Type_e::StopArea)) {
destination.streetnetwork_params = this->streetnetwork_params_of_entry_point(
request.streetnetwork_params(), data, false);
}
}
/// Accessibilité, il faut initialiser ce paramètre
//HOT FIX degueulasse
type::AccessibiliteParams accessibilite_params;
accessibilite_params.properties.set(type::hasProperties::WHEELCHAIR_BOARDING, request.wheelchair());
switch(api) {
case pbnavitia::ISOCHRONE:
return navitia::routing::make_isochrone(*planner, origins[0], request.datetimes(0),
request.clockwise(), accessibilite_params,
forbidden, *street_network_worker,
request.disruption_active(), request.max_duration(),
request.max_transfers(), request.show_codes());
case pbnavitia::NMPLANNER:
return routing::make_nm_response(*planner, origins, destinations, datetimes[0],
case pbnavitia::ISOCHRONE: {
type::EntryPoint ep;
if (! origins.empty()) {
if (! request.clockwise()) {
// isochrone works only on clockwise
pbnavitia::Response response;
fill_pb_error(pbnavitia::Error::bad_format, "isochrone works only for clockwise request", response.mutable_error());
response.set_response_type(pbnavitia::NO_SOLUTION);
return response;
}
ep = origins[0];
} else {
if (request.clockwise()) {
// isochrone works only on clockwise
pbnavitia::Response response;
fill_pb_error(pbnavitia::Error::bad_format, "reverse isochrone works only for anti-clockwise request", response.mutable_error());
response.set_response_type(pbnavitia::NO_SOLUTION);
return response;
}
ep = destinations[0];
}
return navitia::routing::make_isochrone(*planner, ep, request.datetimes(0),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to specify if ep is the origin or the destination?

request.clockwise(), accessibilite_params,
forbidden, *street_network_worker,
request.disruption_active(), request.max_duration(),
request.max_transfers(), request.show_codes());
}
case pbnavitia::NMPLANNER:
return routing::make_nm_response(*planner, origins, destinations, datetimes[0],
request.clockwise(), accessibilite_params,
forbidden, *street_network_worker,
request.disruption_active(), request.max_duration(),
request.max_transfers(), request.show_codes());
default:
return routing::make_response(*planner, origins[0], destinations[0], datetimes,
default:
return routing::make_response(*planner, origins[0], destinations[0], datetimes,
request.clockwise(), accessibilite_params,
forbidden, *street_network_worker,
request.disruption_active(), request.max_duration(),
Expand Down
2 changes: 1 addition & 1 deletion source/routing/raptor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ RAPTOR::isochrone(const vec_stop_point_duration& departures,
forbidden,
disruption_active);
clear(clockwise, bound);
init(departures, departure_datetime, true, accessibilite_params.properties);
init(departures, departure_datetime, clockwise, accessibilite_params.properties);

boucleRAPTOR(accessibilite_params, clockwise, max_transfers);
}
Expand Down
24 changes: 20 additions & 4 deletions source/routing/raptor_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,11 +638,21 @@ static void add_isochrone_response(RAPTOR& raptor,
continue;
}
auto pb_journey = response.add_journeys();
const auto str_departure = to_posix_timestamp(best_lbl, raptor.data);
const auto str_arrival = to_posix_timestamp(best_lbl, raptor.data);

uint64_t departure;
uint64_t arrival;
// Note: since there is no 2nd pass for the isochrone, the departure dt
// is the requested dt (or the arrival dt for non clockwise)
if (clockwise) {
departure = to_posix_timestamp(init_dt, raptor.data);
arrival = to_posix_timestamp(best_lbl, raptor.data);
} else {
departure = to_posix_timestamp(best_lbl, raptor.data);
arrival = to_posix_timestamp(init_dt, raptor.data);
}
const auto str_requested = to_posix_timestamp(init_dt, raptor.data);
pb_journey->set_arrival_date_time(str_arrival);
pb_journey->set_departure_date_time(str_departure);
pb_journey->set_arrival_date_time(arrival);
pb_journey->set_departure_date_time(departure);
pb_journey->set_requested_date_time(str_requested);
pb_journey->set_duration(duration);
pb_journey->set_nb_transfers(round - 1);
Expand Down Expand Up @@ -1062,6 +1072,12 @@ pbnavitia::Response make_isochrone(RAPTOR &raptor,
return journey1.destination().uri() < journey2.destination().uri();
});

if (response.journeys().size() == 0) {
fill_pb_error(pbnavitia::Error::no_solution, "no solution found for this isochrone",
response.mutable_error());
response.set_response_type(pbnavitia::NO_SOLUTION);
}


return response;
}
Expand Down
Loading