conveyal / analysis-backend

Server component of Conveyal Analysis
http://conveyal.com/analysis
MIT License
23 stars 12 forks source link

Regional and static-site analyses ignore remove-trips modification #200

Closed ansoncfit closed 5 years ago

ansoncfit commented 5 years ago

There should be a systematic difference in accessibility for the two scenarios being compared here, but there is only noise: image

In a static-site analysis of the scenario with the route removed, the route still appears in paths.

Single-point results show expected differences in isochrones/accessibility.

The route in question is frequency-based, and it is the only route in its feed.

ansoncfit commented 5 years ago

In a separate test project that also includes frequency-based routes (https://analysis-staging.conveyal.com/regions/5a2e9df732b98e221fef89eb/projects/5c1086a732b98e5d92c601cc), the regional results are more sensible. So maybe there is something particular about the feed used.

ansoncfit commented 5 years ago

This may be a bug related to the route being the only route in its feed. An excerpt of one of the remove-trip scenarios saved with one of the regional analyses:

comment: "Remove NB"
patterns: ["8559a4be-77dd-4333-a1ed-878e91f6ad67:northbound_weekend",…]
0: "8559a4be-77dd-4333-a1ed-878e91f6ad67:northbound_weekend"
1: "8559a4be-77dd-4333-a1ed-878e91f6ad67:northbound"
routes: null
trips: null
type: "remove-trips"
warnings: []

Routes and trips are null, and patterns are specified. That may be problematic (see https://github.com/conveyal/r5/issues/478).

ansoncfit commented 5 years ago

In the mongo modifications collection, this modification has an array of two (non-feed scoped) trips, but patterns: null.

It appears the implementations of com.conveyal.taui.models.Modification::toR5 (used only in modificationsForProject, which in turn is used only in populate task) save trips as patterns. For example, non-null trips are feed-scoped and saved to an array of patterns here: https://github.com/conveyal/analysis-backend/blob/dev/src/main/java/com/conveyal/taui/models/RemoveTrips.java#L25

This same pattern,

        if (trips == null) {
            as.routes = feedScopeIds(feed, routes);
        } else {
            as.patterns = feedScopeIds(feed, trips);
        }

was added to most modifications in https://github.com/conveyal/analysis-backend/commit/678088efc4a19184edbce257cee121df3da8342f

ansoncfit commented 5 years ago

Given the issues in https://github.com/conveyal/r5/pull/484, noise from the frequency-based route is likely what made any differences appear.

ansoncfit commented 5 years ago

https://github.com/conveyal/r5/pull/484