Is `keyword_argument = Val{true}` good practice or an antipattern?

I am seeing more and more library code that is written with keyword arguments of the form kwarg = Val{true} instead of kwarg = true or even kwarg::Bool = true. I know this is done in order to force specialization in precompilation which drastically improves time-to-first-X. However, I can not shake the feeling that it is plain ugly, a legibility sacrifice for a performance idiosyncrasy in Julia (that hopefully would be fixed in the future). After all, we all want a magically fast language that is as expressive as the slow magically dynamic language (I acknowledge that creating the magic is a monumental effort).

I would be grateful to hear from more advanced users of the language how they feel about this style.

1 Like

I probably don’t classify as an advanced user, but IMHO that should only be used as a very late stage optimization if and only if the specific function call was detected to be a bottleneck.

But even then, wouldn’t that lead to two separate styles of keyword calls? Half the libraries would use kwarg=true and the rest kwarg=Val{true}. The inconsistency is the part that bothers me the most.

sounds like a bad idea to me, I think the community consensus is we shouldn’t litter code base with hacks in pursue of the performances or latency, people have faith in fixing them in releases

2 Likes

Likely that is a case to case discussion, but I think we all resort to hacks when those fix some important usability issue. Here and in any other language. The thing is that hacks should only be used as last resort, and likely only in internal functions, so we should not see them all around anyway.

2 Likes

I am also not the most advanced user, and my take is the following: it is okay to use Val{true} if you feel that your function should specialize based on that flag, I have done it in the past, in circumstances where I felt I had basically two different algorithms sharing a body of code, and that many if statements inside hot loops would be optimized away. However, doing it solely for precompilation does seems a little hacky, because it seems like a consequence that is too distanced from the semantics of the specific piece of code.

1 Like

I agree this is bad style and should be avoided wherever possible. If it’s really necessary, I’d prefer the variant kwarg = Val(true) which produces the instance Val{true}() rather than the type Val{true} and was introduced in around Julia 1.0. At least that way, the dispatch is not based on matching Types which is generally much cleaner.

In modern Julia, many (most?) of the use cases for Val(true) and Val(false) are unnecessary because the optimizer has become quite good at mixing constant propagation, type inference and inlining.

In the past, the optimizer couldn’t do this, so something as simple as

f(x, flag) = flag ? Int(x) : Float64(x)

couldn’t be optimized into a concrete type even when flag was a constant in the caller of f, even though f was inlined! For example all the way back in Julia 0.6:

julia> f(x, flag) = flag ? Int(x) : Float64(x)
f (generic function with 1 method)

julia> g(x) = f(x, true)
g (generic function with 1 method)

julia> @code_warntype g(1)
Variables:
  #self# <optimized out>
  x::Int64
  #temp#::Union{Float64, Int64}

Body:
  begin 
      $(Expr(:inbounds, false))
      # meta: location REPL[64] f 1
      unless true goto 6
      #temp#::Union{Float64, Int64} = x::Int64
      goto 8
      6: 
      #temp#::Union{Float64, Int64} = (Base.sitofp)(Float64, x::Int64)::Float64
      8: 
      # meta: pop location
      $(Expr(:inbounds, :pop))
      return #temp#::Union{Float64, Int64}
  end::Union{Float64, Int64}

There’s cases where Val is still necessary to force specialization based on a flag in a deeply nested computation. However in these cases a package’s public API would generally be better if it took a normal Bool and converted it into a Val for use internally, if necessary.

6 Likes

Yeah, I agree with this. In Julia 1.x, I don’t know if there’s a reason to prefer Val{true} (unless if you still carry some pre 1.x codebase). On the other hand, you can loose the advantage of Val if you accidentally put Val{true} in some containers like a tuple:

julia> typeof(tuple(Val{true}))
Tuple{DataType}

julia> typeof(tuple(Val(true)))
Tuple{Val{true}}

The Julia compiler is often clever enough to optimize out the problem that comes with passing around the type Val{true}. But since the purpose of Val is to explicitly communicate what has to be statically known, depending on the optimizer to avoid the problem with Val{true} seems to be a bad idea. So, when using Val, I think it’s better to pass around the instance Val(true).

