Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nanorand::gen::RandomRange::random_range(lower, upper) behaves incorrectly (upper is non-inclusive) #21

Closed
spitfire05 opened this issue Jan 6, 2021 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@spitfire05
Copy link

spitfire05 commented Jan 6, 2021

WyRand::new().generate_range::<u32>(0, 1); seems to always return 0, while the common understanding is that the upper parameter should be inclusive.

This seems to be the understanding of the crate's author as well, as in shuffle implementation:

fn shuffle<I, S: AsMut<[I]>>(&mut self, mut target: S) {
		let target = target.as_mut();
		let target_len = target.len();
		for idx in 0..target_len {
			let random_idx = self.generate_range::<usize>(0, target_len - 1);
			target.swap(idx, random_idx);
		}
	}

target_len - 1 basically means the last value in slice will never be selected as swap target, leading to some bizarre behavior, like shuffling a slice of len 2 always returns reversed slice, instead of giving roughly 50/50 distribution of original and reversed.

If the upper parameter was indeed meant to be non-inclusive, then I guess the docs should be updated and shuffle implementation fixed. If it was not, then the generate_range function has to be fixed.

Tested on crate v 0.5.2, Rust 1.48.0

EDIT: typos, formatting

@Absolucy Absolucy added bug Something isn't working good first issue Good for newcomers labels Jan 6, 2021
@Absolucy Absolucy added this to the 0.5.3 milestone Jan 6, 2021
@Absolucy
Copy link
Owner

Absolucy commented Jan 12, 2021

I believe this has been solved in d2b3ba5

tesselode added a commit to tesselode/kira that referenced this issue Feb 14, 2021
nanorand has some bugs and weirdness that make me not fully trust it

Absolucy/nanorand-rs#24

Absolucy/nanorand-rs#21

for a fundamental thing like randomness i'd rather use a battle-tested lib
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants