Jefffrey commented on code in PR #11244:
URL: https://github.com/apache/arrow-rs/pull/11244#discussion_r4176845984


##########
arrow/benches/cast_kernels.rs:
##########
@@ -257,6 +258,31 @@ fn cast_array(array: &ArrayRef, to_type: DataType) {
     hint::black_box(cast(hint::black_box(array), 
hint::black_box(&to_type)).unwrap());
 }
 
+fn build_half_embedding(
+    rows: usize,
+    dims: usize,
+    null_every: Option<usize>,
+) -> (ArrayRef, ArrayRef) {
+    let len = rows * dims;
+    let f32_values = (0..len)
+        .map(|index| ((index.wrapping_mul(2_654_435_761) % 20_001) as f32 - 
10_000.0) / 127.0)

Review Comment:
   why do we have this custom logic for generating numbers, rather than relying 
on rand? especially where it specifies `null_every` instead of using a float 
for null density



##########
arrow/benches/cast_kernels.rs:
##########
@@ -257,6 +258,31 @@ fn cast_array(array: &ArrayRef, to_type: DataType) {
     hint::black_box(cast(hint::black_box(array), 
hint::black_box(&to_type)).unwrap());
 }
 
+fn build_half_embedding(

Review Comment:
   id suggest dropping "embedding" as it doesnt really have meaning in this 
context; all we care about is float16 regardless of what it represents



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to