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

Changed meaning of .+= in Julia 0.5 #147

Closed
stevengj opened this issue Jul 21, 2016 · 5 comments
Closed

Changed meaning of .+= in Julia 0.5 #147

stevengj opened this issue Jul 21, 2016 · 5 comments

Comments

@stevengj
Copy link

Your test code uses x .+= y, so you should know that in Julia 0.5 this has changed meaning to be equivalent to broadcast!(identity, x, x .+ y), so that it mutates the x array (see JuliaLang/julia#17510 … in Julia 0.6 the whole operation will occur in-place without temporaries). So .+ should only be used if the left-hand side is a mutable array, and you don't mind mutating it.

At first glance, this looks like it is okay for you, because you use it in rgb[:g,:] .+= 1, where rgb[:g,:] is an array that you are mutating anyway. But if it were a problem you could always change it to +=.

@SimonDanisch
Copy link
Owner

That's really great news! Thanks for the heads up!

@stevengj
Copy link
Author

stevengj commented Jul 21, 2016

Feel free to close if your code is okay. However, as I just noticed in JuliaLang/julia#17510, because rgb[:g,:] returns a copy and not a view, I think this may be broken until I can fix the .= to use view for left-hand-side getindex expressions; for now, you can do rgb[:g,:] = rgb[:g,:] .+ 1

@c42f
Copy link
Collaborator

c42f commented Jul 21, 2016

Yes, thanks for letting us know, lately I've been getting used to 0.5-dev breaking FixedSizeArrays without warning ;-)

In the tests, the expression @fslice rgb[:g,:] .+= 1 is being rewritten by @fslice into

destructure(rgb)[fieldindex(eltype(rgb),:g), :] .+= 1

which should use a view in broadcast as it updates rgb in place. However, I don't think this is a critical feature since I only just updated @fslice to allow opassign syntax recently. I'm fine with just waiting until the fix lands in 0.5 and testing then.

@stevengj
Copy link
Author

stevengj commented Jul 21, 2016

It should be fixed in JuliaLang/julia#17546 to call view, although I'm not sure whether that will fix @fslice. Does view(destructure(rgb), fieldindex(eltype(rgb),:g), :) work?

@c42f
Copy link
Collaborator

c42f commented Jul 22, 2016

Thanks @stevengj I just checked your branch, and @fslice works fine there. (destructure() simply reinterprets an Array{T<:FixedArray} as a higher dimensional Array with the fixed dimensions prepended to the original size, so anything which works with a normal array should work with the result of a destructure().)

@c42f c42f closed this as completed Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants