rust-random / rand

A Rust library for random number generation.
https://crates.io/crates/rand
Other
1.67k stars 431 forks source link

Standard: Distribution<__m128i> features requirements are undocumented #1410

Closed dragomang87 closed 1 month ago

dragomang87 commented 8 months ago

Common issues

Problem: rand::distributions::Standard for __m128i

Quick solution: do not have one

Details: Using version 0.8.5 and the documentation reports that there is impl Distribution<__m128i> for Standard . However the following code

use rand::Rng;
use rand::distributions::Distribution;

use std::arch::x86_64::*;

pub fn generate_m128i(n: usize) -> Vec<__m128i> {
    // Set up generator
    let mut rng = rand::thread_rng();
    // Set up random variable
    let random_variable: rand::distributions::Standard;
    // Set up vector
    let mut vector: Vec<__m128i>  = Vec::new();
    // Generate vector
    for _ in 0..n {
        //vector.push(rng.gen::<__m128i>());
        //vector.push(rng.sample(Standard));
        vector.push(random_variable.sample(&mut rng));
    }
    return vector;
}

gives the error

error[E0277]: the trait bound `Standard: Distribution<std::arch::x86_64::__m128i>` is not satisfied
  --> src/random_generators.rs:18:21
   |
18 |         vector.push(random_variable.sample(&mut rng));
   |                     ^^^^^^^^^^^^^^^ ------ required by a bound introduced by this call
   |                     |
   |                     the trait `Distribution<std::arch::x86_64::__m128i>` is not implemented for `Standard`
   |
   = help: the following other types implement trait `Distribution<T>`:
             <Standard as Distribution<()>>
             <Standard as Distribution<(A, B)>>
             <Standard as Distribution<(A, B, C)>>
             <Standard as Distribution<(A, B, C, D)>>
             <Standard as Distribution<(A, B, C, D, E)>>
             <Standard as Distribution<(A, B, C, D, E, F)>>
             <Standard as Distribution<(A, B, C, D, E, F, G)>>
             <Standard as Distribution<(A, B, C, D, E, F, G, H)>>
           and 62 others

What am I doing wrong?

TheIronBorn commented 8 months ago

Do you have the simd_support crate feature enabled?

We seem to have overlooked that in documentation

TheIronBorn commented 8 months ago

To be clear: Standard: Distribution<__m128i> requires the simd_support crate feature.

However, in v0.8.5 simd_support is broken due to packed_simd and it seems rather permanent this time. If you still need the trait impl, you can try the 0.9 alpha

TheIronBorn commented 8 months ago

Is anyone aware of a way to fix the documentation if the simd_support feature won't compile?

dragomang87 commented 8 months ago

I hadn't notice the simd_support as I was not expecting nightly features to be needed. This would be the std::simd nightly crate right? I guess the easiest at this point is to keep using rust stable, generate two i64 and use _mm_set_epi64x. It seems the code for Distribution<__m128i> generates __m128i indirectly by first creating u8x16 using .fill_bytes() anyway.

Bitwise speaking, does it makes a difference if the __m128i is constructed by first randomly generating u8x16, i64, u64, i32, u32, ... ? If I read the code correctly they all use .fill_bytes() so it should make no difference.

Also, looking at `rand/distributions/integer.rs:13:

#[cfg(all(target_arch = "x86", feature = "simd_support"))]
use core::arch::x86::{__m128i, __m256i};
#[cfg(all(target_arch = "x86_64", feature = "simd_support"))]
use core::arch::x86_64::{__m128i, __m256i};

it seems that the __m128i is indeed the one from core::arch so the simd_support is only needed because the implementation goes through generating u8x16 first. Wouldn't it have made more sense to indeed generate a pair of u64 or i64 instead and avoid needing simd? That way Distribution<__m128i> could go stable independently of simd.

PS: why does integer.rs:118 use from_bits of the FromBits trait? The only place where I found it is in the Ubuntu nightly documentation from web.mit.edu which is Version 1.26.0 (a77568041 2018-05-07) from 2018. Both stable and nightly define From trait with the from method. What's the reason for the different implementation?

TheIronBorn commented 8 months ago

Yes we are dropping the feature requirement in 0.9 as well as the from_bits along with the whole packed_simd crate

dhardy commented 3 months ago

This is about documentation of feature flags. May be affected by #1450. There's still no obvious doc here: image

dhardy commented 1 month ago

The above is the expected doc for 0.9: this feature requires x86 or x86-64 but does not require simd_support.