[Utilities] Maintenance of functions.jl

[odow]

I don't think I've ever read through src/Utilities/functions.jl in it's entirety, so I gave it a go. There's a lot of stuff in there...

I made changes as I went, and I'll make some comments on the bits that surprised me.

[odow]

• What is test_models_equal doing here? Can it be moved to MOI.Test?
• How many functions can we make private? There are so many helpers like fill_terms that only seem to be used in this file. Do you have a list of others that are used, e.g., in SumOfSquares? Just operate!? Just ones in the documentation?

[odow]

Theses mapreduce type paradigms require inference and compilation. Unless we have a good reason, we should just use loops.

[blegat]

So we can't use mapreduce anywhere ? Or is there something specific about this case ?

[odow]

In general, every function need to compile is a cost. Ideally, Julia would get better at this, but at the moment;

julia> bar(f, t) = 2 * f(t)
bar (generic function with 1 method)

julia> function foo1(f, x::Vector{T}) where {T}
           mapreduce(t -> bar(f, t), +, x, init = zero(T))
       end
foo1 (generic function with 1 method)

julia> function foo2(f, x::Vector{T}) where {T}
           y = zero(T)
           for t in x
               y += bar(f, t)
           end
           return y
       end
foo2 (generic function with 1 method)

julia> x = rand(3)
3-element Vector{Float64}:
 0.6235504005909198
 0.6123666282042153
 0.21192655166729657

julia> @time @eval foo1(xi -> xi^2, x)
  0.022403 seconds (57.70 k allocations: 3.550 MiB, 96.81% compilation time)
1.617441705433775

julia> @time @eval foo1(xi -> xi^2, x)
  0.011091 seconds (33.68 k allocations: 2.063 MiB, 95.65% compilation time)
1.617441705433775

julia> @time @eval foo2(xi -> xi^2, x)
  0.011673 seconds (9.85 k allocations: 579.826 KiB, 95.12% compilation time)
1.617441705433775

julia> @time @eval foo2(xi -> xi^2, x)
  0.003318 seconds (2.06 k allocations: 137.187 KiB, 86.98% compilation time)
1.617441705433775

It's worth noting that both still infer okay:

julia> @code_warntype foo1(xi -> xi^2, x)
Variables
  #self#::Core.Const(foo1)
  f::Core.Const(var"#71#72"())
  x::Vector{Float64}
  #57::var"#57#58"{var"#71#72"}

Body::Float64
1 ─ %1  = Main.:(var"#57#58")::Core.Const(var"#57#58")
│   %2  = Core.typeof(f)::Core.Const(var"#71#72")
│   %3  = Core.apply_type(%1, %2)::Core.Const(var"#57#58"{var"#71#72"})
│         (#57 = %new(%3, f))
│   %5  = #57::Core.Const(var"#57#58"{var"#71#72"}(var"#71#72"()))
│   %6  = Main.zero($(Expr(:static_parameter, 1)))::Core.Const(0.0)
│   %7  = (:init,)::Core.Const((:init,))
│   %8  = Core.apply_type(Core.NamedTuple, %7)::Core.Const(NamedTuple{(:init,), T} where T<:Tuple)
│   %9  = Core.tuple(%6)::Core.Const((0.0,))
│   %10 = (%8)(%9)::Core.Const((init = 0.0,))
│   %11 = Core.kwfunc(Main.mapreduce)::Core.Const(Base.var"#mapreduce##kw"())
│   %12 = (%11)(%10, Main.mapreduce, %5, Main.:+, x)::Float64
└──       return %12

julia> @code_warntype foo2(xi -> xi^2, x)
Variables
  #self#::Core.Const(foo2)
  f::Core.Const(var"#73#74"())
  x::Vector{Float64}
  @_4::Union{Nothing, Tuple{Float64, Int64}}
  y::Float64
  t::Float64

Body::Float64
1 ─       (y = Main.zero($(Expr(:static_parameter, 1))))
│   %2  = x::Vector{Float64}
│         (@_4 = Base.iterate(%2))
│   %4  = (@_4 === nothing)::Bool
│   %5  = Base.not_int(%4)::Bool
└──       goto #4 if not %5
2 ┄ %7  = @_4::Tuple{Float64, Int64}::Tuple{Float64, Int64}
│         (t = Core.getfield(%7, 1))
│   %9  = Core.getfield(%7, 2)::Int64
│   %10 = y::Float64
│   %11 = Main.bar(f, t)::Float64
│         (y = %10 + %11)
│         (@_4 = Base.iterate(%2, %9))
│   %14 = (@_4 === nothing)::Bool
│   %15 = Base.not_int(%14)::Bool
└──       goto #4 if not %15
3 ─       goto #2
4 ┄       return y

It's just that Julia struggles much more to infer foo1.

[blegat]

Here the developer is doing the job of the compiler though. If we start doing the compiler's job by hand just to gain compiler time, I don't feel it's going in the right direction. This time might be better spent improving the compiler. We don't want to spend our time doing a job that has already been automated.

[odow]

Yes, the compiler should be doing a better job. But the loop syntax is just as readable (if not more), has fewer allocations, and is quicker to run...

[odow]

@martinbiel uses a lot of these: https://github.com/martinbiel/StochasticPrograms.jl/blob/master/src/types/decisions/functions/operators.jl
But it looks like for most of it he may have just copy-pasted.

@dourouc05 has some too, but they're commented out?
https://github.com/dourouc05/ConstraintProgrammingExtensions.jl/blob/master/src/Bridges/helpers.jl

You can play with the search functionality here:
https://juliahub.com/ui/Search?type=code

It'd be nice to narrow down which parts are critical to implement and which parts are nice to have.

[dourouc05]

@dourouc05 has some too, but they're commented out?
https://github.com/dourouc05/ConstraintProgrammingExtensions.jl/blob/master/src/Bridges/helpers.jl

I wrote quite a few helpers, but I only kept uncommented those that are actually used in the code (the others were used previously, in potentially uncommitted code). If you're interested, I can make a PR for them, but I guess not right now :-).

[odow]

We'll keep everything as named for now. It's a bit late to make everything private, and there are reasonable external use-cases for these things.