From 9b179f11fe75d7f639897b0bbc0c685fe858add2 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Thu, 12 Dec 2024 17:59:54 +0000 Subject: [PATCH 1/7] Disable CI fail-fast --- .github/workflows/CI.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index bff53bf64..14fa5e058 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -21,6 +21,8 @@ jobs: test: runs-on: ${{ matrix.runner.os }} strategy: + fail-fast: false + matrix: runner: # Current stable version From fb2340a9c741578c497d1388c2d10054c7c60886 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Thu, 12 Dec 2024 18:01:41 +0000 Subject: [PATCH 2/7] Separate tests into two groups --- .github/workflows/CI.yml | 4 +++ test/runtests.jl | 56 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 14fa5e058..94e8d5ace 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -60,6 +60,9 @@ jobs: os: macos-latest arch: aarch64 num_threads: 2 + test_group: + - Group1 + - Group2 steps: - uses: actions/checkout@v4 @@ -75,6 +78,7 @@ jobs: - uses: julia-actions/julia-runtest@v1 env: + GROUP: ${{ matrix.test_group }} JULIA_NUM_THREADS: ${{ matrix.runner.num_threads }} - uses: julia-actions/julia-processcoverage@v1 diff --git a/test/runtests.jl b/test/runtests.jl index 38e6f87ca..cfe450b41 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -35,12 +35,13 @@ using OrderedCollections: OrderedSet using DynamicPPL: getargs_dottilde, getargs_tilde, Selector +const GROUP = get(ENV, "GROUP", "All") Random.seed!(100) include("test_util.jl") -@testset "DynamicPPL.jl" begin - @testset "interface" begin +@testset verbose = true "DynamicPPL.jl" begin + if GROUP == "All" || GROUP == "Group1" include("utils.jl") include("compiler.jl") include("varnamedvector.jl") @@ -50,15 +51,60 @@ include("test_util.jl") include("sampler.jl") include("independence.jl") include("distribution_wrappers.jl") - include("contexts.jl") - include("context_implementations.jl") include("logdensityfunction.jl") include("linking.jl") - include("threadsafe.jl") include("serialization.jl") include("pointwise_logdensities.jl") include("lkj.jl") + end + + if GROUP == "All" || GROUP == "Group2" + include("contexts.jl") + include("context_implementations.jl") + include("threadsafe.jl") include("debug_utils.jl") + @testset "compat" begin + include(joinpath("compat", "ad.jl")) + end + @testset "extensions" begin + include("ext/DynamicPPLMCMCChainsExt.jl") + include("ext/DynamicPPLJETExt.jl") + end + @testset "ad" begin + include("ext/DynamicPPLForwardDiffExt.jl") + include("ext/DynamicPPLMooncakeExt.jl") + include("ad.jl") + end + @testset "prob and logprob macro" begin + @test_throws ErrorException prob"..." + @test_throws ErrorException logprob"..." + end + @testset "doctests" begin + DocMeta.setdocmeta!( + DynamicPPL, + :DocTestSetup, + :(using DynamicPPL, Distributions); + recursive=true, + ) + doctestfilters = [ + # Older versions will show "0 element Array" instead of "Type[]". + r"(Any\[\]|0-element Array{.+,[0-9]+})", + # Older versions will show "Array{...,1}" instead of "Vector{...}". + r"(Array{.+,\s?1}|Vector{.+})", + # Older versions will show "Array{...,2}" instead of "Matrix{...}". + r"(Array{.+,\s?2}|Matrix{.+})", + # Errors from macros sometimes result in `LoadError: LoadError:` + # rather than `LoadError:`, depending on Julia version. + r"ERROR: (LoadError:\s)+", + # Older versions do not have `;;]` but instead just `]` at end of the line + # => need to treat `;;]` and `]` as the same, i.e. ignore them if at the end of a line + r"(;;){0,1}\]$"m, + # Ignore the source of a warning in the doctest output, since this is dependent on host. + # This is a line that starts with "└ @ " and ends with the line number. + r"└ @ .+:[0-9]+", + ] + doctest(DynamicPPL; manual=false, doctestfilters=doctestfilters) + end end @testset "compat" begin From 3a3a995e4df4177d4687269938518778cf583255 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Thu, 12 Dec 2024 14:42:46 +0000 Subject: [PATCH 3/7] Suppress unneeded output in test logs --- test/debug_utils.jl | 9 ++++++++- test/lkj.jl | 8 ++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/test/debug_utils.jl b/test/debug_utils.jl index dfa46affc..50bcb15d6 100644 --- a/test/debug_utils.jl +++ b/test/debug_utils.jl @@ -199,7 +199,14 @@ @test retype <: Tuple # Just make sure the following is runnable. - @test (DynamicPPL.DebugUtils.model_warntype(model); true) + # Also suppress the output since it's very long + @test begin + oldstd = stdout + redirect_stdout(devnull) + DynamicPPL.DebugUtils.model_warntype(model) + redirect_stdout(oldstd) + true + end end end end diff --git a/test/lkj.jl b/test/lkj.jl index b9c20f916..d581cd21b 100644 --- a/test/lkj.jl +++ b/test/lkj.jl @@ -22,14 +22,14 @@ _lkj_atol = 0.05 model = lkj_prior_demo() # `SampleFromPrior` will sample in constrained space. @testset "SampleFromPrior" begin - samples = sample(model, SampleFromPrior(), 1_000) + samples = sample(model, SampleFromPrior(), 1_000; progress=false) @test mean(map(Base.Fix2(getindex, Colon()), samples)) ≈ target_mean atol = _lkj_atol end # `SampleFromUniform` will sample in unconstrained space. @testset "SampleFromUniform" begin - samples = sample(model, SampleFromUniform(), 1_000) + samples = sample(model, SampleFromUniform(), 1_000; progress=false) @test mean(map(Base.Fix2(getindex, Colon()), samples)) ≈ target_mean atol = _lkj_atol end @@ -39,7 +39,7 @@ end model = lkj_chol_prior_demo(uplo) # `SampleFromPrior` will sample in unconstrained space. @testset "SampleFromPrior" begin - samples = sample(model, SampleFromPrior(), 1_000) + samples = sample(model, SampleFromPrior(), 1_000; progress=false) # Build correlation matrix from factor corr_matrices = map(samples) do s M = reshape(s.metadata.vals, (2, 2)) @@ -50,7 +50,7 @@ end # `SampleFromUniform` will sample in unconstrained space. @testset "SampleFromUniform" begin - samples = sample(model, SampleFromUniform(), 1_000) + samples = sample(model, SampleFromUniform(), 1_000; progress=false) # Build correlation matrix from factor corr_matrices = map(samples) do s M = reshape(s.metadata.vals, (2, 2)) From 233b4ba9d320d860fbaaec25326554a3e2692b41 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Thu, 12 Dec 2024 18:09:16 +0000 Subject: [PATCH 4/7] Cancel CI on subsequent pushes to same PR --- .github/workflows/CI.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 94e8d5ace..fce8d9e30 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -17,6 +17,11 @@ permissions: actions: write contents: read +# Cancel existing tests on the same PR if a new commit is added to a pull request +concurrency: + group: ${{ github.workflow }}-${{ github.ref || github.run_id }} + cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }} + jobs: test: runs-on: ${{ matrix.runner.os }} From 51741433bc5270b2a3205a47a1915cb8706015f4 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Fri, 13 Dec 2024 00:02:17 +0000 Subject: [PATCH 5/7] Surely we don't need the double loop --- test/sampler.jl | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/test/sampler.jl b/test/sampler.jl index 95e838167..e5fe6dc98 100644 --- a/test/sampler.jl +++ b/test/sampler.jl @@ -34,22 +34,20 @@ @testset "init" begin @testset "$(model.f)" for model in DynamicPPL.TestUtils.DEMO_MODELS - @testset "$(model.f)" for model in DynamicPPL.TestUtils.DEMO_MODELS - N = 1000 - chain_init = sample(model, SampleFromUniform(), N; progress=false) - - for vn in keys(first(chain_init)) - if AbstractPPL.subsumes(@varname(s), vn) - # `s ~ InverseGamma(2, 3)` and its unconstrained value will be sampled from Unif[-2,2]. - dist = InverseGamma(2, 3) - b = DynamicPPL.link_transform(dist) - @test mean(mean(b(vi[vn])) for vi in chain_init) ≈ 0 atol = 0.11 - elseif AbstractPPL.subsumes(@varname(m), vn) - # `m ~ Normal(0, sqrt(s))` and its constrained value is the same. - @test mean(mean(vi[vn]) for vi in chain_init) ≈ 0 atol = 0.11 - else - error("Unknown variable name: $vn") - end + N = 1000 + chain_init = sample(model, SampleFromUniform(), N; progress=false) + + for vn in keys(first(chain_init)) + if AbstractPPL.subsumes(@varname(s), vn) + # `s ~ InverseGamma(2, 3)` and its unconstrained value will be sampled from Unif[-2,2]. + dist = InverseGamma(2, 3) + b = DynamicPPL.link_transform(dist) + @test mean(mean(b(vi[vn])) for vi in chain_init) ≈ 0 atol = 0.11 + elseif AbstractPPL.subsumes(@varname(m), vn) + # `m ~ Normal(0, sqrt(s))` and its constrained value is the same. + @test mean(mean(vi[vn]) for vi in chain_init) ≈ 0 atol = 0.11 + else + error("Unknown variable name: $vn") end end end From d007022d95b882685fa74bed57f2d114e54c5b06 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Fri, 13 Dec 2024 12:22:08 +0000 Subject: [PATCH 6/7] Add comment about tests --- test/runtests.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index cfe450b41..d18f4c542 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -41,6 +41,9 @@ Random.seed!(100) include("test_util.jl") @testset verbose = true "DynamicPPL.jl" begin + # The tests are split into two groups so that CI can run in parallel. The + # groups are chosen to make both groups take roughly the same amount of + # time, but beyond that there is no particular reason for the split. if GROUP == "All" || GROUP == "Group1" include("utils.jl") include("compiler.jl") From 0c266a6b0e2426f97eb5b84411888d810c6fc4b0 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Fri, 13 Dec 2024 12:23:36 +0000 Subject: [PATCH 7/7] Don't suppress output from debug_utils test --- test/debug_utils.jl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/debug_utils.jl b/test/debug_utils.jl index 50bcb15d6..294364758 100644 --- a/test/debug_utils.jl +++ b/test/debug_utils.jl @@ -199,14 +199,7 @@ @test retype <: Tuple # Just make sure the following is runnable. - # Also suppress the output since it's very long - @test begin - oldstd = stdout - redirect_stdout(devnull) - DynamicPPL.DebugUtils.model_warntype(model) - redirect_stdout(oldstd) - true - end + @test DynamicPPL.DebugUtils.model_warntype(model) isa Any end end end