Should `min.(1,f)` be broadcasting as `broadcast(x->min(1,x),f)`?

I ran into an issue where I expected min.(1,f) to be lowered to broadcast(min,1,f), but instead it is lowered to broadcast(x->min(1,x),f). There are a few issues with this behaviour:

  1. It breaks the “semantic” contract of broadcasting.
  2. Every constant needs to recompile.
  3. In my case, f is a Fun, and I want to override broadcast(::typeof(min),::Number,::Fun) to calculate break points. This override is not called with the current behaviour. (The current approach of overriding min(::Number,::Fun) works, but it’s breaking the expected behaviour of min to return one of the arguments.)

Is there an argument in favour of the current approach? Or should I create an Issue on GitHub?

1 Like

Inlining literals is significantly faster than passing them as broadcast arguments. Because of fusion, overloading broadcast(::typeof(min), ...) is not very useful anyway since your method won’t be called as soon as someone combines min with another dot call. e.g. your function wouldn’t be called for min.(x, f.^2) either.

It’s only faster if it doesn’t have to recompile.

Yes, it won’t fuse properly, but if a user wants the correct behaviour, they’d need to write it without fusing.

But I think my biggest argument was that it’s surprising as a user. In particular, min(1+1,f) and min(2,f) dispatch differently:

julia> struct Foo end

julia> Base.broadcast(::typeof(min),x::Number,f::Foo) = "hi"

julia> min.(1,Foo())
ERROR: MethodError: no method matching isless(::Foo, ::Int64)
Closest candidates are:
  isless(::AbstractFloat, ::Real) at operators.jl:98
  isless(::Real, ::Real) at operators.jl:266
Stacktrace:
 [1] (::##1#2)(::Foo) at ./<missing>:0
 [2] broadcast(::Function, ::Foo) at ./broadcast.jl:434

julia> min.(1+3,Foo())
"hi"

I would suggest removing the special case, as there is still access to it via (x->min(1,x)).(f).

No recompilation will happen because it doesn’t redefine anything. It needs to be compiled as many time as it appears in the code which is consistent with everything else.

The difference is supposed to be invisible and any method definition that breaks this is invalid.

Also, if you are defining a method that does not support any code transformation done by the dot syntax, it’s a clear sign that it should just be defined as a normal function instead.

No recompilation will happen because it doesn’t redefine anything. It needs to be compiled as many time as it appears in the code which is consistent with everything else.

I’m thinking more in the REPL usage:

julia> @time broadcast(min,2,[1,2,3])
  0.074139 seconds (48.92 k allocations: 2.768 MiB)
3-element Array{Int64,1}:
 1
 2
 2

julia> @time broadcast(min,3,[1,2,3])
  0.000093 seconds (34 allocations: 1.578 KiB)
3-element Array{Int64,1}:
 1
 2
 3

julia> @time min.(1,[1,2,3])
  0.622028 seconds (38.05 k allocations: 1.993 MiB)
3-element Array{Int64,1}:
 1
 1
 1

julia> @time min.(1,[1,2,3])
  0.025320 seconds (4.05 k allocations: 228.566 KiB)
3-element Array{Int64,1}:
 1
 1
 1

julia> @time min.(2,[1,2,3])
  0.024821 seconds (4.04 k allocations: 228.160 KiB)
3-element Array{Int64,1}:
 1
 2
 2

That doesn’t make any difference in real timing.

  • The REPL is way slower than this compilation time.
  • These are global statements so they are never assumed to be fast.

In my case, min.(1,f) will just not work, so the choice is either it sometimes work or it never works. I’d prefer sometimes works.

A related example: I would argue that a reasonable override is

broadcast!(::typeof(*), a::Number, b::Vector) = scale!(a,b)

(assuming scale! is actually faster). Now this would not work when fused, but so what? If it’s faster sometimes, then it’s worthwhile.

Julia is not a static compiled language. The REPL is real timing for a lot of people.

So don’t redefine broadcast as I mentioned?

I hope you mean Vector first, but

  1. No it won’t be faster (if it is, it’s a performance bug).
  2. If you are just defining optimization, it is perfectly valid and you are not supposed to be able to observe the difference.

The high performance part of julia is a statically compiled language.

That’s exactly why we document and comment in all different places so that people don’t do this.

One needs to be able to override broadcast to use generic code (e.g., DifferentialEquations.jl).

Besides, it wasn’t my idea to override broadcast, I just merged a pull request from @stevengj

My point is less about this particular usage, but rather this sort of optimization is morally equivalent to compiler optimizations (a la Matlab), where the speed and behaviour of min.(1,f) is inconsistent with the implied lowering to broadcast(min,1,f).

That is never meant to be the implied lowering.

To quote https://docs.julialang.org/en/release-0.6/manual/functions/#man-vectorized-1:

That doc can be clarified for it excludes fusing.
In general, the lowering should be allowed to do any transformation that’s valid for the base broadcast implementation.

More generally, f.(args…) is actually equivalent to broadcast(f, args…)

That relation implies (and the underlying implementation assumes) that f is defined for the element types (or type if not with a size) of the container-like objects in args.

I opened up an issue to fix the docs:

https://github.com/JuliaLang/julia/issues/23445

Thinking about this a bit more, there might be a way to make the literal transformation after parsing, so if someone overwrites broadcast(::typeof(f), ...) it will get called when no dot-fusion occurs.

The only problem is that it can prevent that a broadcast call with a lot of arguments gets inferred (given the restrictions to avoid handling potentially infinite types). But this is a already a problem anyway.

If I recall correctly, I suggested that you override broadcast for arbitrary functions f, so that it works with fusion etcetera…

Yes that’s what’s implemented. But it doesn’t work for functions that require break points, like abs.(x) or min.(1,x).

The latter example is challenging because the documentation for min says it should return one of the inputs, so the current pointwise implementation of min(1,f) is possibly confusing.