e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

unique_modes = [TRACKS, TRAIN], but we support only two, [BUS, TRAIN] #426

Open PatGendre opened 5 years ago

PatGendre commented 5 years ago

this exception was raised with the data of Susanne, who tests the fabmob on iOS in Berlin. This is in the mode detection pipeline in the ground_truth_matching branch. As the error prevents the pipeline to end properly, I added an horrible so that the exception is not raised if sorted(unique_modes) != ['BUS', 'TRAIN']: unique_modes = ['BUS', 'TRAIN'] for the short term...

I suspect the TRACKS mode is something special to iOS ? I've never seem that in Android pipeline logs.

shankari commented 5 years ago

as you can see, the modes listed here are inferred modes (e.g. BUS, TRAIN) that we get from the GIS integration with OSM. Since the UI currently only supports the big 5 modes, we map the modes from OSM to TRAIN here https://github.com/e-mission/e-mission-server/pull/578/files#diff-b64a9eb8dbe2ce052d4a1779485f7abdR190

I have already handled all the train-related entries listed in the documentation https://wiki.openstreetmap.org/wiki/Public_transport#Route_Master_relations but it looks like it is not comprehensive and new ones turn up from time to time (e.g. funicular).

TRACKS is another one that we haven't seen before. You should be able to figure out the mapped OSM relation from the logs e.g. where this is printed out https://github.com/e-mission/e-mission-server/pull/578/files#diff-be667dc3c2a2b8a98b1810f4acb829bfR43 and if it is indeed a train, you can add it to the list of train modes

I also pointed out to @tms-martin that retaining the original modes instead of collapsing them could give us a much better carbon estimate. However, we would then need the carbon intensity for every possible mode, including FUNICULAR and TRACKS, and I'm not sure we have them :)

shankari commented 5 years ago

I explicitly added that ASSERT so that we could figure out such undocumented modes and map them to the supported modes.

shankari commented 5 years ago

re-opening so that we can add TRACKS to the list if it turns out that it is in fact a train.

PatGendre commented 5 years ago

Hi, thanks a lot for the explanations. It is not clear to me what tracks is in OSM : it seems to be only an unpaved way https://wiki.openstreetmap.org/wiki/Track not a transportation mode?

I've also found a tracks for a tramway line, for counting the number of tracks and only one stop in Berlin (where the tester lives) with this argument https://www.openstreetmap.org/node/247116372

but if I understand well the mode is determined here https://github.com/shankari/e-mission-server/blob/dc60ccdfab34e0be4fe76154911243e648085672/emission/net/ext_service/transit_matching/match_stops.py#L82 according the value of the railway tag. In case of tramway the value should be tram_stop not tracks, I believe the TRACKS should be safely ignored, this is not a correct tag.

However, tram is very common in Europe and I don't think it is available (I remarked that it was not detected while in Paris), this would be definitely useful to add it (and maybe not too difficult? at least in the pipeline mode detection stage ; I guess it would also imply to adapt the UI...).

So if you close this ticket I could reopen one for this enhancement : adding tram to the mode list, what do you think?

shankari commented 5 years ago

@PatGendre we only fall back to "railway" if we don't find a matching relation

In this code, we find all the route relations between the start and end points. And if there is a matching route, then we return its route tag.

    p_start_routes = list(itertools.chain.from_iterable([extract_routes(s) for s in start_stops]))
    p_end_routes = list(itertools.chain.from_iterable([extract_routes(s) for s in end_stops]))

    rel_id_matches = get_rel_id_match(p_start_routes, p_end_routes)
    logging.debug("len(start_routes) = %d, len(end_routes) = %d, len(rel_id_matches) = %d" %
        (len(p_start_routes), len(p_end_routes), len(rel_id_matches)))
    if len(rel_id_matches) > 0:
        return [rim.tags.route for rim in rel_id_matches]

e.g. if we find this route relation https://www.openstreetmap.org/relation/6274531#map=13/43.3096/5.4020&layers=TD then we read its tags.route to return subway, which gets mapped to TRAIN in the final step.

similarly, when we find this route relation https://www.openstreetmap.org/relation/5296538#map=14/43.3008/5.3753&layers=TD we read its tags.route to return tram, which gets mapped to TRAIN in the final step.

So I think that that somewhere in Germany, there is a route relation that is tagged as TRACKS. We won't return TRACKS as a mode otherwise - the fallback mechanisms will return TRAIN or BUS directly.

If you still have the logs, you should be able to search for the string matching routes = closest to the error and figure out what the matching route ID was, and then plug it into OSM to see what it is.

PatGendre commented 5 years ago

Ok, thanks, I understand better ! route=tracks seems the official way of describing a railway in OSM, at least for Germany and the german speaking countries, who proposed these rules for OSM tagging: https://wiki.openstreetmap.org/wiki/DE:Tag:route%3Dtracks So there are chances that this concerns also @jf87 in Heidelberg? I will check in the logs tomorrow if I see something.

And the code detects tram but collapses it with train, Ok. Nevertheless I have taken the tram 3 times in Paris with the app and this was never detected, as far as I remember. I'll have to look at how the tram routes are described in OSM in Paris...

And even if not a priority and possibly a quite large effort, it would be appreciated by several users that a distinction is made between TRAIN and TRAM (when we find resources for that ;-)

shankari commented 5 years ago

So I used overpass to figure this out! Using http://overpass-turbo.eu/ with the query

[out:json];

(
  relation[route=tracks]({{bbox}});
);

out body;
>;

I found quite a few route relations in the Berlin area with tags.route = tracks. The first one was https://www.openstreetmap.org/relation/193465, the last one was https://www.openstreetmap.org/relation/9736426. The full list is attached as a text file.

all_tracks_around_berlin.txt

PatGendre commented 5 years ago

Thanks again, I've tried overpass but did not take the time to understand the syntax :-(

Le mer. 3 juil. 2019 22:17, shankari notifications@github.com a écrit :

So I used overpass to figure this out! Using http://overpass-turbo.eu/ with the query

[out:json];

( // get cycle route relatoins relationroute=tracks; );

out body;

;

I found quite a few route relations in the Berlin area with tags.route = tracks. The first one was https://www.openstreetmap.org/relation/193465, the last one was https://www.openstreetmap.org/relation/9736426. The full list is attached as a text file.

all_tracks_around_berlin.txt https://github.com/e-mission/e-mission-docs/files/3356703/all_tracks_around_berlin.txt

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/e-mission/e-mission-docs/issues/426?email_source=notifications&email_token=ABGOKV6UEEEDYKZHRR6SJZDP5UCN3A5CNFSM4H5FPWS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZFSDAQ#issuecomment-508240258, or mute the thread https://github.com/notifications/unsubscribe-auth/ABGOKV34RXZ5K3UX3UCQDQDP5UCN3ANCNFSM4H5FPWSQ .

shankari commented 5 years ago

In terms of next steps:

One approach may be the one that the Sydney team is using for displaying the results from the complex survey (e.g. similar to test_survey_launch.mp4.gz)

But I think that we should open a new issue for discussing that long-term enhancement

PatGendre commented 5 years ago

@Shankari , I checked to intake logs, currently there are 20 hits pour OSM routes tagged as 'tracks', and maybe 3 or 4 different in fact, only in Berlin (not in France, as I suspected). Only one person tested in Germany, et a few hours only, so yes it will be a good idea to add tracks as a variant of TRAIN, as it seems quite quick to do. Note that for route:'tracks', it seems it is not tram but rather metro or local train... For example 'routes': [{'id': 6636477, 'tags': {'loc_name': 'Hundekopf', 'name': '6020 Berliner Ringbahn (S-Bahn)', 'operator': 'DB Netz AG', 'ref': '6020', 'route': 'tracks', 'type': 'route', 'wikipedia': 'de:Berliner Ringbahn'}}, {'id': 7375631, 'tags': {'from': 'Berlin Nordbahnhof Strw 6032/6002', 'name': '6032 Berlin Nordbahnhof Strw - Berlin-Schöneberg', 'operator': 'DB Netz AG', 'ref': '6032', 'route': 'tracks', 'to': 'Berlin-Schöneberg', 'type': 'route'}}, {'id': 7375632, 'tags': {'from': 'Berlin-Schöneberg (Vorortbahn)', 'name': '6033 Schöneberg - Wannsee', 'operator': 'DB Netz AG', 'ref': '6033', 'route': 'tracks', 'to': 'Berlin-Wannsee S-Bahn', 'type': 'route'}}] https://www.openstreetmap.org/relation/6636477

As for support for tram and metro, as a longer term enhancement Ok for the proposal, thanks, I will add an issue. It implies a lot of questions wrt UI. It is nice to look at the Sydney enketo survey, they did a great job! I wonder how proceed for asking the mode of a multimodal trip ; same question as with the mode button.

PatGendre commented 5 years ago

Hi, I added 'tracks' to the train mode list in L191 in rule_engine.py it seems safe to include the tag route='tracks' as it looks like widely used in the German OSM community. Is this the only simple thing to do for closing the issue?

I cannot create directly a PR in your repo as our fabmob repo is forked from e-mission, not from your repo (unless we should also fork your repo for PRs to the ground truth matching branch?).

shankari commented 5 years ago

@PatGendre I think I will merge the ground_truth_matching to a different branch on the e-mission server repo. I am not going to get to implement the beautiful pipeline stage choice architecture that I have in my head into master in the short-term.

shankari commented 5 years ago

@PatGendre new branch is at https://github.com/e-mission/e-mission-server/tree/gis-based-mode-detection also synced with master. So you can submit a PR to that branch.

Is this the only simple thing to do for closing the issue?

Yes!