Skip to content

Commit 869b2d1

Browse files
authored
Add extension constant pushdown rule and fix InnerProduct rule (#7507)
## Summary Most of our optimizer rules interact with `ConstantArray`, as often there is an optimized implementation when a child is known to be constant. We can have both of these arrays that mean the same thing, but one is not easily matched on: ``` ConstantArray( scalar: Scalar::Extension(...) ) ExtensionArray( storage: ConstantArray( Scalar::* ) ) ``` This change adds a rule to convert the second representation into the first. It additionally fixes a bug in the `InnerProduct` reduction (right now it is in `execute` but it should really be a reduction rule, that will come in the future) where it checks for the second case instead of the first. ## Testing Basic tests. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent ce52b71 commit 869b2d1

2 files changed

Lines changed: 118 additions & 25 deletions

File tree

vortex-array/src/arrays/extension/compute/rules.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,60 @@ use vortex_error::VortexResult;
66
use crate::ArrayRef;
77
use crate::IntoArray;
88
use crate::array::ArrayView;
9+
use crate::arrays::Constant;
10+
use crate::arrays::ConstantArray;
911
use crate::arrays::Extension;
1012
use crate::arrays::ExtensionArray;
1113
use crate::arrays::Filter;
1214
use crate::arrays::extension::ExtensionArrayExt;
1315
use crate::arrays::filter::FilterReduceAdaptor;
1416
use crate::arrays::slice::SliceReduceAdaptor;
17+
use crate::matcher::AnyArray;
1518
use crate::optimizer::rules::ArrayParentReduceRule;
1619
use crate::optimizer::rules::ParentRuleSet;
20+
use crate::scalar::Scalar;
1721
use crate::scalar_fn::fns::cast::CastReduceAdaptor;
1822
use crate::scalar_fn::fns::mask::MaskReduceAdaptor;
1923

2024
pub(crate) const PARENT_RULES: ParentRuleSet<Extension> = ParentRuleSet::new(&[
25+
ParentRuleSet::lift(&ExtensionConstantParentRule),
2126
ParentRuleSet::lift(&ExtensionFilterPushDownRule),
2227
ParentRuleSet::lift(&CastReduceAdaptor(Extension)),
2328
ParentRuleSet::lift(&FilterReduceAdaptor(Extension)),
2429
ParentRuleSet::lift(&MaskReduceAdaptor(Extension)),
2530
ParentRuleSet::lift(&SliceReduceAdaptor(Extension)),
2631
]);
2732

33+
/// Normalize `Extension(Constant(storage))` children to `Constant(Extension(storage))`.
34+
#[derive(Debug)]
35+
struct ExtensionConstantParentRule;
36+
37+
impl ArrayParentReduceRule<Extension> for ExtensionConstantParentRule {
38+
type Parent = AnyArray;
39+
40+
fn reduce_parent(
41+
&self,
42+
child: ArrayView<'_, Extension>,
43+
parent: &ArrayRef,
44+
child_idx: usize,
45+
) -> VortexResult<Option<ArrayRef>> {
46+
let Some(const_array) = child.storage_array().as_opt::<Constant>() else {
47+
return Ok(None);
48+
};
49+
50+
let storage_scalar = const_array.scalar().clone();
51+
let ext_scalar = Scalar::extension_ref(child.ext_dtype().clone(), storage_scalar);
52+
53+
let constant_with_extension_scalar =
54+
ConstantArray::new(ext_scalar, child.len()).into_array();
55+
56+
parent
57+
.clone()
58+
.with_slot(child_idx, constant_with_extension_scalar)
59+
.map(Some)
60+
}
61+
}
62+
2863
/// Push filter operations into the storage array of an extension array.
2964
#[derive(Debug)]
3065
struct ExtensionFilterPushDownRule;
@@ -58,6 +93,7 @@ mod tests {
5893
use crate::IntoArray;
5994
#[expect(deprecated)]
6095
use crate::ToCanonical as _;
96+
use crate::arrays::Constant;
6197
use crate::arrays::ConstantArray;
6298
use crate::arrays::Extension;
6399
use crate::arrays::ExtensionArray;
@@ -177,6 +213,31 @@ mod tests {
177213
assert_eq!(canonical.len(), 3);
178214
}
179215

216+
#[test]
217+
fn test_extension_constant_child_normalizes_under_scalar_fn() {
218+
let ext_dtype = test_ext_dtype();
219+
220+
let constant_storage = ConstantArray::new(Scalar::from(10i64), 3).into_array();
221+
let constant_ext = ExtensionArray::new(ext_dtype.clone(), constant_storage).into_array();
222+
223+
let storage = buffer![15i64, 25, 35].into_array();
224+
let ext_array = ExtensionArray::new(ext_dtype, storage).into_array();
225+
226+
let scalar_fn_array = Binary
227+
.try_new_array(3, Operator::Lt, [constant_ext, ext_array])
228+
.unwrap();
229+
230+
let optimized = scalar_fn_array.optimize().unwrap();
231+
let scalar_fn = optimized.as_opt::<crate::arrays::ScalarFnVTable>().unwrap();
232+
let children = scalar_fn.children();
233+
let constant = children[0]
234+
.as_opt::<Constant>()
235+
.expect("constant extension child should be normalized");
236+
237+
assert!(constant.scalar().as_extension_opt().is_some());
238+
assert_eq!(constant.len(), 3);
239+
}
240+
180241
#[test]
181242
fn test_scalar_fn_no_pushdown_different_ext_types() {
182243
#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]

vortex-tensor/src/scalar_fns/inner_product.rs

Lines changed: 57 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -448,26 +448,18 @@ impl InnerProduct {
448448
return Ok(None);
449449
}
450450

451-
// The other side must be a constant-backed tensor-like extension whose scalar is
452-
// non-null.
453-
let Some(const_ext) = const_ref.as_opt::<Extension>() else {
451+
// The other side must be a constant tensor.
452+
let Some(const_storage) = constant_tensor_storage(const_ref) else {
454453
return Ok(None);
455454
};
456-
let const_storage = const_ext.storage_array();
457-
let Some(const_backing) = const_storage.as_opt::<Constant>() else {
458-
return Ok(None);
459-
};
460-
if const_backing.scalar().is_null() {
461-
return Ok(None);
462-
}
463455

464456
let dim = sorf_view.options.dimension as usize;
465457
let num_rounds = sorf_view.options.num_rounds as usize;
466458
let seed = sorf_view.options.seed;
467459
let padded_dim = dim.next_power_of_two();
468460

469461
// Extract the single stored row of the constant via the stride-0 short-circuit.
470-
let flat = extract_flat_elements(const_storage, dim, ctx)?;
462+
let flat = extract_flat_elements(&const_storage, dim, ctx)?;
471463
if flat.ptype() != PType::F32 {
472464
// TODO(connor): as above, f16/f64 are not supported by this rewrite yet. The
473465
// standard path handles them correctly.
@@ -482,9 +474,9 @@ impl InnerProduct {
482474
let mut rotated_query = vec![0.0f32; padded_dim];
483475
rotation.rotate(&padded_query, &mut rotated_query);
484476

485-
// Build the rewritten constant as a `Vector<padded_dim, f32>` extension wrapping a
486-
// `ConstantArray` of length `len`. We reuse the original storage FSL nullability so
487-
// the new extension dtype stays consistent with whatever the original tree expected.
477+
// Build the rewritten constant as a `Vector<padded_dim, f32>` extension scalar. We reuse
478+
// the original storage FSL nullability so the new extension dtype stays consistent with
479+
// whatever the original tree expected.
488480
let storage_fsl_nullability = const_storage.dtype().nullability();
489481
let element_dtype = DType::Primitive(PType::F32, Nullability::NonNullable);
490482
let children: Vec<Scalar> = rotated_query
@@ -493,7 +485,6 @@ impl InnerProduct {
493485
.collect();
494486
let fsl_scalar =
495487
Scalar::fixed_size_list(element_dtype.clone(), children, storage_fsl_nullability);
496-
let new_storage = ConstantArray::new(fsl_scalar, len).into_array();
497488

498489
// Build a fresh `Vector<padded_dim, f32>` extension dtype. We cannot reuse the
499490
// original extension dtype because that one has `dim`, not `padded_dim`.
@@ -504,7 +495,8 @@ impl InnerProduct {
504495
storage_fsl_nullability,
505496
);
506497
let new_ext_dtype = ExtDType::<Vector>::try_new(EmptyMetadata, new_fsl_dtype)?.erased();
507-
let new_constant = ExtensionArray::new(new_ext_dtype, new_storage).into_array();
498+
let new_constant =
499+
ConstantArray::new(Scalar::extension_ref(new_ext_dtype, fsl_scalar), len).into_array();
508500

509501
// Extract the SorfTransform child (the already-padded Vector<padded_dim, f32>).
510502
let sorf_child = sorf_view
@@ -572,16 +564,9 @@ impl InnerProduct {
572564
};
573565

574566
// Navigate the constant side and require its scalar be non-null.
575-
let Some(const_ext) = const_candidate.as_opt::<Extension>() else {
567+
let Some(const_storage) = constant_tensor_storage(const_candidate) else {
576568
return Ok(None);
577569
};
578-
let const_storage = const_ext.storage_array();
579-
let Some(const_backing) = const_storage.as_opt::<Constant>() else {
580-
return Ok(None);
581-
};
582-
if const_backing.scalar().is_null() {
583-
return Ok(None);
584-
}
585570

586571
// Canonicalize codes and values. Codes may be e.g. BitPacked; executing is cheaper
587572
// than falling through to the standard path (which would also canonicalize).
@@ -602,7 +587,7 @@ impl InnerProduct {
602587

603588
let padded_dim = usize::try_from(fsl.list_size()).vortex_expect("fsl list_size fits usize");
604589

605-
let flat = extract_flat_elements(const_storage, padded_dim, ctx)?;
590+
let flat = extract_flat_elements(&const_storage, padded_dim, ctx)?;
606591
if flat.ptype() != PType::F32 {
607592
// TODO(connor): case 2 is f32-only. For f16/f64 we fall through to the standard
608593
// path, which computes the inner product with the correct element type.
@@ -637,6 +622,16 @@ impl InnerProduct {
637622
}
638623
}
639624

625+
/// Return the storage constant for a canonical tensor-like constant query.
626+
fn constant_tensor_storage(array: &ArrayRef) -> Option<ArrayRef> {
627+
let constant = array.as_opt::<Constant>()?;
628+
if constant.scalar().is_null() {
629+
return None;
630+
}
631+
let ext_scalar = constant.scalar().as_extension_opt()?;
632+
Some(ConstantArray::new(ext_scalar.to_storage_scalar(), array.len()).into_array())
633+
}
634+
640635
/// Computes the inner product (dot product) of two equal-length float slices.
641636
///
642637
/// Returns `sum(a_i * b_i)`.
@@ -959,6 +954,7 @@ mod tests {
959954
use vortex_array::ArrayRef;
960955
use vortex_array::IntoArray;
961956
use vortex_array::VortexSessionExecute;
957+
use vortex_array::arrays::Constant;
962958
use vortex_array::arrays::ConstantArray;
963959
use vortex_array::arrays::ExtensionArray;
964960
use vortex_array::arrays::FixedSizeListArray;
@@ -978,9 +974,11 @@ mod tests {
978974
use vortex_session::VortexSession;
979975

980976
use crate::scalar_fns::inner_product::InnerProduct;
977+
use crate::scalar_fns::inner_product::constant_tensor_storage;
981978
use crate::scalar_fns::sorf_transform::SorfMatrix;
982979
use crate::scalar_fns::sorf_transform::SorfOptions;
983980
use crate::scalar_fns::sorf_transform::SorfTransform;
981+
use crate::utils::extract_flat_elements;
984982
use crate::vector::Vector;
985983

986984
static SESSION: LazyLock<VortexSession> =
@@ -1011,6 +1009,19 @@ mod tests {
10111009
Ok(ExtensionArray::new(ext_dtype, storage).into_array())
10121010
}
10131011

1012+
/// Expression-literal shape: a ConstantArray whose scalar itself is a Vector extension.
1013+
fn literal_vector_f32(elements: &[f32], len: usize) -> ArrayRef {
1014+
let element_dtype = DType::Primitive(PType::F32, Nullability::NonNullable);
1015+
let children: Vec<Scalar> = elements
1016+
.iter()
1017+
.map(|&v| Scalar::primitive(v, Nullability::NonNullable))
1018+
.collect();
1019+
let storage_scalar =
1020+
Scalar::fixed_size_list(element_dtype, children, Nullability::NonNullable);
1021+
let vector_scalar = Scalar::extension::<Vector>(EmptyMetadata, storage_scalar);
1022+
ConstantArray::new(vector_scalar, len).into_array()
1023+
}
1024+
10141025
/// Build an `ExtensionArray<Vector<list_size, f32>>` whose storage is
10151026
/// `FSL(DictArray(codes: u8, values: f32))`. This mirrors the shape that
10161027
/// TurboQuant produces as the SorfTransform child.
@@ -1115,6 +1126,27 @@ mod tests {
11151126

11161127
// ---- Case 1: SorfTransform + Constant pull-through ----
11171128

1129+
#[test]
1130+
fn constant_tensor_storage_accepts_extension_scalar_literal() -> VortexResult<()> {
1131+
let literal = literal_vector_f32(&[1.0, 2.0, 3.0], 5);
1132+
let storage =
1133+
constant_tensor_storage(&literal).expect("literal vector should be recognized");
1134+
1135+
assert_eq!(storage.len(), 5);
1136+
let const_storage = storage
1137+
.as_opt::<Constant>()
1138+
.expect("storage should remain constant-backed");
1139+
assert!(matches!(
1140+
const_storage.scalar().dtype(),
1141+
DType::FixedSizeList(_, 3, Nullability::NonNullable)
1142+
));
1143+
1144+
let mut ctx = SESSION.create_execution_ctx();
1145+
let flat = extract_flat_elements(&storage, 3, &mut ctx)?;
1146+
assert_eq!(flat.row::<f32>(0), &[1.0, 2.0, 3.0]);
1147+
Ok(())
1148+
}
1149+
11181150
/// Case 1: SorfTransform on LHS, constant query on RHS, with `dim < padded_dim`
11191151
/// so the zero-padding branch is exercised.
11201152
#[test]

0 commit comments

Comments
 (0)