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

Prioritize by density #188

Merged
merged 2 commits into from
Mar 6, 2025
Merged

Conversation

michaelkirk
Copy link
Contributor

FIXES #154

This is based on #187, so review that first.

We still need to adjust the buckets (#143) - it looks pretty washed out, even in Glasgow, but over all, I feel like this fits into our existing patterns pretty well!

Screenshot 2025-03-05 at 19 10 13

You'll no doubt noice that I'm using our existing population zone data - the ones we're using for SIMD. I understand there may be finer grained population geometries we could use.

I think it may be worth investigating using these smaller geometries, but consider:

  1. It's additional geometry data we'd have to send the user, and store in their RAM
  2. It's more computation — we'll have to Prepare and Intersect an additional set of population Geometries rather than piggy-backing on the Intersection calculation we're already doing for SIMD.
  3. Ultimately, it might not materially affect the output all that much, since the existing population zone geometries are already reasonably to scale with our LTN boundaries.

My suggestion is to make a follow-up issue, and prioritize it with respect to everything else we want to get done, but what do you think?

I think we might end up removing this prioritization metric anyway, but
as I understand it, we generally want to prioritize "larger" ltns, so we
might as well align with "darker" -> "better candidate"
Copy link
Collaborator

@dabreegster dabreegster left a comment

Choose a reason for hiding this comment

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

Starting with the data zone level data is the right call. If we get feedback that output area is necessary and we have time, we can try it and see what the perf impact is, but this looks totally fine to me.

@dabreegster dabreegster merged commit d0d2373 into a-b-street:main Mar 6, 2025
1 check 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.

prioritization metric: population density
2 participants