arup-group / pam

Generate and modify transport demand scenarios via a Python API.
MIT License
55 stars 21 forks source link

facility link snapping #276

Closed Theodore-Chatziioannou closed 1 month ago

Theodore-Chatziioannou commented 4 months ago

Adds CLI method for snapping activity facilities to a network geometry.

It can be invoked as: pam snap-facilities <path_population_in> <path_population_out> <path_network_geometry> -f <link_id_field>

pam snap-facilities --help

Usage: pam snap-facilities [OPTIONS] PATH_POPULATION_IN PATH_POPULATION_OUT
                           PATH_NETWORK_GEOMETRY

  Snap facilities to a network geometry.

Options:
  -f, --link_id_field TEXT  The link ID field to use in the network shapefile.
                            Defaults to 'id'.
  --help                    Show this message and exit.

For each activity, the method will identify the closest link in the shapefile, and add it as a link=.. attribute to the activity element. Finally, the updated plans file will be saved to a new xml file.

Checklist

Any checks which are not relevant to the PR can be pre-checked by the PR creator. All others should be checked by the reviewer(s). You can add extra checklist items here if required by the PR.

Theodore-Chatziioannou commented 4 months ago

thanks @brynpickering - I have made some updates in response to your comments. Before merging, @gac55 let me know if it works for your use case.

gac55 commented 4 months ago

Thanks @Theodore-Chatziioannou , picking this up now

syhwawa commented 2 months ago

Hi @Theodore-Chatziioannou,

I've tested the new pam snap-facilities function with 100 agents' plan for Londinium, and it worked quite well!

When I tried it on TE pop, I noticed it perform quite slow when scaling up to larger populations. It took about 30 minutes of runtime for 0.01% of the population for TE pop.

I feel the primary slowdown seems to come from the way distances are calculated for each activity to all network links. Maybe some vectorized operations or something similar would help to potential speed-ups to process?

Theodore-Chatziioannou commented 2 months ago

thanks @syhwawa

syhwawa commented 2 months ago

Thanks for the comments @Theodore-Chatziioannou

The function has been updated to remove the separate loops for collecting and applying data. Now, the updated locations are queried and assigned directly within the first loop. And the lint error has been fixed.