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

Aggregates: allow aggregates pipeline the processing of full tiles. #4465

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

KiterLuc
Copy link
Contributor

This adds a new aggregate_full_tile interface to iaggregator so that we can later use the data in the fragment metadata to improve aggregation performance. The data is packaged in the new FullTileData class, which will later be produced by the fragment metadata.

Note that this only implements the aggregators side of the work... Reader/FragmentMetadata changes will follow.


TYPE: FEATURE
DESC: Aggregates: allow aggregates pipeline the processing of full tiles.

This adds a new aggregate_full_tile interface to iaggregator so that we can later use the data in the fragment metadata to improve aggregation performance. The data is packaged in the new FullTileData class, which will later be produced by the fragment metadata.

Note that this only implements the aggregators side of the work... Reader/FragmentMetadata changes will follow.

---
TYPE: FEATURE
DESC: Aggregates: allow aggregates pipeline the processing of full tiles.
@KiterLuc KiterLuc requested a review from robertbindar October 26, 2023 12:15
Copy link
Contributor

@robertbindar robertbindar left a comment

Choose a reason for hiding this comment

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

Thanks! The implementation seems sound, thanks for keeping this code tidy with the new update_data. Can we do anything with the "Full Tile" naming? I have a problem with it in the sense that it doesn't tell me at all that the data is coming from fragment metadata, so unless you're in the reader/fragmeta code and you see where the data originates from, you have no idea that aggregate_data and aggregate_full_tile operate on two completely different data sources that have very different operating costs.

@KiterLuc KiterLuc force-pushed the lr/aggregates-full-tiles branch from 38cc469 to d9ad5bc Compare October 27, 2023 10:13
@KiterLuc KiterLuc force-pushed the lr/aggregates-full-tiles branch from d9ad5bc to df91a6e Compare October 27, 2023 10:15
@KiterLuc KiterLuc merged commit 41c96a6 into dev Oct 27, 2023
46 checks passed
@KiterLuc KiterLuc deleted the lr/aggregates-full-tiles branch October 27, 2023 13:37
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.

2 participants