Skip to content

Add fns Uniform::min, max using trait UniformSamplerRange#1775

Open
dhardy wants to merge 3 commits intomasterfrom
push-myxwltuvvqpy
Open

Add fns Uniform::min, max using trait UniformSamplerRange#1775
dhardy wants to merge 3 commits intomasterfrom
push-myxwltuvvqpy

Conversation

@dhardy
Copy link
Copy Markdown
Member

@dhardy dhardy commented Apr 25, 2026

  • Added a CHANGELOG.md entry

Summary

Add fns Uniform::min, max using trait UniformSamplerRange

Motivation

I wanted Uniform::max and realised it's simple enough to add.

Note: some rand_distr distribution objects already support querying parameters (example).

Details

trait UniformSamplerRange: UniformSampler is an extension-trait, adding optional support for min and max. This doesn't technically need the UniformSampler, though we do used its assoc. type X.

We could use a more generic name (like HasRangeBounds or HasMin and HasMax traits), but UniformSamplerRange fits in nicely (given the bound).

Adds min, max fns on Uniform for types supporting UniformSamplerRange.

Implements UniformSamplerRange for UniformInt samplers only.

I looked briefly at the other samplers and we certainly could implement these too, but code is less trivial. In particular, UniformChar would involve duplicating unsafe code. I suggest we leave these off for now and see if anyone else requests them.

@dhardy dhardy requested review from josephlr and newpavlov April 25, 2026 11:27
Comment thread src/distr/uniform_int.rs
for &(low, high) in $v.iter() {
let my_uniform = Uniform::new(low, high).unwrap();
assert_eq!(my_uniform.min(), low);
// assert_eq!(my_uniform.max(), high - 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these commented out?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the macro input uses both integers and Simd values. We'd need a generic Simd::splat supporting both... possible, but not in std I think so we'd need extra utility code (I started on this before deciding it's probably better not to increase the code size without good reason).

Copy link
Copy Markdown
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the trait? I think these could be just inherent methods. I am also not sure about max/min returning splatted SIMD values. Some may expect to get scalar values instead (i.e. u32 instead of u32x4). It also would resolve the assert problem.

@dhardy
Copy link
Copy Markdown
Member Author

dhardy commented Apr 27, 2026

Do we need the trait?

If we only implement these on UniformInt probably not. If we want the methods on Uniform too, I think we do (unless we fold them into UniformSampler, but that would be a breaking change and require extra impls).

I am also not sure about max/min returning splatted SIMD values.

UniformInt stores SIMD fields internally; we don't require range inputs to be splatted scalars.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants