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.

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

Penalty being applied for both boarding and alighting transit route #40

Closed
kuanb opened this issue Sep 26, 2017 · 3 comments
Closed

Penalty being applied for both boarding and alighting transit route #40

kuanb opened this issue Sep 26, 2017 · 3 comments

Comments

@kuanb
Copy link
Member

kuanb commented Sep 26, 2017

Heads up that the internal logic suggests that, while the boarding penalty (1/2 headway) from OSM network to transit network (ped_to_transit_edges_df) was intended to be applied to only that direction, the argument being passed to it (net_connector_edges) is actually both to and from edges. Thus, the library is applying a penalty for both the boarding and alighting from a transit route line onto a walk network.

Location of the issue:

net_connector_edges = _connector_edges(
osm_nodes=urbanaccess_network.osm_nodes,
transit_nodes=urbanaccess_network.transit_nodes,
travel_speed_mph=3)
urbanaccess_network.net_connector_edges = _add_headway_impedance(
ped_to_transit_edges_df=net_connector_edges,
headways_df=urbanaccess_gtfsfeeds_df.headways,
headway_statistic=headway_statistic)

@sablanchard
Copy link
Contributor

sablanchard commented Sep 26, 2017

Hi @kuanb thanks for the report. I have quickly looked into it and I cannot replicate the issue it all looks like it should be correct to me.

What is happening is:
The mean headway gets calculated for each route stop pair then it gets joined to the connector edge table by using only the to edge id where in this case only ids that match the route stop ids in the two tables are joined - the osm ids are not joined on as these are not route stop ids thus resulting in nans for the mean. This results in the df below that eventually turns into the connector edge table:

image

Note the osm to transit edges have a value in the mean column and that value has divided by 2 and been added to the weight column and the transit to osm edges have nans and the weight column for those is just the walking time. When the mean is applied to the weight column only the mean values that are not nan are added to the weight which in this case are only the osm to transit edges.

I hope that makes it clear, let me know if you are seeing something different. If so, please send a script of the full workflow so that we can replicate it and highlight the values in the df table of interest. If you cannot replicate and this explanation makes sense also let me know and I will close this issue.

@kuanb
Copy link
Member Author

kuanb commented Sep 26, 2017

Thanks Sam.

Given your above message, it sounds like the intent of these lines:

osm_to_transit_wheadway['weight_tmp'].fillna(
osm_to_transit_wheadway['weight'], inplace=True)

Is to replace the NaN values with just the original calculated walk time.

If that is the case, then we may want to make that explicit and enforce it as only on that direction by updating using a Pandas .loc[] lookup to replace just those values.

In addition, we’ll want to make that explicit in the parameters, too:

    Parameters
    ----------
    ped_to_transit_edges_df : pandas.DataFrame
        DataFrame of the osm to transit connectors

Should acknowledge that what is being input is the combined (bi-directional) edges and then a add’tl kwargs could be edge_tag_to_add_headway=‘osm_to_transit’ and then we can .loc by all tags that are not that and make their weighting just the value of distance.

This is also an opportunity to parameterize how distance/cost for the connector is calculated. For example, one could defensibly argue for no-cost (weight 0) connectors from transit to OSM network (plus headway cost). Parameterization would enable this customization.

@sablanchard
Copy link
Contributor

sablanchard commented Sep 26, 2017

Thanks @kuanb yes that is the intent of those lines. What you suggest is a clearer and cleaner way of doing that. You are correct, this would bring us one step closer to allowing custom parameterization of the connector cost.

One issue is that all the functions in urbanaccess assume travel cost in terms of time not distance so more work would have to go into making time a optional weight parameter throughout the tool so that raw distance could be used throughout. Distance is calculated throughout its just not exposed to the user via parameters.

I will close this issue as the headways are being calculated correctly as intended but feel free to post in this open issue: #36 the details on how to achieve the parameterization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants