Converting between integer types #22235

arghhhh · 2017-06-06T02:11:07Z

Julia currently has two ways of converting between integer types.
You can construct one integer type from another and this is range checked eg:

julia> Int8(1000)
ERROR: InexactError()

The other way is to use the modulo operator %:

julia> 1000 % Int8
-24

This never fails and just chops the MSBs to give you the answer modulo style - and this is very fast (a single LLVM instruction).

This pull request adds a third method - where you also want the conversion to always succeed, but you want the closest representable value. This is essentially the clamp function followed by the % operator - so I've extended the functionality of clamp rather than make up a new name. eg:

julia> clamp( 260, UInt8 )
0xff

For completeness, I have also added the equivalent in-place function clamp! though it cannot change the type.

ararslan · 2017-06-06T03:13:34Z

base/math.jl

@@ -41,9 +41,11 @@ end

 """
    clamp(x, lo, hi)
+    clamp(x, T)


Typically in Base APIs, the type argument comes first. So this should be clamp(T, x).

Documentation wasn't updated.

I pushed a commit to fix that.

ararslan · 2017-06-06T03:13:49Z

base/math.jl

@@ -59,10 +61,13 @@ clamp(x::X, lo::L, hi::H) where {X,L,H} =
                  convert(promote_type(X,L,H), lo),
                  convert(promote_type(X,L,H), x)))

+clamp( x, ::Type{T} ) where {T} = clamp( x, typemin(T), typemax(T) ) % T


No spaces around parens (here and elsewhere)

ararslan · 2017-06-06T03:14:29Z

How does this differ from trunc(T, x)?

TotalVerb · 2017-06-06T03:16:31Z

julia> trunc(Int8, 1000)
ERROR: InexactError()
Stacktrace:
 [1] trunc(::Type{Int8}, ::Int64) at ./int.jl:407

julia> clamp(Int8, 1000)
127

TotalVerb · 2017-06-06T03:18:40Z

base/math.jl

@@ -72,6 +77,13 @@ function clamp!(x::AbstractArray, lo, hi)
    x
 end

+function clamp!(x::AbstractArray, ::Type{T}) where {T}


not strictly related, but is the clamp! function even necessary? Seems to me it could be written

A .= clamp.(lo, A, hi)

instead of

clamp!(A, lo, hi)

I completely agree. It seemed strange that the vector version of clamp has been deprecated but the in place version hadn't.

Isn't that the case with multiple in place functions right now, e.g:

fill!(A, x) -> A .= x

fill! is a little different IMO since it's intended to work on the array as a whole, whereas this clamp! method is more like implicit vectorization.

What does "intended to work on the array as a whole" mean?

I guess I think of fill! as more like an initialization operation for the array (even though it isn't exactly)

fredrikekre · 2017-06-06T18:35:36Z

FWIW, since the type T here has the role of both lo and hi would it not make more sense to define clamp(x, T) as the PR started? I like the symmetry more with clamp(x, hi, lo) and clamp(x, T). As @ararslan pointed out the type is usually the first argument though...

ararslan · 2017-06-06T18:38:39Z

Personally I think it's more important to be consistent with APIs.

rfourquet · 2017-06-06T18:45:39Z

I also prefer the symmetry with clamp(x, hi, lo) and clamp(x, T) for the function consistency. Even if rare, a type argument can be not the first argument, e.g. rem(x, T) vs rem(x, y) where the 2nd arg always specifies the destination range.

ararslan · 2017-06-06T18:47:40Z

clamp(T, x) matches trunc(T, x) though, to which it's related. Plus AFAICT no one directly uses the rem(x, T) method; it's more common to do x % T, which looks better than T % x.

StefanKarpinski · 2017-06-06T18:49:27Z

+1 to this functionality; not sure about the argument order although my first instinct is clamp(x, T). We could actually support both, but that's kind of messy.

fredrikekre · 2017-06-07T08:56:26Z

base/math.jl

@@ -41,9 +41,11 @@ end

 """
    clamp(x, lo, hi)
+    clamp(T, x)


I think that this would be more clear (i.e. more distinct) with two separate docstrings instead of trying to explain two things in the same docstring.
Something like:

""" clamp(T, x) Clamp `x` between `typemin(T)` and `typemax(T)` and convert the result to type `T`. ```jldoctest julia> clamp(Int8, 200) 127 julia> clamp(Int8, -200) -128 ``` """

fredrikekre · 2017-06-07T08:56:53Z

base/math.jl

@@ -59,6 +61,7 @@ clamp(x::X, lo::L, hi::H) where {X,L,H} =
                  convert(promote_type(X,L,H), lo),
                  convert(promote_type(X,L,H), x)))

+clamp(::Type{T}, x) where {T} = clamp(x, typemin(T), typemax(T)) % T


