Broadcasted Operations Stall Julia

[ChrisRackauckas]

When broadcasting some operations, Julia will get stuck. Example:

using StaticArrays
u0 = @SMatrix rand(4,2)
reltol = 0.003
uprev = u0
u = u0
max.(abs.(u),abs.(uprev)) .* reltol

This is tested on the latest release (on v0.6-rc2) and on StaticArrays master.

[c42f]

Yikes, I'm not sure what to make of this one yet. More minimal test case:

using StaticArrays
x = broadcast((a,b,c)->0, SVector(1,1), 0, 0)

[andyferris]

Oh dear. These should be our implementation of broadcast, I suppose.

[bshall]

I think the issue is in the broadcast_sizes function. If you manually define the sizes @c42f example works

s = (Size{(2,)}(), Size(), Size())
StaticArrays._broadcast((a,b,c)->0, s, SVector(1,1), 0, 0)

But calling StaticArrays.broadcast_sizes(SVector(1,1), 0, 0) hangs.

[bshall]

It seems like this could be a julia bug. A test case is

f(t::Tuple) = t
f(t::Tuple, a::Int, as...) = f((t..., a), as...)
f(t::Tuple, a::Float64, as...) = f((t..., a), as...)

f((), 1, 1, 1) # works fine
f((), 1.0, 1.0, 1.0) # works fine
f((), 1, 1.0, 1) # hangs

What do you guys think @andyferris @c42f is this a bug or is there something wrong with the recursion? By the way, it doesn't even start the recursion, julia hangs after the initial call.

As a workaround for StaticArrays you don't really need to define broadcast_sizes recursively, you could just loop over the as to build up the sizes.

[c42f]

Yes, I was just coming to the same conclusion myself, I think this is a julia bug, at least in julia-0.6.0-rc2.

Unfortunately we can't use a simple loop, as we need this to be fully type inferred. We might be able to get away with a carefully constructed generated function...

[bshall]

I just checked on 0.5.1 and it also doesn't work there. Should I go ahead and report it?

Unfortunately we can't use a simple loop, as we need this to be fully type inferred

Oh right, that makes sense.

[andyferris]

This is a bit odd. This is v0.6-rc2?

I know @vtjnash has been going on a mission lately to remove the "collector" pattern from Base because it doesn't interact with well with inference. See e.g. JuliaLang/julia#22019.

An example on how to do this pattern correctly is here:

https://github.com/JuliaLang/julia/pull/22019/files#diff-2264bb51acec4e7e2219a3cb1c733651R206

This pattern easily allows mapping, filtering, reducing and finding of tuple elements. It's important that types going to the inner functions are getting simpler (smaller tuples) not more complex (longer tuples). One "better" way to do our trivial f is:

f(a::Int, as...) = (a, f(as...)...)
f(a::Float64, as...) = (a, f(as...)...)

Note that the inner function has one less argument than the outer, so inference can feel safe that types are getting simpler. We should make StaticArrays adhere to such patterns, rather than the collector one (which I put everywhere).

[c42f]

Ok, nice that makes a lot of sense.

This is still a bug but the workaround seems easy.

[c42f]

PR coming.

[bshall]

I've got it working if you'd like me to do it?

[c42f]

Sure, push it up if you like. I've got it working locally but I was just refactoring to beef up the broadcast tests.

[andyferris]

I like the competition. :)

[c42f]

Hah, I should be working on my logging package instead, given juliacon is nearly on us. @bshall has kindly agreed to finish it off :-)