-
Notifications
You must be signed in to change notification settings - Fork 230
Test on 1.12 #2707
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
base: main
Are you sure you want to change the base?
Test on 1.12 #2707
Conversation
|
|
||
| # Skip Mooncake on 1.12 as it is not compatible yet | ||
| const INCLUDE_MOONCAKE = VERSION >= v"1.12" | ||
| const INCLUDE_MOONCAKE = VERSION < v"1.12" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silly mistake I made previously.
|
Turing.jl documentation for PR #2707 is available at: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2707 +/- ##
=======================================
Coverage 86.45% 86.45%
=======================================
Files 21 21
Lines 1418 1418
=======================================
Hits 1226 1226
Misses 192 192 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@mhauru Do you know if the test failure is expected? |
|
Nope, haven't seen that before. I hope this isn't indeterministic to reproduce. Looks like MCMCThreads gets Libtask tasks running on multiple threads, and two happened to write to the global MistyClosures cache within Libtask at the same time. Maybe need to do some locks or |
|
Wanna try to make a little MWE by creating tasks on multiple threads, or shall I? My (sad) prediction is that the CI failure will go away if you rerun it and get a bit lucky. |
|
I'm struggling to make a Libtask-only MWE, this doesn't work: using Libtask
function task(i)
# define a new method, ensuring that Libtask has to construct
# a new MistyClosure and add it to the cache
function f(::Val{j}) where {j}
produce(j)
return nothing
end
# construct a TapedTask for it
return TapedTask(nothing, f, Val(i))
end
fetch.([Threads.@spawn task(i) for i in 1:200])This is doing concurrent writes to In fact even this doesn't trigger it: d = Dict{Int,Int}()
empty!(d)
function seti(i)
d[i] = i
end
fetch.([Threads.@spawn seti(i) for i in 1:1000000])Will continue trying. |
|
Patching that |
|
So the real underlying problem (concurrent writes to Dicts) can occur on both 1.11 and 1.12, which of course leads to the question of why does this error on 1.12 but not 1.11? I assume that there's some performance regression on |
|
New Libtask version should fix this, rerunning the failing CI. |
|
Gibbs on Windows takes a full hour, meh. |
Now that Libtask works on 1.12, I think we can and should run CI on it.
Mooncake can be disabled for the time being (it will still be tested in on 1.10).