fix: Cache and Stacker should keep band order for subsets (#1120)

* fix(cache):filter returned band by query.band

* stacker: produce tiles with band selection stored

* update cache stored bands

* lint test

* add debug outputs on error tiles (only in debug mode)

* remove no needed method

* enhance expect method

* remove comment regarding TimeInstants

* remove band update in stored query since bands must not change

* remove redundant filterin in cache

* remove and cleanup code fix MockRasterSource

* back to old StackerAdapter stratey where bands are not selected

* baseline stacker approach. Fix tests

* improve comment

* lints

* add band selection logic to SimpleRasterStacker

* add regular time stackerwith band selection to RasterStacker Operator

* more tests and checks

* lints

* more lint

Author: jdroenner

datatypes/src/primitives/query_rectangle.rs

@@ -176,6 +176,14 @@ impl BandSelection {
     pub fn is_single(&self) -> bool {
         self.count() == 1
     }
+
+    pub fn contains(&self, band: u32) -> bool {
+        self.0.contains(&band)
+    }
+
+    pub fn contains_all(&self, bands: &[u32]) -> bool {
+        bands.iter().all(|band| self.contains(*band))
+    }
 }
 
 impl From<u32> for BandSelection {
@@ -184,6 +192,12 @@ impl From<u32> for BandSelection {
     }
 }
 
+impl AsRef<[u32]> for BandSelection {
+    fn as_ref(&self) -> &[u32] {
+        self.as_slice()
+    }
+}
+
 impl TryFrom<Vec<u32>> for BandSelection {
     type Error = crate::error::Error;
 
@@ -200,11 +214,19 @@ impl<const N: usize> TryFrom<[u32; N]> for BandSelection {
     }
 }
 
+impl TryFrom<&[u32]> for BandSelection {
+    type Error = crate::error::Error;
+
+    fn try_from(value: &[u32]) -> Result<Self, Self::Error> {
+        Self::new(value.to_vec())
+    }
+}
+
 impl QueryAttributeSelection for BandSelection {}
 
 #[derive(Clone, Debug, PartialEq, Serialize)]
 pub struct BandSelectionIter {
-    band_selection: BandSelection,
+    pub band_selection: BandSelection,
     next_index: usize,
 }
 

datatypes/src/primitives/time_gap_fill_iter.rs

@@ -1364,7 +1364,7 @@ mod tests {
     }
 
     #[test]
-    fn time_irregular_all_caes() {
+    fn time_irregular_all_cases() {
         let intervals = vec![
             TimeInterval::new_unchecked(
                 TimeInstance::from_millis(15).unwrap(),
@@ -1471,4 +1471,50 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    fn irregular_fill_min_max_time() {
+        let intervals = vec![
+            TimeInterval::new_unchecked(
+                TimeInstance::from_millis(15).unwrap(),
+                TimeInstance::from_millis(25).unwrap(),
+            ),
+            TimeInterval::new_unchecked(
+                TimeInstance::from_millis(45).unwrap(),
+                TimeInstance::from_millis(55).unwrap(),
+            ),
+        ];
+
+        let iter = intervals
+            .into_iter()
+            .map(|t| -> Result<TimeInterval, &str> { Ok(t) })
+            .try_time_irregular_range_fill(TimeInterval::default());
+        let result: Result<Vec<TimeInterval>, _> = iter.collect::<Result<Vec<_>, _>>();
+
+        assert_eq!(
+            result.unwrap(),
+            vec![
+                TimeInterval::new_unchecked(
+                    TimeInstance::MIN,
+                    TimeInstance::from_millis(15).unwrap()
+                ),
+                TimeInterval::new_unchecked(
+                    TimeInstance::from_millis(15).unwrap(),
+                    TimeInstance::from_millis(25).unwrap()
+                ),
+                TimeInterval::new_unchecked(
+                    TimeInstance::from_millis(25).unwrap(),
+                    TimeInstance::from_millis(45).unwrap()
+                ),
+                TimeInterval::new_unchecked(
+                    TimeInstance::from_millis(45).unwrap(),
+                    TimeInstance::from_millis(55).unwrap()
+                ),
+                TimeInterval::new_unchecked(
+                    TimeInstance::from_millis(55).unwrap(),
+                    TimeInstance::MAX
+                ),
+            ]
+        );
+    }
 }

datatypes/src/raster/grid_spatial.rs

@@ -284,7 +284,7 @@ impl GeoTransformAccess for SpatialGridDefinition {
     }
 }
 
-#[derive(Clone, Copy, Debug, Serialize, Deserialize)]
+#[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq)]
 pub struct TilingSpatialGridDefinition {
     // Don't make this public to avoid leaking inner
     element_grid_definition: SpatialGridDefinition,

datatypes/src/raster/mod.rs

@@ -54,6 +54,7 @@ pub use raster_properties::{
     RasterProperties, RasterPropertiesEntry, RasterPropertiesEntryType, RasterPropertiesKey,
 };
 pub use raster_traits::{CoordinatePixelAccess, GeoTransformAccess, Raster};
+pub use util::{TileIdxBandCrossProductIter, TileInformationBandCrossProductIter};
 
 mod arrow_conversion;
 mod band_names;
@@ -79,3 +80,4 @@ mod raster_traits;
 mod tiling;
 mod typed_raster_conversion;
 mod typed_raster_tile;
+mod util;

datatypes/src/raster/tiling.rs

@@ -10,7 +10,7 @@ use crate::{
 use serde::{Deserialize, Serialize};
 
 /// The static parameters required to create a `TilingStrategy`
-#[derive(Debug, Serialize, Deserialize, Clone, Copy)]
+#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq)]
 pub struct TilingSpecification {
     pub tile_size_in_pixels: GridShape2D,
 }

datatypes/src/raster/util.rs

@@ -0,0 +1,121 @@
+use crate::{
+    primitives::{BandSelection, BandSelectionIter},
+    raster::{GridBoundingBox2D, GridIdx2D, GridIdx2DIter, TileInformation, TileInformationIter},
+};
+
+#[derive(Clone, Debug)]
+pub struct TileIdxBandCrossProductIter {
+    tile_iter: GridIdx2DIter,
+    band_iter: BandSelectionIter, // TODO: maybe change this to actual attributes from ResultDescriptor not the Selection?
+    current_tile: Option<GridIdx2D>,
+}
+
+impl TileIdxBandCrossProductIter {
+    pub fn new(tile_iter: GridIdx2DIter, band_iter: BandSelectionIter) -> Self {
+        let mut tile_iter = tile_iter;
+        let current_tile = tile_iter.next();
+        Self {
+            tile_iter,
+            band_iter,
+            current_tile,
+        }
+    }
+
+    pub fn grid_bounds(&self) -> GridBoundingBox2D {
+        self.tile_iter.grid_bounds
+    }
+
+    pub fn band_selection(&self) -> &BandSelection {
+        &self.band_iter.band_selection
+    }
+
+    pub fn with_grid_bounds_and_selection(
+        bounds: GridBoundingBox2D,
+        band_selection: BandSelection,
+    ) -> Self {
+        let tile_iter = GridIdx2DIter::new(&bounds);
+        let band_iter = BandSelectionIter::new(band_selection);
+        Self::new(tile_iter, band_iter)
+    }
+
+    pub fn reset(&mut self) {
+        self.band_iter.reset();
+        self.tile_iter.reset();
+        self.current_tile = self.tile_iter.next();
+    }
+}
+
+impl Iterator for TileIdxBandCrossProductIter {
+    type Item = (GridIdx2D, u32);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let current_t = self.current_tile;
+
+        match (current_t, self.band_iter.next()) {
+            (None, _) => None,
+            (Some(t), Some(b)) => Some((t, b)),
+            (Some(_t), None) => {
+                self.band_iter.reset();
+                self.current_tile = self.tile_iter.next();
+                self.current_tile.map(|t| {
+                    (
+                        t,
+                        self.band_iter
+                            .next()
+                            .expect("There must be at least one band"),
+                    )
+                })
+            }
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub struct TileInformationBandCrossProductIter {
+    tile_iter: TileInformationIter,
+    band_iter: BandSelectionIter,
+    current_tile: Option<TileInformation>,
+}
+
+impl TileInformationBandCrossProductIter {
+    pub fn new(tile_iter: TileInformationIter, band_iter: BandSelectionIter) -> Self {
+        let mut tile_iter = tile_iter;
+        let current_tile = tile_iter.next();
+        Self {
+            tile_iter,
+            band_iter,
+            current_tile,
+        }
+    }
+
+    pub fn reset(&mut self) {
+        self.band_iter.reset();
+        self.tile_iter.reset();
+        self.current_tile = self.tile_iter.next();
+    }
+}
+
+impl Iterator for TileInformationBandCrossProductIter {
+    type Item = (TileInformation, u32);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let current_t = self.current_tile;
+
+        match (current_t, self.band_iter.next()) {
+            (None, _) => None,
+            (Some(t), Some(b)) => Some((t, b)),
+            (Some(_t), None) => {
+                self.band_iter.reset();
+                self.current_tile = self.tile_iter.next();
+                self.current_tile.map(|t| {
+                    (
+                        t,
+                        self.band_iter
+                            .next()
+                            .expect("There must be at least one band"),
+                    )
+                })
+            }
+        }
+    }
+}

datatypes/src/util/test.rs

@@ -1,8 +1,11 @@
 use float_cmp::approx_eq;
 
-use crate::raster::{
-    EmptyGrid, GeoTransform, Grid, GridIndexAccess, GridOrEmpty, GridSize, MaskedGrid, Pixel,
-    RasterTile2D, grid_idx_iter_2d,
+use crate::{
+    primitives::TimeInterval,
+    raster::{
+        EmptyGrid, GeoTransform, Grid, GridIdx2D, GridIndexAccess, GridOrEmpty, GridSize,
+        MaskedGrid, Pixel, RasterTile2D, grid_idx_iter_2d,
+    },
 };
 use std::panic;
 
@@ -103,6 +106,10 @@ pub fn assert_eq_two_list_of_tiles<P: Pixel>(
     list_b: &[RasterTile2D<P>],
     compare_cache_hint: bool,
 ) {
+    fn tile_pos<T>(tile: &RasterTile2D<T>) -> (TimeInterval, GridIdx2D, u32) {
+        (tile.time, tile.tile_position, tile.band)
+    }
+
     assert_eq!(
         list_a.len(),
         list_b.len(),
@@ -156,10 +163,12 @@ pub fn assert_eq_two_list_of_tiles<P: Pixel>(
             assert_eq!(
                 a.grid_array.is_empty(),
                 b.grid_array.is_empty(),
-                "grid shape of tile {} input_a is_empty: {:?}, input_b is_empty: {:?}",
+                "grid shape of tile {} input_a is_empty: {:?}, input_b is_empty: {:?}, a info: {:?}, b info: {:?}",
                 i,
                 a.grid_array.is_empty(),
                 b.grid_array.is_empty(),
+                tile_pos(a),
+                tile_pos(b)
             );
             if !a.grid_array.is_empty() {
                 let mat_a = a.grid_array.clone().into_materialized_masked_grid();

operators/src/adapters/mod.rs

@@ -2,27 +2,22 @@ mod band_extractor;
 mod feature_collection_merger;
 mod raster_stacker;
 mod raster_subquery;
-mod raster_time;
 mod raster_time_substream;
 mod simple_raster_stacker;
-mod sparse_tiles_fill_adapter;
 mod stream_statistics_adapter;
 mod time_stream_merge;
 
 use band_extractor::BandExtractor;
 pub use feature_collection_merger::FeatureCollectionChunkMerger;
-pub use raster_stacker::{RasterStackerAdapter, RasterStackerSource};
+pub use raster_stacker::{
+    PartialQueryRect, QueryWrapper, RasterStackerAdapter, RasterStackerSource,
+};
 pub use raster_subquery::{
     FoldTileAccu, FoldTileAccuMut, RasterSubQueryAdapter, SubQueryTileAggregator,
     TileReprojectionSubQuery, TileReprojectionSubqueryGridInfo, fold_by_coordinate_lookup_future,
 };
-pub use raster_time::{QueryWrapper, Queryable, RasterArrayTimeAdapter, RasterTimeAdapter};
 pub use simple_raster_stacker::{
-    SimpleRasterStackerAdapter, SimpleRasterStackerSource, stack_individual_aligned_raster_bands,
-};
-pub(crate) use sparse_tiles_fill_adapter::{
-    FillerTileCacheExpirationStrategy, FillerTimeBounds, SparseTilesFillAdapter,
-    SparseTilesFillAdapterError,
+    SimpleRasterStackerAdapter, SimpleRasterStackerError, SimpleRasterStackerSource,
 };
 pub use stream_statistics_adapter::StreamStatisticsAdapter;
 pub use time_stream_merge::TimeIntervalStreamMerge;