Skip to content

Confusing difference between float and integer in min and max #247

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

Closed
lukaslihotzki opened this issue Feb 22, 2022 · 3 comments · Fixed by #260
Closed

Confusing difference between float and integer in min and max #247

lukaslihotzki opened this issue Feb 22, 2022 · 3 comments · Fixed by #260

Comments

@lukaslihotzki
Copy link

I tried this code:

let a = u32x4::from_array([0, 1, 0, 1]);
let b = u32x4::from_array([1, 0, 1, 0]);
print!("{:x?} ", a.min(b));

let a = f32x4::from_array([0., 1., 0., 1.]);
let b = f32x4::from_array([1., 0., 1., 0.]);
print!("{:x?}\n", a.min(b));

I expected to see this happen: [0, 0, 0, 0] [0.0, 0.0, 0.0, 0.0].
[0, 1, 0, 1] [0.0, 1.0, 0.0, 1.0] would be a little surprising, because that isn't commonly available as native SIMD instruction.

Instead, this happened: [0, 1, 0, 1] [0.0, 0.0, 0.0, 0.0]

This is inconsistent. I expected either min or lanes_min (like in lanes_eq) to get the minimum for each lane.

Also, when you search min( in https://rust-lang.github.io/portable-simd/core_simd/simd/struct.Simd.html#method.min-1, where it says "Returns the minimum of each lane.", you have to scroll up quite a bit to see it is for floats only and the documentation of min and max for integers is very generic, doesn't explicitly mention that this isn't per lane.

@programmerjake
Copy link
Member

we should probably add integer min/max operations.

@calebzulawski
Copy link
Member

I added them in #206 but that never made it to master 😔

@programmerjake
Copy link
Member

ran into this same issue with clamp in #253

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 a pull request may close this issue.

3 participants