4 Likes

I think this is not the reason people are using Val{true} today. It is not for optimizing the runtime by ensuring the compiler does something smart. Rather, I am seeing it used because it ensures pre-compilation works and import time is drastically lowered. And it seems to be used in new important libraries like Symbolics.jl. Or maybe I am completely misunderstanding its purpose here?

Would not be better to ask the package maintainers why they are doing this instead of speculating on it?

Doing this for keyword arguments is a bit pointless since the types of kwargs don’t participate in method dispatch: Methods · The Julia Language

They do not participate in method dispatch, but they participate in what and how is being precompiled.

My question is not specifically about one package.

That’s kinda the point. Outside of method dispatch, the instance Val(true) , the type Val{true} and the constant true are just different values (and constant ones at that). So, especially in light of the continuing improvements in constant propagation, it really doesn’t make any difference for keyword arguments, because it’s the same thing as propagating any other value. Types are all just a value of Type .

I am not referring to the quality of the compiled code - that would indeed be the same either way. However, from what I have read on this forum, this has an effect on whether Julia decides to pre-compile your code, and how long it takes to import your package. See the TTFX thread from last week Taking TTFX seriously: Can we make common packages faster to load and use - #15 by Krastanov

1 Like

As hinted at above, if you really are obsessing about making sure as much as possible gets precompiled, then it would be better to not doing anything fancy in the kwargs and just have an internal method. In other words, let the user specify a normal value and you wrap it as a Val singleton instance:

f(x::SomeType; kw=true) = f(x, Val(kw))

function f(x::SomeType, ::Val{true}) 
    # something to definitely precompile
end

function f(x::SomeType, ::Val{false})
    # something to definitely precompile
end

You should probably do

function f(x::SomeType, ::Val{T}) where T
    ... # code that branches on T
end

Otherwise you probably end with a ton of code repetition or you never needed a boolean keyword.

1 Like

This (anti-)pattern makes a function, which I wrote, type-stable. What should I use as a better way?

I have a function that extract specific components from two plain vectors (state and parameters of an ODESystem-ODEProblem). The order of the components is only known after constructing the ODEProblem from the system, and I need a way to index it efficiently and conveniently by Symbols.

One version returns the plain vector and another version returns a LabelledArray to actually see the components. With a plain bool argument the result istype-unstable.

One alternative would be providing two differently named functions each returning its own result type.

Another alternative would require the use to explicitly call label_paropt himself, but he would need to pass the position-label-information (in ProblemParSetter) twice.

function get_paropt(ps::ProblemParSetter, u0::Vector, p::Vector; label::Val{LABEL}=Val(false)) where LABEL
    v = [(first(t) == :par) ? p[last(t)] : u0[last(t)] for t in ps.optinfo]
    LABEL ? label_paropt(ps,v) : v
end

function label_paropt(ps, popt::Vector); LArray{paroptsyms(ps)}(popt); end
1 Like

You will probably get very different answers depending on who you are asking. I am starting to lean to “restructure the code so that there are no functions of that kind”, but that could lead to severe code-duplication.

The next option is “write a ::Val{bool} ‘private’ function and then just a ::Bool ‘public’ function”, as type instability in the outer-most user-facing code probably does not matter for performance. But that fails when you are worrying about functions that users might be using in tight loops.

Which leads to "give up and use the ugly ::Val{bool}" if the above are not an option. The compiler/language is just not yet good enough for a less hacky solution.

The label::Val{LABEL}=Val(false) hints to code smells and encourages rethinking the design.

From rethinking my specific example tells, I conclude that extracting an unlabeled or a labeled subset from the parameters are really two different usages. They warrant two different functions.

function get_paropt(ps::ProblemParSetter, u0::Vector, p::Vector) 
    v = [(first(t) == :par) ? p[last(t)] : u0[last(t)] for t in ps.optinfo]
end

function get_paropt_labeled(ps::ProblemParSetter, u0::Vector, p::Vector) 
    v = get_paropt(ps, u0, p)
    label_paropt(ps,v)
end
1 Like