You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#122 added clamp_min and clamp_max functions which preserve NANs in the input. I didn't see any discussion about what the best behavior is when the min/max argument is NAN, and I think it's worth considering before clamp_min/clamp_max are included in an official release. I think four different possible behaviors are reasonable when min/max is NAN:
Panic in debug mode
Panic in both debug and release mode
Return input
Return NAN
Option 1 matches num_traits::clamp. Option 2 matches (currently unstable) f32::clamp/f64::clamp. Option 3 is what the implementation currently does.
I lean towards option 1 for consistency with num_traits::clamp and because it avoids the question of whether option 3 or option 4 makes more sense.
This is important to consider -- thanks for bringing it up! I think consistency with num_traits::clamp would be best -- panic in debug mode, and return input in release mode. Maybe in a future release (semver bumped) we could consider making them all always panic like the std clamp methods.
#122 added
clamp_min
andclamp_max
functions which preserve NANs in theinput
. I didn't see any discussion about what the best behavior is when themin
/max
argument is NAN, and I think it's worth considering beforeclamp_min
/clamp_max
are included in an official release. I think four different possible behaviors are reasonable whenmin
/max
is NAN:input
Option 1 matches
num_traits::clamp
. Option 2 matches (currently unstable)f32::clamp
/f64::clamp
. Option 3 is what the implementation currently does.I lean towards option 1 for consistency with
num_traits::clamp
and because it avoids the question of whether option 3 or option 4 makes more sense.Thoughts?
cc @termoshtt @cuviper
The text was updated successfully, but these errors were encountered: