clean up

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

Author: connortsui20

vortex-array/public-api.lock

@@ -22144,6 +22144,14 @@ pub fn vortex_array::ExecutionCtx::new(session: vortex_session::VortexSession) -
 
 pub fn vortex_array::ExecutionCtx::session(&self) -> &vortex_session::VortexSession
 
+impl core::clone::Clone for vortex_array::ExecutionCtx
+
+pub fn vortex_array::ExecutionCtx::clone(&self) -> vortex_array::ExecutionCtx
+
+impl core::fmt::Debug for vortex_array::ExecutionCtx
+
+pub fn vortex_array::ExecutionCtx::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+
 impl core::fmt::Display for vortex_array::ExecutionCtx
 
 pub fn vortex_array::ExecutionCtx::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result

vortex-btrblocks/public-api.lock

@@ -22,26 +22,26 @@ use crate::schemes::temporal;
 /// the final scheme list, so that tie-breaking is deterministic.
 pub const ALL_SCHEMES: &[&dyn Scheme] = &[
     // Integer schemes.
-    &integer::UncompressedScheme as &dyn Scheme,
-    &integer::ConstantScheme,
-    // NOTE: For must precede BitPacking to avoid unnecessary patches.
+    &integer::IntUncompressedScheme as &dyn Scheme,
+    &integer::IntConstantScheme,
+    // NOTE: FoR must precede BitPacking to avoid unnecessary patches.
     &integer::FoRScheme,
     &integer::BitPackingScheme,
     &integer::ZigZagScheme,
     &integer::SparseScheme,
-    &integer::DictScheme,
+    &integer::IntDictScheme,
     &integer::RunEndScheme,
     &integer::SequenceScheme,
     &integer::RLE_INTEGER_SCHEME,
     #[cfg(feature = "pco")]
     &integer::PcoScheme,
     // Float schemes.
-    &float::UncompressedScheme,
-    &float::ConstantScheme,
+    &float::FloatUncompressedScheme,
+    &float::FloatConstantScheme,
     &float::ALPScheme,
     &float::ALPRDScheme,
-    &float::DictScheme,
-    &float::NullDominated,
+    &float::FloatDictScheme,
+    &float::NullDominatedSparseScheme,
     &float::RLE_FLOAT_SCHEME,
     #[cfg(feature = "pco")]
     &float::PcoScheme,
@@ -50,11 +50,11 @@ pub const ALL_SCHEMES: &[&dyn Scheme] = &[
     // Temporal schemes.
     &temporal::TemporalScheme,
     // String schemes.
-    &string::UncompressedScheme,
-    &string::DictScheme,
+    &string::StringUncompressedScheme,
+    &string::StringDictScheme,
     &string::FSSTScheme,
-    &string::ConstantScheme,
-    &string::NullDominated,
+    &string::StringConstantScheme,
+    &string::NullDominatedSparseScheme,
     #[cfg(feature = "zstd")]
     &string::ZstdScheme,
     #[cfg(all(feature = "zstd", feature = "unstable_encodings"))]
@@ -86,20 +86,20 @@ pub fn default_excluded() -> HashSet<SchemeId> {
 ///
 /// ```rust
 /// use vortex_btrblocks::{BtrBlocksCompressorBuilder, Scheme, SchemeExt};
-/// use vortex_btrblocks::schemes::integer::DictScheme;
+/// use vortex_btrblocks::schemes::integer::IntDictScheme;
 ///
 /// // Default compressor - all non-excluded schemes allowed.
 /// let compressor = BtrBlocksCompressorBuilder::default().build();
 ///
 /// // Exclude specific schemes.
 /// let compressor = BtrBlocksCompressorBuilder::default()
-///     .exclude([DictScheme.id()])
+///     .exclude([IntDictScheme.id()])
 ///     .build();
 ///
 /// // Exclude then re-include.
 /// let compressor = BtrBlocksCompressorBuilder::default()
-///     .exclude([DictScheme.id()])
-///     .include([DictScheme.id()])
+///     .exclude([IntDictScheme.id()])
+///     .include([IntDictScheme.id()])
 ///     .build();
 /// ```
 #[derive(Debug, Clone)]

vortex-btrblocks/src/builder.rs

@@ -20,14 +20,14 @@ use crate::CascadingCompressor;
 ///
 /// ```rust
 /// use vortex_btrblocks::{BtrBlocksCompressor, BtrBlocksCompressorBuilder, Scheme, SchemeExt};
-/// use vortex_btrblocks::schemes::integer::DictScheme;
+/// use vortex_btrblocks::schemes::integer::IntDictScheme;
 ///
 /// // Default compressor - all schemes allowed.
 /// let compressor = BtrBlocksCompressor::default();
 ///
 /// // Exclude specific schemes using the builder.
 /// let compressor = BtrBlocksCompressorBuilder::default()
-///     .exclude([DictScheme.id()])
+///     .exclude([IntDictScheme.id()])
 ///     .build();
 /// ```
 #[derive(Clone)]

vortex-btrblocks/src/canonical_compressor.rs

@@ -41,15 +41,15 @@
 //!
 //! ```rust
 //! use vortex_btrblocks::{BtrBlocksCompressor, BtrBlocksCompressorBuilder, Scheme, SchemeExt};
-//! use vortex_btrblocks::schemes::integer::DictScheme;
+//! use vortex_btrblocks::schemes::integer::IntDictScheme;
 //! use vortex_array::DynArray;
 //!
 //! // Default compressor with all schemes enabled.
 //! let compressor = BtrBlocksCompressor::default();
 //!
 //! // Configure with builder to exclude specific schemes.
 //! let compressor = BtrBlocksCompressorBuilder::default()
-//!     .exclude([DictScheme.id()])
+//!     .exclude([IntDictScheme.id()])
 //!     .build();
 //! ```
 //!

vortex-btrblocks/src/lib.rs

@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-// Re-export builtin schemes from vortex-compressor.
+//! Float compression schemes.
+
 use vortex_alp::ALP;
 use vortex_alp::ALPArray;
 use vortex_alp::RDEncoder;
@@ -12,13 +13,8 @@ use vortex_array::IntoArray;
 use vortex_array::ToCanonical;
 use vortex_array::arrays::PrimitiveArray;
 use vortex_array::dtype::PType;
-pub use vortex_compressor::builtins::FloatConstantScheme as ConstantScheme;
-pub use vortex_compressor::builtins::FloatDictScheme as DictScheme;
-pub use vortex_compressor::builtins::FloatUncompressedScheme as UncompressedScheme;
-pub use vortex_compressor::builtins::is_float_primitive;
 use vortex_compressor::scheme::ChildSelection;
 use vortex_compressor::scheme::DescendantExclusion;
-pub use vortex_compressor::stats::FloatStats;
 use vortex_error::VortexResult;
 use vortex_error::vortex_panic;
 use vortex_sparse::Sparse;
@@ -45,8 +41,10 @@ pub struct ALPScheme;
 pub struct ALPRDScheme;
 
 /// Sparse encoding for null-dominated float arrays.
+///
+/// This is the same as the integer `SparseScheme`, but we only use this for null-dominated arrays.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
-pub struct NullDominated;
+pub struct NullDominatedSparseScheme;
 
 /// Pco (pcodec) compression for floats.
 #[cfg(feature = "pco")]
@@ -57,6 +55,13 @@ pub struct PcoScheme;
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct FloatRLEConfig;
 
+// Re-export builtin schemes from vortex-compressor.
+pub use vortex_compressor::builtins::FloatConstantScheme;
+pub use vortex_compressor::builtins::FloatDictScheme;
+pub use vortex_compressor::builtins::FloatUncompressedScheme;
+pub use vortex_compressor::builtins::is_float_primitive;
+pub use vortex_compressor::stats::FloatStats;
+
 impl rle::RLEConfig for FloatRLEConfig {
     type Stats = FloatStats;
 
@@ -206,7 +211,7 @@ impl Scheme for ALPRDScheme {
     }
 }
 
-impl Scheme for NullDominated {
+impl Scheme for NullDominatedSparseScheme {
     fn scheme_name(&self) -> &'static str {
         "vortex.float.sparse"
     }
@@ -219,6 +224,8 @@ impl Scheme for NullDominated {
         1
     }
 
+    // TODO(connor): There seems to be stuff missing here...
+    /// The indices of a null-dominated sparse array should not be sparse-encoded again.
     fn descendant_exclusions(&self) -> Vec<DescendantExclusion> {
         vec![DescendantExclusion {
             excluded: IntSparseScheme.id(),

vortex-btrblocks/src/schemes/float.rs

@@ -1,22 +1,20 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-// Re-export builtin schemes from vortex-compressor.
+//! Integer compression schemes.
+
 use vortex_array::ArrayRef;
 use vortex_array::Canonical;
 use vortex_array::IntoArray;
 use vortex_array::ToCanonical;
 use vortex_array::arrays::ConstantArray;
 use vortex_array::arrays::PrimitiveArray;
 use vortex_array::scalar::Scalar;
-pub use vortex_compressor::builtins::IntConstantScheme as ConstantScheme;
-pub use vortex_compressor::builtins::IntDictScheme as DictScheme;
-pub use vortex_compressor::builtins::IntUncompressedScheme as UncompressedScheme;
-pub use vortex_compressor::builtins::is_integer_primitive;
+use vortex_compressor::builtins::FloatDictScheme;
+use vortex_compressor::builtins::StringDictScheme;
 use vortex_compressor::scheme::AncestorExclusion;
 use vortex_compressor::scheme::ChildSelection;
 use vortex_compressor::scheme::DescendantExclusion;
-pub use vortex_compressor::stats::IntegerStats;
 use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
 use vortex_error::vortex_bail;
@@ -74,6 +72,13 @@ pub struct SequenceScheme;
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct PcoScheme;
 
+// Re-export builtin schemes from vortex-compressor.
+pub use vortex_compressor::builtins::IntConstantScheme;
+pub use vortex_compressor::builtins::IntDictScheme;
+pub use vortex_compressor::builtins::IntUncompressedScheme;
+pub use vortex_compressor::builtins::is_integer_primitive;
+pub use vortex_compressor::stats::IntegerStats;
+
 /// Threshold for the average run length in an array before we consider run-end encoding.
 const RUN_END_THRESHOLD: u32 = 4;
 
@@ -224,10 +229,13 @@ impl Scheme for ZigZagScheme {
         1
     }
 
+    /// Container-style schemes (Dict, RunEnd, Sparse) restructure data and should only be
+    /// applied once in a cascade chain. ZigZag's output is a simple value transformation,
+    /// so further restructuring is wasteful.
     fn descendant_exclusions(&self) -> Vec<DescendantExclusion> {
         vec![
             DescendantExclusion {
-                excluded: DictScheme.id(),
+                excluded: IntDictScheme.id(),
                 children: ChildSelection::All,
             },
             DescendantExclusion {
@@ -361,9 +369,11 @@ impl Scheme for SparseScheme {
         2
     }
 
+    /// Sparse values and indices are already low-cardinality by construction, so dictionary
+    /// encoding doesn't add anything.
     fn descendant_exclusions(&self) -> Vec<DescendantExclusion> {
         vec![DescendantExclusion {
-            excluded: DictScheme.id(),
+            excluded: IntDictScheme.id(),
             children: ChildSelection::All,
         }]
     }
@@ -488,24 +498,21 @@ impl Scheme for RunEndScheme {
         2
     }
 
+    /// Run-end values and ends are already deduplicated, so dictionary encoding doesn't add
+    /// anything.
     fn descendant_exclusions(&self) -> Vec<DescendantExclusion> {
         vec![DescendantExclusion {
-            excluded: DictScheme.id(),
+            excluded: IntDictScheme.id(),
             children: ChildSelection::All,
         }]
     }
 
+    // TODO(connor): There seems to be stuff missing here...
     fn ancestor_exclusions(&self) -> Vec<AncestorExclusion> {
-        use vortex_compressor::builtins::FloatDictScheme;
-
-        vec![
-            // Exclude from FloatDict values child (child 0). This replaces the old ALP
-            // conditional propagation of float RLE exclusion to integer RunEnd.
-            AncestorExclusion {
-                ancestor: FloatDictScheme.id(),
-                children: ChildSelection::One(0),
-            },
-        ]
+        vec![AncestorExclusion {
+            ancestor: FloatDictScheme.id(),
+            children: ChildSelection::One(0),
+        }]
     }
 
     fn expected_compression_ratio(
@@ -564,22 +571,19 @@ impl Scheme for SequenceScheme {
         is_integer_primitive(canonical)
     }
 
+    /// Sequence encoding on dictionary codes just adds a layer of indirection without compressing
+    /// the data. Dict codes are compact integers that benefit from BitPacking or FoR, not from
+    /// sequence detection.
     fn ancestor_exclusions(&self) -> Vec<AncestorExclusion> {
-        use vortex_compressor::builtins::FloatDictScheme;
-        use vortex_compressor::builtins::StringDictScheme;
-
         vec![
-            // Exclude from IntDict codes.
             AncestorExclusion {
-                ancestor: DictScheme.id(),
+                ancestor: IntDictScheme.id(),
                 children: ChildSelection::All,
             },
-            // Exclude from FloatDict codes (child 1).
             AncestorExclusion {
                 ancestor: FloatDictScheme.id(),
                 children: ChildSelection::One(1),
             },
-            // Exclude from StringDict codes (child 1).
             AncestorExclusion {
                 ancestor: StringDictScheme.id(),
                 children: ChildSelection::One(1),

vortex-btrblocks/src/schemes/integer.rs

@@ -3,15 +3,12 @@
 
 //! Compression scheme implementations.
 
-/// Decimal compression schemes.
-pub mod decimal;
-/// Float compression schemes.
 pub mod float;
-/// Integer compression schemes.
 pub mod integer;
-pub(crate) mod patches;
-pub(crate) mod rle;
-/// String compression schemes.
 pub mod string;
-/// Temporal compression schemes.
+
+pub mod decimal;
 pub mod temporal;
+
+pub(crate) mod patches;
+pub(crate) mod rle;

vortex-btrblocks/src/schemes/mod.rs

@@ -20,7 +20,7 @@ use crate::CompressorContext;
 use crate::Scheme;
 use crate::SchemeExt;
 use crate::estimate_compression_ratio_with_sampling;
-use crate::schemes::integer::DictScheme as IntDictScheme;
+use crate::schemes::integer::IntDictScheme;
 
 /// Threshold for the average run length in an array before we consider run-length encoding.
 pub const RUN_LENGTH_THRESHOLD: u32 = 4;
@@ -35,6 +35,8 @@ pub trait RLEStats {
     fn source(&self) -> &PrimitiveArray;
 }
 
+// TODO(connor): This trait is super confusing, we should probably just remove it and hardcode the
+// only 2 implementations (integer and float).
 /// Configuration trait for RLE schemes.
 ///
 /// Implement this trait to define the behavior of an RLE scheme for a specific
@@ -92,6 +94,9 @@ impl<C: RLEConfig> Scheme for RLEScheme<C> {
         3
     }
 
+    /// RLE indices (child 1) and offsets (child 2) are monotonically increasing, so dictionary
+    /// encoding is pointless.
+    /// Values (child 0) are not excluded since they may benefit from dict encoding.
     fn descendant_exclusions(&self) -> Vec<DescendantExclusion> {
         vec![DescendantExclusion {
             excluded: IntDictScheme.id(),