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

Make buffer_cfg work for meta tile build #401

Merged
merged 18 commits into from Oct 15, 2021
Merged

Make buffer_cfg work for meta tile build #401

merged 18 commits into from Oct 15, 2021

Conversation

peitili
Copy link
Contributor

@peitili peitili commented Oct 12, 2021

The buffer_cfg exists but it doesn't work correctly, this change makes the config wired in the stack.

tilequeue/process.py Outdated Show resolved Hide resolved
line=bounds,
point=bounds,
)
query_bounds_pad_fn = layer_datum['query_bounds_pad_fn']
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is already a part of every layer_datum:

query_bounds_pad_fn=create_query_bounds_pad_fn(

tilequeue/process.py Outdated Show resolved Hide resolved
@@ -724,9 +724,11 @@ def _add_feature(source, feature):

return source_features.iteritems()

def __call__(self, zoom, unpadded_bounds):
def __call__(self, zoom, bounds):
Copy link
Contributor Author

@peitili peitili Oct 14, 2021

Choose a reason for hiding this comment

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

it is not always unpadded_bounds any more, that's why we renamed it

@peitili peitili changed the title Buffer Make buffer_cfg work for meta tile build Oct 14, 2021
@peitili peitili marked this pull request as ready for review October 15, 2021 19:52
Copy link
Member

@iandees iandees left a comment

Choose a reason for hiding this comment

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

Looks ok to me, but I'm not 100% clear on the primary change.

@@ -735,25 +737,27 @@ def __call__(self, zoom, unpadded_bounds):
# loaded?
assert zoom <= self.tile_pyramid.max_z
assert zoom >= self.tile_pyramid.z
assert bbox.within(self.tile_pyramid.bbox())
# assert bbox.within(self.tile_pyramid.bbox())
Copy link
Member

Choose a reason for hiding this comment

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

Can this be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can this be deleted?

for this one I think probably leave it there so that we know there might be some verification we can do somewhere else.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can probably do this the opposite way - self.tile_pyramid.bbox should be within bbox, right, but the opposite might not be true...?

@@ -735,25 +737,27 @@ def __call__(self, zoom, unpadded_bounds):
# loaded?
assert zoom <= self.tile_pyramid.max_z
assert zoom >= self.tile_pyramid.z
assert bbox.within(self.tile_pyramid.bbox())
# assert bbox.within(self.tile_pyramid.bbox())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# assert bbox.within(self.tile_pyramid.bbox())
assert self.tile_pyramid_bbox().within(bbox)

cut_coords_by_zoom = calculate_cut_coords_by_zoom(
self.coord, self.metatile_zoom, self.cfg_tile_sizes, self.max_zoom)
feature_layers_by_zoom = {}

for nominal_zoom, _ in cut_coords_by_zoom.items():
source_rows = self.fetch_fn(nominal_zoom, unpadded_bounds)
source_rows = self.fetch_fn(nominal_zoom, self.max_padded_bounds)
Copy link
Contributor

Choose a reason for hiding this comment

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

@iandees this is the part 1 of the change, basically fetch everything inside a bbox that is the largest of the numbers in the various per-layer and per-geometry buffer configs

line=bounds,
point=bounds,
)
query_bounds_pad_fn = layer_datum['query_bounds_pad_fn']
Copy link
Contributor

Choose a reason for hiding this comment

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

@iandees this is part 2 of the change. Rather than just using the unpadded_bounds passed in, use the query_bounds_pad_fn attached to the layer datum to create an appropriately padded bbox.

@@ -97,6 +97,31 @@ def calc_buffered_bounds(
return bounds


def calc_max_padded_bounds(bounds, meters_per_pixel_dim, buffer_cfg):
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a stupid function that just finds the biggest number in the buffer config and makes a bounding box expanded by that much

Copy link
Contributor

@tgrigsby-sc tgrigsby-sc left a comment

Choose a reason for hiding this comment

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

Thanks so much for your hard work on this!

@peitili peitili merged commit 911e618 into master Oct 15, 2021
@peitili peitili deleted the buffer branch October 15, 2021 22:39
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.

None yet

4 participants