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

consistent eltypes for all scalar+scalar, scalar+array, array+array ops #14252

Closed
StefanKarpinski opened this issue Dec 3, 2015 · 23 comments
Closed
Labels
good first issue Indicates a good issue for first-time contributors to Julia Hacktoberfest Good for Hacktoberfest participants help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@StefanKarpinski
Copy link
Member

See discussion here: https://groups.google.com/forum/#!topic/julia-users/hA0FLKqYEOs. We've already brought many scalar+scalar operations in line with the array+array behaviors; we should probably take this all the way so that these behaviors are always consistent.

@StefanKarpinski StefanKarpinski added help wanted Indicates that a maintainer wants help on an issue or pull request good first issue Indicates a good issue for first-time contributors to Julia labels Dec 3, 2015
@stephancb
Copy link

I have now tried probably all of the standard combinations (with v4.1), but not the more "esoteric" ones (with rational or complex).

scalar+scalar operations seem to be already in line with array+array behaviors. An outlier is array+scalar, where an integer array is not promoted if the scalar is a larger integer. I had stumbled over this behaviour.

For Int_n_ + UInt_m_, where n,m in 8,16,32, scalar+scalar and array+array promote to always Int64, which can be a bit wasteful, but is probably rarely used:

julia> Int8[1]+UInt8[2]
1-element Array{Int64,1}:
3

Int16 would be sufficient in this case. For array+scalar there is no promotion, so some alignment would be needed.

@tkelman
Copy link
Contributor

tkelman commented Dec 3, 2015

An outlier is array+scalar, where an integer array is not promoted if the scalar is a larger integer.

I think that's intended, as promoting the element type of the array could result in substantially more memory allocation for the result and may not be ideal to do unless specifically asked for.

I thought we changed the integer promotion rules, but maybe not when combining signed with unsigned?

@StefanKarpinski
Copy link
Member Author

This is kind of a mess but it is consistent between scalars and arrays:

julia> typeof(Int8(1) + Int8(2))
Int8

julia> typeof(Int8[1] + Int8[2])
Array{Int8,1}

julia> typeof(Int8(1) + UInt8(2))
Int64

julia> typeof(Int8[1] + UInt8[2])
Array{Int64,1}

julia> typeof(Int8(1) + Int16(2))
Int16

julia> typeof(Int8[1] + Int16[2])
Array{Int16,1}

@stephancb
Copy link

As a user I would expect and prefer, if also array+scalar were consistent with array+array and scalar+scalar. If I wanted to avoid getting an in memory large array, I had used e.g.

julia> repmat(Int8[0:126;], 1000000, 1)+Int8(1)

instead of

julia> repmat(Int8[0:126;], 1000000, 1)+1

In summary, a "high degree of consistency" could be achieved with the following changes:

  • make also the array+scalar (and array*scalar) consistent with the general rule to promote integers to the larger one of both
  • make also the signed+unsigned combination consistent with the general rule to not promote integers larger than the largest of the operands, e.g. Int8+UInt8 -> Int8 (or Int16?). This allows the user to always explicetely ask for a result consuming minimal memory, at the risk of getting InexactErrors.

@J-Sarnoff
Copy link

+1

@ScottPJones
Copy link
Contributor

Would it be possible to also have a parallel set of "safe" promotion rules, that would be guaranteed to return a type that could fit any value from either, without truncation or rounding, UInt128+Int128 -> BigInt, UInt16+UInt16->UInt16, Int8+UInt8->Int16, Int64+Float64->BigFloat, etc.)?
The current rules, which can take an Int64 and convert to a Float64, without any errors even if a value doesn't fit, are troublesome, they assume that preserving the size of the box is more important than preserving information. IMO it would be good to at least have the option of non-lossy promotions.

@J-Sarnoff
Copy link

The checked versions of arith ops checked_add, checked_sub, checked_mul do alert.
Are you requesting strongly widening quiet variants, e.g. widened_add?

@ScottPJones
Copy link
Contributor

No, I was just talking about promotion rules, so that you don't get things like this silently happening:
promote(1.0, 1234123423412341234) -> (1.0,1.2341234234123412e18)

@rawls238
Copy link
Contributor

rawls238 commented Dec 6, 2015

going to take a shot at this

@sarvghotra
Copy link
Contributor

@rawls238 Are you working on this ? If not I would like to.

@sarvghotra
Copy link
Contributor

@StefanKarpinski So what is the final thing to do for this issue?
Is it fine to fix it by implementing the summary given by stephancb in above comment ?

@StefanKarpinski
Copy link
Member Author

The open question seems to be what to do in the same-size, mixed signedness case, e.g. Int8 + UInt8. We currently promote Int + UInt => UInt, which I think should maybe be changed to produce Int, in which case it would make sense to have Int8 + UInt8 => Int8 and raise an error if the value can't be represented by an Int8.

@JeffreySarnoff
Copy link
Contributor

I agree. Julia views UInts more as machine numbers than as non-negative integers. Promoting Int + UInt => UInt puts the higher level type (Int, a subset of the Integers) under the lower level type (UInt). That is inappropriate; also people using UInts as non-negative integers would be surprised to see the signedness of Int in Int + UInt go away (as it does currently) while people using UInts as machine numbers generally know what's up with promotion rules.

@StefanKarpinski
Copy link
Member Author

There's also the argument that it's much easier to do a computation that gets a smallish negative value than a computation that gets a huge value. Obviously, they're both equally possible, but you get what I mean.

@sarvghotra
Copy link
Contributor

@StefanKarpinski

typeof(Int8(5) + UInt(6))
UInt64

julia> a = Int8(5) + UInt8(6)
11

julia> typeof(a)
Int64

Why different promotion here ?

@StefanKarpinski
Copy link
Member Author

I'm proposing that they should be the same.

@StefanKarpinski
Copy link
Member Author

The reasoning for the current behavior is that Int can represent all possible values of Int8 + UInt8 whereas UInt cannot. Neither Int nor UInt can represent all possible values of Int + UInt, so neither choice is a clear winner, but the choosing Int, as you point out, would have the advantage of making the result type more consistent.

@JeffreySarnoff
Copy link
Contributor

At last count, there were no votes for continuing the inconsistency.

@eschnett
Copy link
Contributor

eschnett commented Mar 1, 2016

One argument that appeared in the last round of discussions is that all literal numbers are Int, and thus e.g. adding a constant to an UInt becomes cumbersome. (I'm not saying I'm in favour of this, I'm just trying to ensure this argument isn't forgotten.)

I'd also invert the "higher level type" argument: Clearly, Int is the common case, and UInt is more special in the sense that it's used much less often. Thus, if you're using UInt, you're aware of it, and it's likely that you want to get an UInt again if you perform an operation on an UInt.

@nalimilan
Copy link
Member

Indeed, AFAIK the rule as regards integer sizes is that non-standard types win.

@JeffreySarnoff
Copy link
Contributor

agree to disagree (my experience with UInts is I need to avoid mixing Ints to keep the calc clean).

and how do you alter the background of the type names?

@tkelman
Copy link
Contributor

tkelman commented Mar 1, 2016

and how do you alter the background of the type names?

@JeffreySarnoff with backticks UInt, see https://guides.github.com/features/mastering-markdown/

@JeffBezanson
Copy link
Member

I think this is now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Indicates a good issue for first-time contributors to Julia Hacktoberfest Good for Hacktoberfest participants help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

No branches or pull requests