Maybe where {T<:Integer} ?

Yes. It doesn't make sense for floats - the % operator isn't defined, and the problem doesn't exist because conversion to a narrower float clamps to typemin and typemax (+/-Inf) anyway.

ararslan · 2017-06-07T17:33:30Z

base/math.jl

    clamp!(array::AbstractArray, lo, hi)
+    clamp!(array::AbstractArray, T)


This line should be removed since you aren't defining this method

rfourquet · 2017-06-08T09:10:22Z

Sorry to insist for the order of parameters, but in my eyes clamp(T, x) looks really confusing. The fact that a type often comes first in Base APIs is not a convention (AFAICT) we try to follow, like having the first arg being the mutated one in mutating functions. I would say that x % T "looks better than" T % x because we are used to having the left arg of % being the value of which we take the remainder, and the right arg specifying the range. Again I see clamp closer to % than to trunc, I feel it's gratuitous to change the argument order depending on whether the range is specified as a type or not. I would like more opinions before merging.

tkelman · 2017-06-08T18:52:00Z

Type first is a frequent convention when the return type (or element type) is part of what's asked for in an API, but not an absolutely universal one. I think clamp(T, x) would have more in common with related API's for functions other than clamp, and clamp(x, T) would have more in common with the other methods of clamp since it's used to determine the bounds.

If this had the same promotion behavior as clamp(x, lo, hi) (and there might be an argument that it should?) then I'd be more in favor of clamp(x, T), if it always returns type T then I'd be in favor of clamp(T, x).

rfourquet · 2017-06-08T20:30:53Z

Oh, I didn't notice the % T at the end of the definition of clamp(T, x). I think I prefer clamp(x, T) = clamp(x, typemin(T), typemax(T) without conversion to T...

ararslan · 2017-06-08T20:34:01Z

Not converting the result to T would then make it inconsistent with both trunc and %.

arghhhh · 2017-06-09T12:53:14Z

For me, the conversion is important - I want to be able to define clamping-conversions for my own integer types - preferably using a standardized interface.

How about:
clamp(T, x) converts - like trunc
clamp(x,T) doesn't convert - like clamp(x, typemin(T), typemax(T)) (I had this wrong originally)

and implement & document both.

fredrikekre · 2017-06-09T15:44:14Z

clamp(T1, x, T2) = clamp(x, typemin(T2), typemax(T2)) % T1
clamp(x, T2) = clamp(x, typemin(T2), typemax(T2))
Seems to follow the convention with optional return type T1 as first argument, and T2 takes the place of lo and hi.

arghhhh · 2017-06-09T17:08:39Z

Not sure about the three argument version:

julia> Base.clamp( ::Type{T1}, x::T3, ::Type{T2}) where {T1<:Integer,T2,T3} = clamp(x, typemin(T2), typemax(T2)) % T1

julia> clamp( Int8, 10000, Int16 )
16

This has overflowed rather than been clamped.

I guess we could try to limit the type T1 to be 'bigger than or equal to' T2. The original use of '%' was to do a quick and safe (from overflow) conversion.

It seems simpler just to have two variants of the two arg version:
clamp(T, x) clamps and safely converts - like trunc with clamping and never throwing
clamp(x, T) doesn't necessarily convert - it is exactly clamp(x, typemin(T), typemax(T)) which returns a value with the type promote_type(typeof(x), typemin(T), typemax(T))

ararslan · 2017-06-09T18:23:43Z

IMO having methods clamp(R, x, T)::R and clamp(x, T)::typeof(x) seems too confusing. I think we should stick to one approach/method here rather than converting in some cases and not in others.

arghhhh · 2017-06-09T19:17:48Z

My entire motivation for this was to define a standard name for a clamping-conversion. The existing clamp does not convert, and the one I want to add does. I don't think we need a version where we supply two type names though.

Keno · 2020-01-17T23:10:40Z

👍 to the clamp(x, T) argument order. The clamp function takes the range to clamp to after the second argument and here T specifies the range. The fact that that's also the return value is somewhat incidental, so the I don't think the return value argument applies.

This is #22235, but with the reversed argument order. ``` julia> clamp( 260, UInt8 ) 0xff ``` Co-authored-by: "arghhhh <[email protected]>"

Keno · 2020-01-17T23:21:58Z

Rebased with the reversed argument order in #34426.

This is #22235, but with the reversed argument order. ``` julia> clamp( 260, UInt8 ) 0xff ``` Co-authored-by: "arghhhh <[email protected]>"

arghhhh added 2 commits June 5, 2017 21:48

Two argument version of clamp and clamp!

25fa6f1

Removed whitespace

4a59f65

ararslan reviewed Jun 6, 2017

View reviewed changes

TotalVerb reviewed Jun 6, 2017

View reviewed changes

TotalVerb mentioned this pull request Jun 6, 2017

Deprecate clamp!(A, lo, hi) for broadcast!(clamp, A, A, lo, hi)? #22236

Closed

arghhhh and others added 2 commits June 6, 2017 09:16

Removed clamp\!, removed whitespace

27d9634

Fix docs

ed37037

fredrikekre reviewed Jun 7, 2017

View reviewed changes

arghhhh added 3 commits June 7, 2017 10:39

Merge branch 'master' of github.com:JuliaLang/julia into clamp2arg

6efde61

Separated docs for new clamp, restricted T<:Integer

fcd526e

Resolved conflict.. in clamp docstring

9a8a73c

ararslan reviewed Jun 7, 2017

View reviewed changes

Removed clamp! from doc string

2243307

ararslan approved these changes Jun 8, 2017

View reviewed changes

Keno added a commit that referenced this pull request Jan 17, 2020

Add clamp(x, T)

ac4e92a

This is #22235, but with the reversed argument order. ``` julia> clamp( 260, UInt8 ) 0xff ``` Co-authored-by: "arghhhh <[email protected]>"

Keno added a commit that referenced this pull request Jan 17, 2020

Add clamp(x, T)

5aed97e

This is #22235, but with the reversed argument order. ``` julia> clamp( 260, UInt8 ) 0xff ``` Co-authored-by: "arghhhh <[email protected]>"

Keno mentioned this pull request Jan 17, 2020

Add clamp(x, T) #34426

Merged

Keno added a commit that referenced this pull request Jan 19, 2020

Add clamp(x, T) (#34426)

5bed770

This is #22235, but with the reversed argument order. ``` julia> clamp( 260, UInt8 ) 0xff ``` Co-authored-by: "arghhhh <[email protected]>"

Keno closed this Jan 19, 2020

KristofferC pushed a commit that referenced this pull request Apr 11, 2020

Add clamp(x, T) (#34426)

1b0e987

This is #22235, but with the reversed argument order. ``` julia> clamp( 260, UInt8 ) 0xff ``` Co-authored-by: "arghhhh <[email protected]>"

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Converting between integer types #22235

Converting between integer types #22235

arghhhh commented Jun 6, 2017

ararslan Jun 6, 2017

omus Jun 6, 2017

ararslan Jun 6, 2017

ararslan Jun 6, 2017

ararslan commented Jun 6, 2017

TotalVerb commented Jun 6, 2017

TotalVerb Jun 6, 2017

arghhhh Jun 6, 2017

KristofferC Jun 6, 2017 •

edited

Loading

ararslan Jun 6, 2017

KristofferC Jun 6, 2017

ararslan Jun 6, 2017

fredrikekre commented Jun 6, 2017

ararslan commented Jun 6, 2017

rfourquet commented Jun 6, 2017

ararslan commented Jun 6, 2017

StefanKarpinski commented Jun 6, 2017

fredrikekre Jun 7, 2017 •

edited

Loading

fredrikekre Jun 7, 2017

arghhhh Jun 7, 2017

ararslan Jun 7, 2017

rfourquet commented Jun 8, 2017

tkelman commented Jun 8, 2017

rfourquet commented Jun 8, 2017

ararslan commented Jun 8, 2017

arghhhh commented Jun 9, 2017 •

edited

Loading

fredrikekre commented Jun 9, 2017

arghhhh commented Jun 9, 2017

ararslan commented Jun 9, 2017

arghhhh commented Jun 9, 2017

Keno commented Jan 17, 2020

Keno commented Jan 17, 2020

		clamp!(array::AbstractArray, lo, hi)
		clamp!(array::AbstractArray, T)

Converting between integer types #22235

Converting between integer types #22235

Conversation

arghhhh commented Jun 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan commented Jun 6, 2017

TotalVerb commented Jun 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC Jun 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikekre commented Jun 6, 2017

ararslan commented Jun 6, 2017

rfourquet commented Jun 6, 2017

ararslan commented Jun 6, 2017

StefanKarpinski commented Jun 6, 2017

fredrikekre Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfourquet commented Jun 8, 2017

tkelman commented Jun 8, 2017

rfourquet commented Jun 8, 2017

ararslan commented Jun 8, 2017

arghhhh commented Jun 9, 2017 • edited Loading

fredrikekre commented Jun 9, 2017

arghhhh commented Jun 9, 2017

ararslan commented Jun 9, 2017

arghhhh commented Jun 9, 2017

Keno commented Jan 17, 2020

Keno commented Jan 17, 2020

KristofferC Jun 6, 2017 •

edited

Loading

fredrikekre Jun 7, 2017 •

edited

Loading

arghhhh commented Jun 9, 2017 •

edited

Loading