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

Add departure headings calculation #247

Merged
merged 4 commits into from
Aug 13, 2024
Merged

Conversation

nataliejschultz
Copy link
Contributor

In reference to #202:
Previously, generate_dataset would calculate the arrival heading using the preset osmnx .bearing method, while departure headings were set to None.

This PR adds two functions written by @nreinicke:

  1. calculate_bearings, which passes in linestrings and determines what to do with the coordinates based on the number of lat lon pairs
  2. compass_heading, which calculates a compass bearing based on two lat lon pairs

These functions replace the osmnx method used previously, rather than using osmnx for arrival and the functions for departure. Both headings are added to the same data frame and output to edges-headings-enumerated.csv

Add departure headings calculation and output departure headings as a separate column in edges-headings-enumerated.csv.
Copy link
Collaborator

@nreinicke nreinicke left a comment

Choose a reason for hiding this comment

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

This is looking great! I left a couple of small comments but I like how simple you've kept this

@@ -209,3 +206,38 @@ def replace_id(vertex_uuid):
with importlib.resources.as_file(model_file) as model_path:
model_dst = model_output_directory / model_path.name
shutil.copy(model_path, model_dst)

#Function written by Nick Reinicke
def compass_heading(point1, point2):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we move these functions into the utils module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we move these functions into the utils module?

Moved with commit b9f7726


return compass_bearing

#Function written by Nick Reinicke
Copy link
Collaborator

Choose a reason for hiding this comment

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

I appreciate the attribution but I'm okay if we omit them 🙂

Removed the two functions for calculating bearings and put them into utils.py.

Changed the calculate_bearings call accordingly.
Moving compass_heading and calculate_bearings functions over from generate_dataset.py
@nataliejschultz
Copy link
Contributor Author

I added the two functions to utils.py and changed the relevant command:

headings = [utils.calculate_bearings(i) for i in e.geometry.values]

For testing these changes locally, I added a name == main if statement to the bottom of the file with parameters to run:

if __name__ == '__main__':
    g = ox.graph_from_place("Denver, Colorado, USA", network_type="drive")
    testing_changes = generate_compass_dataset(g, 'testing-terminal')

Ran with python ./routee-compass/python/nrel/routee/compass/io/generate_dataset.py in terminal. After cd-ing into the testing-terminal output directory and using gunzip, here's what the top of the file looks like:

(base) nschultz-35728s:testing-terminal nschultz$ gunzip edges-headings-enumerated.csv.gz 
(base) nschultz-35728s:testing-terminal nschultz$ head edges-headings-enumerated.csv
arrival_heading,departure_heading
111,112
139,131
127,122
124,95
91,90
270,270
89,89
0,0
269,269

All is well!

Fixing utils import
@nataliejschultz
Copy link
Contributor Author

@nreinicke tests failed because the runner couldn't find utils, so I changed the import utils that worked on my machine to the more universal:

from nrel.routee.compass.io import utils

in commit b6cbd62. Hopefully this solves it!

Copy link
Collaborator

@nreinicke nreinicke left a comment

Choose a reason for hiding this comment

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

Excellent, looks good!

@nreinicke nreinicke merged commit 4d5cd01 into NREL:main Aug 13, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants