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

Add clamp RFC #1961

Merged
merged 17 commits into from
Sep 6, 2017
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 141 additions & 0 deletions text/0000-clamp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
- Feature Name: clamp functions
- Start Date: 2017-03-26
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Add functions to the language which take a value and an inclusive range, and will "clamp" the input to the range. I.E.

```Rust
if input > max {
return max;
}
else if input < min {
return min;
} else {
return input;
}
```

Likely locations would be in std::cmp::clamp implemented for all Ord types, and a special version implemented for f32 and f64.
The f32 and f64 versions could live either in std::cmp or in the primitive types themselves. There are good arguments for either
location.
Copy link
Member

Choose a reason for hiding this comment

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

Should probably update this now that the RFC has made a decision on where.


# Motivation
[motivation]: #motivation

Clamp is a very common pattern in Rust libraries downstream. Some observed implementations of this include:

http://nalgebra.org/rustdoc/nalgebra/fn.clamp.html

http://rust-num.github.io/num/num/fn.clamp.html

Many libraries don't expose or consume a clamp function but will instead use patterns like this:
```Rust
if input > max {
max
}
else if input < min {
min
} else {
input
}
```
and
```Rust
input.max(min).min(max);
```
and even
```Rust
match input {
c if c > max => max,
c if c < min => min,
c => c,
}
```

Typically these patterns exist where there is a need to interface with APIs that take normalized values or when sending
output to hardware that expects values to be in a certain range, such as audio samples or painting to pixels on a display.

While this is pretty trivial to implement downstream there are quite a few ways to do it and just writing the clamp
inline usually results in rather a lot of control flow structure to describe a fairly simple and common concept.

# Detailed design
[design]: #detailed-design

Add the following to std::cmp

```Rust
/// Returns max if input is greater than max, and min if input is less than min.
/// Otherwise this will return input.
#[inline]
pub fn clamp<T: Ord>(input: T, min: T, max: T) -> T {
if input < min {
min
}
else if input > max {
max
}
else {
input
}
}
```

And the following to libstd/f32.rs, and a similar version for f64

```Rust
/// Returns max if self is greater than max, and min if self is less than min.
/// Otherwise this returns self.
///
/// # Examples
///
/// ```
/// assert!((-3.0f32).clamp(-2.0f32, 1.0f32) == -2.0f32);
/// assert!((0.0f32).clamp(-2.0f32, 1.0f32) == 0.0f32);
/// assert!((2.0f32).clamp(-2.0f32, 1.0f32) == 1.0f32);
/// ```
#[inline]
pub fn clamp(self, min: f32, max: f32>) -> f32 {
if input < min {
min
}
else if input > max {
max
}
else {
input
}
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative implementation which results in better code is:

pub fn clamp(input:f32, min: f32, max: f32) -> f32 {
    let mut x = input;
    if !(x < min) { x = min; }
    if !(x > max) { x = max; }
    x
}

It conveniently preserves the source when it is NaN, as required in the edge cases listed below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Looks great!

Copy link
Contributor

@ranma42 ranma42 Mar 28, 2017

Choose a reason for hiding this comment

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

... but it incorrectly returns NaN when max == NaN || min == NaN (EDIT: this is pseudocode... as crazy as it looks, the check for NaN would be max != max || min != min).
I am unsure if this is good or bad: while the infinity corresponding to no enforcement is straightforward, it is not obvious to me what is the desirable behaviour for NaN.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You know honestly that might be better behavior than what I proposed. It assumes a NaN is unintentional which they often are. If someone explicitly wants no bounds enforced they should provide infinity.

Copy link
Member

Choose a reason for hiding this comment

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

@ranma42 I think you mean if !(x >= min) etc.


There are 3 special float values the clamp function will need to handle, and 3 positions into which they can go so I will represent
the edge case behavior with a 3x3 chart.

| |INFINITY|NEG_INFINITY|NAN|
|---|---|---|---|
|self|return max;|return min;|return NAN;|
|max|No max enforced|return NEG_INFINITY;|No max enforced|
|min|return INFINITY;|No min enforced|No min enforced|

# How We Teach This
[how-we-teach-this]: #how-we-teach-this

The proposed changes would not mandate modifications to any Rust educational material.

# Drawbacks
[drawbacks]: #drawbacks

This is trivial to implement downstream, and several versions of it exist downstream.

# Alternatives
[alternatives]: #alternatives

Alternatives were explored at https://internals.rust-lang.org/t/clamp-function-for-primitive-types/4999

# Unresolved questions
[unresolved]: #unresolved-questions

Should the float version of the clamp function live in f32 and f64, or in std::cmp as that's where the Ord version would go?