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

small performance improvement for streets API #2988

Closed
misaugstad opened this issue Aug 9, 2022 · 3 comments · Fixed by #3415
Closed

small performance improvement for streets API #2988

misaugstad opened this issue Aug 9, 2022 · 3 comments · Fixed by #3415

Comments

@misaugstad
Copy link
Member

Brief description of problem/feature

I was creating some documentation for our streets API, and I noticed a pretty obvious and easy to correct inefficiency in how we are doing it. The relevant parts of the algorithm basically look like...

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

But we could easily just create the Point objects for all of the attributes just once, instead of for every street. I imagine that this could have a pretty big overhead if we are using this on a large area. It should look like this...

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

I can only assume that we do this for the neighborhoods API as well, but it shouldn't be as big of a deal there since a city generally has 10's of neighborhoods but 1000's of streets.

FYI I'm looking at the computeAccessScoresForStreets() function in ProjectSidewalkAPIController.scala.

@jonfroehlich
Copy link
Member

I'm all for performance improvements, and I love how your brain is constantly looking for ways to do things better! 🧠

@davphan
Copy link
Collaborator

davphan commented Oct 25, 2023

Can every label only be assigned to one street? If so, can we do something like:

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

to further increase performance? Or do we need to loop through every single point for every street?

@misaugstad
Copy link
Member Author

Although we could assign each label to a single street (in fact, there is a street_edge_id column in the label table), I think that for this situation, it makes more sense to include labels on multiple streets. This primarily is about intersections, where curb ramps (or lack thereof) impacts all streets at the intersection, so it makes sense to count them for multiple streets.

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

Successfully merging a pull request may close this issue.

3 participants