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

Addition of UniformScaling with scalar breaks associativity #17083

Closed
toivoh opened this issue Jun 24, 2016 · 7 comments
Closed

Addition of UniformScaling with scalar breaks associativity #17083

toivoh opened this issue Jun 24, 2016 · 7 comments
Labels
linear algebra Linear algebra

Comments

@toivoh
Copy link
Contributor

toivoh commented Jun 24, 2016

Currently, addition of a UniformScaling and a scalar results in a scalar:

julia> I + 1
2

But this breaks associativity:

julia> Z = zeros(Int,2,2)
2×2 Array{Int64,2}:
 0  0
 0  0

julia> (Z + I) + 1
2×2 Array{Int64,2}:
 2  1
 1  2

julia> Z + (I + 1)
2×2 Array{Int64,2}:
 2  2
 2  2

If I+1 had produced 2I instead, the last result had been [2 0; 0 2], so it is not how addition of UniformScaling and scalars is defined that is the problem, but simply the fact that it is defined at all.

I suggest to deprecate and then remove addition of UniformScaling and scalars.

@kshyatt kshyatt added the linear algebra Linear algebra label Jun 24, 2016
@andreasnoack
Copy link
Member

andreasnoack commented Jun 24, 2016

So after all, automatic broadcasting of + and - is ambiguous. Maybe it's time to require .+ again. 😃

@StefanKarpinski
Copy link
Sponsor Member

Wouldn't defining I + 1 to produce 2I make this associative? Meanwhile I .+ 1 should continue to produce 2.

@andreasnoack
Copy link
Member

I don't think so. Wouldn't it be
(I + 1) + zeros(2,2) = 2I + zeros(2,2) = eye(2)
but
I + (1 + zeros(2,2)) = I + ones(2,2) = [2 1; 1 2]

@StefanKarpinski
Copy link
Sponsor Member

Yeah, ok, so I guess that I .+ 1 should be 2 and I + 1 should be an error. If we had insisted on .+ for matrix addition then this wouldn't have been a problem, but I think this isn't a huge sacrifice.

@toivoh
Copy link
Contributor Author

toivoh commented Jun 24, 2016

I don't have access to julia atm, but since + has basically been defined to act like .+ when one argument is a scalar, and I stands for an identity matrix of indeterminate size, I would expect

(Z .+ I) .+ 1

to give the same result as

(Z + I) + 1

and thus give rise to the same problems. So now that you mention .+, I don't think that I .+ 1 should be legal either.

If we had gone with requiring .+ to add a scalar elementwise to an array, then scalars would have worked more or less as UniformScaling does now (except when broadcasting at least) and we probably wouldn't have needed I, which would indeed have got rid of this issue.

I think it was clear when we let 1+A::Array be elementwise that (1+A)*B would not produce the same result as 1*B + A*B violating distributivity, but we accepted that infraction to avoid the inconvenience of writing so many dots.

But I don't think that I+1 or I.+1 provides anywhere near that kind of utility in exchange for violating distributivity.

@dlfivefifty
Copy link
Contributor

I think this issue needs to be revisited when +(::Matrix, ::Number) is removed in favour of .+, as the example is no longer valid.

I propose removing the deprecation and restoring the original behaviour of +(::UniformScaling, ::Number).

@Keno
Copy link
Member

Keno commented Oct 26, 2017

Just to complete the reference cycle the un-deprecation was done in #23923.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra
Projects
None yet
Development

No branches or pull requests

6 participants