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

Implemented performance improvement to access scores API call. #3415

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

davphan
Copy link
Collaborator

@davphan davphan commented Oct 25, 2023

Resolves #2988

Before, the computation for the access streets score was done as follows:

for every street s:
    for every clustered attribute c:
        create a jts Point object from the c's latitude and longitude, Point(c)
        if the Point(c) is within 10 meters of s:
            increase cluster count for s

which duplicated the Point creation step for all labels on every street. The computation was modified to follow this scheme:

for every clustered attribute c:
    create a jts Point object from the c's latitude and longitude, Point(c)
for every street s:
    for every Point(c):
        if Point(c) is within 10 meters of s:
            increase cluster count for s

to reduce the amount of creations of Point objects to just one per label.

Testing instructions
  1. Go to the API page (http://localhost:9000/api) and use the drop down in the "Access Score: Streets" section to get the scores for the entire city (or use this link, http://[localhost:9000/v2/access/score/streets?filetype=geojson) and verify that the API call still works.
Things to check before submitting the PR
  • I've written a descriptive PR title.
  • I've added/updated comments for large or confusing blocks of code.

@misaugstad
Copy link
Member

We talked about this during the meeting today, but as a reminder:

  1. Can you check that the output is exactly the same?
  2. Can you check what sort of performance improvement you get? I think that you have the full Pittsburgh database that is pretty recent, so I'm hoping that there is enough data to be able to measure a difference. Maybe run it 5 times or so with each version to get an average time for the computation!

@davphan
Copy link
Collaborator Author

davphan commented Oct 31, 2023

Performance benchmarks:
Runtime calculated using System.nano() at the beginning and end of the computeAccessScoresForStreets method, averaging 5 runs with no caching.
Before update: 7.62 s average runtime
After update: 2.87 s average runtime

Output difference:
Checked for any differences between the geojson file output.
Exactly the same, order and data wise!

@misaugstad
Copy link
Member

Awesome, thank you @davphan! Will test (and I'm assuming merge) tomorrow!

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Nicely done!!

@misaugstad misaugstad merged commit da609b9 into develop Nov 1, 2023
@misaugstad misaugstad deleted the 2988-streets-performance branch November 1, 2023 19:22
@misaugstad misaugstad mentioned this pull request Nov 1, 2023
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.

small performance improvement for streets API
2 participants