Support for Julia 1.11 #284
Conversation
|
Hmm a brittle test is failing consistently across OSs Ubuntu: https://github.com/Julia-Tempering/Pigeons.jl/actions/runs/11296182614/job/31420480756#step:8:1790 At least all these new values are consistent across platforms. Increasing rtol to 0.02 solves the issue so I'll do that. |
Interesting... Why would the ESS change when we change the Julia version?? |
|
Ok this one is kind of problematic: the new MemoryRef thing broke the parallelism invariance equality test for something inside the Replicas, not sure what exactly. https://github.com/Julia-Tempering/Pigeons.jl/actions/runs/11296182614/job/31420480756#step:8:4670 It actually may be just the fact that vectors are now memoryref |
My guess is that they changed the ESS methodology in MCMCChains slightly, which coincided with this new julia version. Or it could be a change in any of the packages required by that (FFT stuff... who knows). I guess that to rule out a package change instead of julia version, I could rerun the last test for main and see if that throws that error. |
|
What is a MemoryRef? |
|
So I'm trying to learn right now but I read it in the NEWS.md file. It actually sounds like a huge change to put in a minor release... |
|
Ah, thank you, I didn't know about that change! Seems pretty major, but should be hidden from users like us, no? I am not sure I understand how it would be break Parallelism Invariance, but I am very curious! |
|
It's just that we probably need to add a custom rule like all the others we have. Here's the error |
|
Ok I'm gonna stop the tests for now, rerun the one in main to rule out pkg changes for ESS numbers, and diagnose the memoryref issue locally |
|
Ah I see!! So some kind of change of the type hierarchy... agreed with above change |
|
Thank you Miguel!! |
|
Ok so the fields in Arrays are totally different. They now have a ref field of type MemoryRef that contains the data 1.11 julia> v=zeros(2)
2-element Vector{Float64}:
0.0
0.0
julia> fieldnames(typeof(v))
(:ref, :size)
julia> v.ref
MemoryRef{Float64}(Ptr{Nothing} @0x0000757f5d276ff0, [0.0, 0.0])1.10 julia> v=zeros(2)
2-element Vector{Float64}:
0.0
0.0
julia> fieldnames(typeof(v))
()
julia> v.ref
ERROR: type Array has no field ref
Stacktrace:
[1] getproperty(x::Vector{Float64}, f::Symbol)
@ Base ./Base.jl:37
[2] top-level scope
@ REPL[3]:1 |
|
So it seems that the equality check passes for arrays with isbit eltype ; e.g., returns true, whereas this fails throws the error. |
|
So it seems that the default |
|
Ok this explains the issue, because we do have a special rule for Vector{<:Replica} (and Vector{<:InterpolatedLogPotential} so that will fail too). Will remove these and look for a less heavy handed approach. |
|
BTW, the relaunched tests on main pass the ESS check (except for 2 that are actually running 1.11). The versions of external packages launched are exactly the same. Definitely the culprit is in stdlib, most likely LinearAlgebra since the default ESS method in MCMCChains is a straightforward autocovariance estimator |
|
Ugh another numerical accuracy. This time not via ESS but log normalization constant https://github.com/Julia-Tempering/Pigeons.jl/actions/runs/11297944298/job/31425868094#step:8:7690 Edit: Good news is that it is not the "truth" value that changed (that would've been catastrophic). They match exactly in 1.10 and 1.11. |
|
@alexandrebouchard it seems that all issues have been fixed. Are you ok with keeping the increased tolerances for those 2 tests? |
|
Thanks a lot Miguel. Quick question what is "Julia 1" in the CI? Maybe a more descriptive name would be better. Not opposed to merging per se, but isn't it strange that numerical accuracy of the log normalization constant changes when doing a minor version increment though? |
It tells CI to use the latest released julia major version 1. Now that means 1.11.0. Last week it was 1.10.5.
100%. But I have no idea what we could do in this case. Thoughts? |
|
One thing that changed is the MersenneTwister implementation: from a fresh session 1.11 julia> using Random
julia> rng = MersenneTwister(321)
MersenneTwister(321)
julia> x = rand(rng, 2)
2-element Vector{Float64}:
0.4643060308852154
0.50861050362250461.10 julia> using Random
julia> rng = MersenneTwister(321)
MersenneTwister(321)
julia> x = rand(rng, 2)
2-element Vector{Float64}:
0.6306801637635273
0.9123961296861567We do use this RNG in the tests, but in the particular ones that failed. |
|
AFAICT, the only thing in common that the two failed tests have is that they use AutoMALA with ForwardDiff as backend. And weirdly, there is an open issue in ForwardDiff about a weird interaction with RNGs, although I don't think it is pertinent to our situation. |
|
Double checking that it is the RNG implementation and not the rand*() functions that changed Code: using Random, SplittableRandoms
all_comb = Iterators.product(
(rand, randn, randexp),
(MersenneTwister(1), Xoshiro(1), SplittableRandom(1))
)
for (rfun, rng) in all_comb
println("$rfun\t$(nameof(typeof(rng)))\t$(rand(rng))")
end1.11 1.10 |
|
@alexandrebouchard the failed tests in main are due to the slight change in results under Julia 1.11, not due to DynamicPPL version bump. |
|
The current failures are due to an issue with the resolution of the environment |
No description provided.