Make locality thinning grids smaller#2040
Conversation
nvkelso
left a comment
There was a problem hiding this comment.
If this is a 512 px tile size versus 256 issue, then should we try doubling the grid_width instead? Your example from 3 > 10 looks far too dense to me.
|
Sounds good. I was trying to roughly match the styled map from before thinning (where the client was doing all the thinning), and a plain doubling seemed to be missing some of the smaller localities. But here's a new round of screenshots with plain doubling:
|
nvkelso
left a comment
There was a problem hiding this comment.
Looking good, let's see it in a build :)
|
Build says the numbers still aren't right. If Tilequeue is operating on a larger window (like 4x4 grid of requested tiles), then we should we instead apply multiplier to vector-datasource for an allowlisted list of config props values (like also for |
…eady multiplied by 2
|
Sounds like we're happy with this most recent change. So last couple things here before we merge:
|
|
We also need to fix the config and logic so it works for both Tilequeue and Tileserver, please. |
I haven't found a path to do that yet. I'll keep thinking. |
|
After talking with Travis for a bit this afternoon we weren't able to find an approach that handles both tileserver and tilequeue. I'm going to spend the rest of the evening trying to get The sticking point is that metatile-based builds run the functions in queries.yaml (like grid thinning) once for all the data in the metatile and the functions only get the |
| items_matching: { kind: locality } | ||
| max_items: 1 | ||
| grid_width: 3 | ||
| grid_width: 52181.0113 |
There was a problem hiding this comment.
It's not obvious this is in meters... should the param name be grid_width_meters?
Is the fractional meters necessary?
There was a problem hiding this comment.
I suppose they're not necessary, but using fractions means we get closer to exactly 3x3 subdivisions in a single 256px tile at this zoom level. If we go to integer values here, then the tiny subdivisions at the edge of the tile are slightly bigger and are more likely to get an extra feature in there.
| # so we can bucketize the padding area, too. | ||
| bucket_x = int((shape.x - minx) / bucket_width) | ||
| bucket_y = int((shape.y - miny) / bucket_height) | ||
| bucket_x = int((shape.x - minx) / grid_width) |
There was a problem hiding this comment.
Is this already flexible to whole number grid_width meters? It's rounding it to an int, so maybe? If it's already padded then seems like it's fine.
There was a problem hiding this comment.
It's rounding to an int after doing floating point math.
nvkelso
left a comment
There was a problem hiding this comment.
Two nits but otherwise LGTM and works in a QA build.
Fix keep_n_gridded tests after changes in #2040
Extending #2001 with a few tweaks to the queries.yaml arguments for the locality thinning.
The
grid_width: 3was way too small, resulting in way too few localities showing up:Changing to
grid_width: 10results in a more reasonable number of localities: