-
-
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
Strange type parameterization issue introduced around 30 June (or perhaps 16 June) #12063
Comments
More data showing the failure with floats but not with ints:
|
perhaps related to / the same as #11587? |
maybe the inliner is having trouble correctly uniquing the |
I tried |
|
cc @JeffBezanson for advice :) Also ref #11606 from the commit msg. |
How exactly do you run the tests. I've just run the LightGraphs tests with no errors (two deprecation warnings) In general, it's better if you could provide a full script to reproduce your error (rather than pieces of it). Would be even better if you can reduce you test cases. |
Pkg.test("LightGraphs") will do it. You'll see the error when it gets to The full test script is in
|
Yeah. No errors. |
@yuyichao On what version? |
julia> versioninfo()
Julia Version 0.4.0-dev+5841
Commit f428392* (2015-07-07 14:58 UTC)
Platform Info:
System: Linux (x86_64-unknown-linux-gnu)
CPU: Intel(R) Core(TM) i7-4702HQ CPU @ 2.20GHz
WORD_SIZE: 64
BLAS: libopenblas (DYNAMIC_ARCH NO_AFFINITY Haswell)
LAPACK: libopenblas
LIBM: libm
LLVM: libLLVM-3.6.1 |
It's definitely happening here (see pkg.julialang.org for confirmation, and I can confirm it as well):
Also here:
|
Just compiled the latest, same issue:
|
Appears to be an issue with sparse matrices? because
works. |
also cc @IainNZ since this originally came from pkg.julialang.org's test results :) |
I reproduced this with LightGraphs v0.1.15 on OSX build |
So somehow this is OSX only? |
Nope, it happened on the Ubuntu VM that PkgEval runs in (which is on an Arch host, as if that'd matter). So its definitely reproducible and platform independent (to an extent). |
One commonality between the PkgEval machine/env and my machine is that they are not source builds by me, they're coming from the magical buildbots (PkgEval is generic linux binaries, I'm using whatever homebrew gives out). |
Inspired by #12059 and @vtjnash 's #12063 (comment) can you try if |
running julia with |
@IainNZ my two julia instances are source builds. |
In the event this turns out to be a local code issue, the changes to the files in question around that date are here: sbromberger/LightGraphs.jl@a2eee8a |
@yuyichao could the difference in llvm versions be causing a problem here? |
I was hoping that it shouldn't but I can give it a try... (just didn't want to spend an hour compiling llvm if someone else could figure out the issue otherwise ;P ) |
@yuyichao if you wouldn't mind trying it and posting your results, I'd be grateful. Thanks. |
Hmm. Somehow I couldn't reproduce with Julia Version 0.4.0-dev+5860
Commit 7fa43ed* (2015-07-08 20:57 UTC) but I can now reproduce it with the latest master... Julia Version 0.4.0-dev+5923
Commit f5c46f8* (2015-07-11 04:03 UTC) |
Reduced the repro to type M{T}
graph
parities::AbstractArray{Bool,1}
colormap::Vector{Int}
bestweight::T
cutweight::T
visited::Integer
distmx::AbstractArray{T, 2}
vertices::Vector{Int}
end
function f(n, g, eweights)
parities = falses(n)
M(g,
parities,
zeros(Int, n),
typemax(Float64),
zero(Float64),
zero(Int),
eweights,
Int[])
end
n = 8
g = []
eweights = spzeros(n, n)
f(n, g, eweights) Independent of |
@yuyichao that fails for me in 0-day old master, but works in my 45-day old master (consistent with previous tests). |
Yeah. Just tries to get rid of @sbromberger Can you check if the version I said is working for me also works for you? #12063 (comment) Might help to bisect (if Jeff is not faster). Probably cc. @JeffBezanson . |
@yuyichao I know it fails with |
Just to be clear - I don't have to make clean or anything after I switch branches / revs, right? A "make" should do what I need? |
Shouldn't need to. |
|
PS: your assertion error is strange. Why is it asserting on LightGraphs when you're not using it? |
|
I'm not asserting on |
and your assertion error above:
(Oh, your test code is in maxadjvisit.jl, I guess. I've been copying and pasting into the REPL.) |
If you are talking about the path. Well, I'm just editting the tests inplace to reduce it... |
And what I pasted above is the content of my |
So, can we say with some degree of assurance that
I'm a bit lost at this point. How can I help? |
Also, see my bisect results above: #12063 (comment) |
|
Ahh, I forgot that you've already bisected. That's about the only think I can suggest... (you can probably check if my smaller script can reproduce the same result arround the bad commit although I'm not sure if we'll learn anything new...) I think @vtjnash and @JeffBezanson are also working on some type id issues that might be related to this so I'll just wait to see what they think. This is also enough debugging of typeinf for me today.... P.S. I have to say that you need to be quite unlucky to hit this. At least in the script I have above, even very subtle changes of the order or types makes the problem go away... |
Pinging here - what can I do to help fix this? It's causing breakages on LightGraphs for 0.4 only. |
@yuyichao Thanks for the reduced test. I managed to reduce it further to this:
If you so much as breathe on this it starts working, otherwise it fails. |
it looks like
|
fwiw, this was not an issue on v0.3:
the difference I see between these is the order in which they got inserted into the Union, which might help explain the bisect result (since that commit might have changed the order of uid assignment for these two types). |
Why does it get that julia> a = spzeros(0,0)
0x0 sparse matrix with 0 Float64 entries:
julia> isa(a, AbstractArray{Float64,2})
true |
Is this related at all to the changes that have affected inlining as well? (number of subscripts) |
Ok, I think this is fixed. Plz confirm whether the original LightGraphs.jl problem is fixed. |
@JeffBezanson THANK YOU - this appears to have fixed the failures in LightGraphs. Just out of interest - was it the commit I found via bisect that introduced the issue? (If so, it'd be the first time bisect's actually worked for me.) |
Well, the bisect result was correct, but that commit did not point to the root cause. @vtjnash 's comment nailed it: #12063 (comment) |
So I was preparing a backport branch for a probably-final 0.3.x release and accidentally brought a few extra tests in
edit: the above bisect might be garbage, if this happens intermittently... not sure |
My LightGraphs.jl code started failing tests around 6/30. Before that date, it was working. The problem is in this code:
with this test:
with this error:
Now, the strange part: if I change the outer constructor in one of two ways, the tests pass:
{T}
, it works{T}
parameterization in the outer constructor definition (and changeAbstractArray{T,2}
toAbstractArray
), it works.The first one is confusing because I don't know why I'd have to pass the type to the inner constructor.
The second one is particularly confusing since I reference
T
in the code of the outer constructor. Yet the tests pass.Even more confusing: if the
distmx
is a sparse array of Integers (instead of Floats as in the test), it works without any modification.In any case, more details are here: https://groups.google.com/d/msg/julia-users/uAntZ-eJ7kY/Heqq32Lhr3EJ
The text was updated successfully, but these errors were encountered: