Jefffrey commented on code in PR #10566:
URL: https://github.com/apache/arrow-rs/pull/10566#discussion_r3725199490
##########
arrow/src/util/bench_util.rs:
##########
@@ -34,6 +34,13 @@ use rand::{
use std::ops::Range;
use std::sync::Arc;
+fn f16_random(rng: &mut StdRng) -> f16 {
+ // Otherwise it can round up to 1.0 even though we should generate in [0,
1)
+ // See: https://github.com/VoidStarKat/half-rs/issues/152
+ let one_next_down = f16::from_bits(0x3BFFu16); // 0.9995117
+ f16::from_f32(rng.random::<f32>()).clamp(f16::ZERO, one_next_down)
Review Comment:
this is approximately how it was done in the original:
https://github.com/VoidStarKat/half-rs/blob/688f1e2c2a541186802b6b0fe0e6761cdb507428/src/rand_distr.rs#L6-L14
i just added some minor logic to clamp it
##########
arrow/src/util/bench_util.rs:
##########
@@ -53,6 +60,22 @@ where
.collect()
}
+/// Same as [`create_primitive_array`] but specialized for f16 since it doesn't
+/// implement the required rand traits.
+pub fn create_nullable_f16_array(size: usize, null_density: f32) ->
Float16Array {
Review Comment:
its not ideal to need this separate function, but because of the orphan rule
we cant do
```rust
impl Distribution<f16> for StandardUniform {}
```
we can't implement it on a newtype of `f16` either since
`create_primitive_array()` needs
```rust
T: ArrowPrimitiveType,
StandardUniform: Distribution<T::Native>,
```
the only other way i think is to depend on our own random trait, something
like this that codex suggested:
```rust
#[doc(hidden)]
pub trait RandomNative: Sized {
fn random<R: rand::Rng + ?Sized>(rng: &mut R) -> Self;
}
macro_rules! impl_random_native {
($($t:ty),* $(,)?) => {
$(
impl RandomNative for $t {
fn random<R: rand::Rng + ?Sized>(rng: &mut R) -> Self {
rng.random()
}
}
)*
};
}
impl_random_native!(
i8, i16, i32, i64, i128,
u8, u16, u32, u64, u128,
f32, f64,
);
impl RandomNative for f16 {
fn random<R: rand::Rng + ?Sized>(rng: &mut R) -> Self {
f16::from_f32(rng.random::<f32>())
}
}
pub fn create_primitive_array<T>(
size: usize,
null_density: f32,
) -> PrimitiveArray<T>
where
T: ArrowPrimitiveType,
T::Native: RandomNative,
{
let mut rng = seedable_rng();
(0..size)
.map(|_| {
if rng.random::<f32>() < null_density {
None
} else {
Some(T::Native::random(&mut rng))
}
})
.collect()
}
```
not sure if this is preferable 🤔
--
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]
