-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Inference failure with 0.6 + StaticArrays hcat #21244
Comments
StaticArrays is doing bad things with its |
I don't see an |
I misremembered the name: https://github.com/JuliaArrays/StaticArrays.jl/blob/master/src/indexing.jl#L55-L66 |
I'm curious - what is the problem with these methods? |
Your |
To expand a bit: Inference ensures termination by limiting the complexity of types in terms of nesting depth, tuple length, etc. But if the argument to But while for this particular case a fix to StaticArrays seems in order, we might want to keep this issue open as a reminder that the inference termination heuristics are far from perfect. (Or do we have a dedicated issue for that?) |
While In the exceedingly rare case that this is a large number of input arguments, then the varargs would be so long that inference will give up and widen, and it would call Sorry, I'm just a bit confused what the problem is here, and what the solution is. Do I have a bug that makes |
While Oh, I should mention that I haven't checked whether this is the actual cause of the OP problem. |
Yes. At this time, it is assumed that an |
Ok, confirmed insofar as replacing all @inline _index_sizes(s::Size, v::Type{Val{1}}, x::Tuple, a::Colon, inds...) = _index_sizes(s, increment(v), (x..., Size(s[1])), inds...)
@inline _index_sizes(s::Size, v::Type{Val{2}}, x::Tuple, a::StaticArray, inds...) = _index_sizes(s, increment(v), (x..., Size(a)), inds...)
@inline _index_sizes(s::Size, ::Type{Val{3}}, x::Tuple) = x fixes it. So let me ask again, should we keep this issue open as a reminder that the inference termination heuristics are far from perfect, or do we have a dedicated issue for that? |
Anticipating this to be fixed in StaticArrays, here is a self-contained, somewhat reduced repro of the underlying problem for future reference: immutable Size{s}; end
Base.@pure increment(::Type{Val{N}}) where {N} = Val{N+1}
@inline index_sizes(s::Size, inds...) = _index_sizes(s, Val{1}, (), inds...)
@inline _index_sizes(s::Size, ::Type{Val{N}}, x::Tuple) where {N} = x
@inline _index_sizes(s::Size, v::Type{Val{N}}, x::Tuple, ::Int, inds...) where {N} = _index_sizes(s, increment(v), (x..., Size()), inds...)
@inline _index_sizes(s::Size, v::Type{Val{N}}, x::Tuple, a::Colon, inds...) where {N} = _index_sizes(s, increment(v), (x..., Size(s[N])), inds...)
code_warntype(index_sizes, Tuple{Vararg{Any}}) |
Working at fixing it in StaticArrays, I found another case that is also quite entertaining; reduced, self-contained repro: immutable Size{s}; end
# EDIT: The @pure here is not necessary to trigger the problem
#=Base.@pure=# tail(::Type{Size{S}}) where {S} = Size{Base.tail(S)}
@inline tail(::S) where {S<:Size} = tail(S)()
@inline index_sizes(s::Size, inds...) = _index_sizes(s, (), inds...)
@inline _index_sizes(s::Size, x::Tuple) = x
@inline _index_sizes(s::Size, x::Tuple, ::Int, inds...) = _index_sizes(tail(s), (x..., Size()), inds...)
@inline _index_sizes(s::Size, x::Tuple, a::Colon, inds...) = _index_sizes(tail(s), (x..., Size(s[1])), inds...)
for n in 1:16
t = @elapsed code_warntype(DevNull, index_sizes, NTuple{n, Any})
println("$n: $t")
end
@elapsed code_warntype(DevNull, index_sizes, Tuple{Vararg{Any}}) Exponential growth in run-time, similar to #20848 (comment), but even more pronounced. |
We will need to refactor the inference code somewhat substantially to be able to track this information correctly. For now, I simply recommend avoiding marking functions as |
For the OP case, where the recursion is infinitely deep, --- a/base/inference.jl
+++ b/base/inference.jl
@@ -1282,6 +1282,18 @@ function abstract_call_gf_by_type(f::ANY, atype::ANY, sv::InferenceState)
fullmatch = true
end
+ method_active_count = 0
+ for infstate in active
+ infstate === nothing && continue
+ infstate = infstate::InferenceState
+ if isdefined(infstate.linfo, :def) && method === infstate.linfo.def
+ method_active_count += 1
+ end
+ end
+ if method_active_count > 200 # or whatever limit makes most sense
+ sig = method.sig
+ end
+
# limit argument type tuple growth
msig = unwrap_unionall(m[3].sig)
lsig = length(msig.parameters) This fixes the case from #21244 (comment) (which should be equivalent to the OP case), and does not seem to have any adverse effects on first sight. I don't know how kosher this is, as for recursion looping through multiple functions/methods, it will depend on the entry function where this heuristic kicks in. Might be better than nothing, still? |
A much tougher problem seems to be a bifurcating recursion, e.g. the following non-sensical code: function foo(y, ::Any, xs...)
foo((y..., Val{0}()), xs...)
foo((y..., Val{1}()), xs...)
foo((y..., Val{2}()), xs...)
foo((y..., Val{3}()), xs...)
foo((y..., Val{4}()), xs...)
foo((y..., Val{5}()), xs...)
foo((y..., Val{6}()), xs...)
foo((y..., Val{7}()), xs...)
# add more for even greater fun
end
foo(y) = y Here, I think for cases like this, we would indeed need to track how often a certain method has been inferred (in the current inference run) to figure out that something funny is going on. |
For reference, the lines in StaticArrays @Keno was referring to are (Keno's link was referring to master, which has since changed). |
This set was introduced to test an inference issue (JuliaLang/julia#21244) in an old version of Julia. It currently fails on Julia master because of JuliaLang/julia#27276. In my opinion, it's currently not really testing anything valuable, and the Julia PR that fixed the issue has associated test cases that should cover this.
With StaticArrays latest master,
sometimes results in
other times in
versioninfo()
:StaticArrays SHA: bd3236ac39372aa84891bea066c71d63559af29e
cc: @andyferris
The text was updated successfully, but these errors were encountered: