From 3077a5efa1e9672b5e6e4d805893f45ad85546da Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Fri, 13 Dec 2024 17:31:25 +0000 Subject: [PATCH] Make CI + tests more efficient (#749) * Disable CI fail-fast * Separate tests into two groups * Suppress unneeded output in test logs * Cancel CI on subsequent pushes to same PR * Surely we don't need the double loop * Add comment about tests * Don't suppress output from debug_utils test --- .github/workflows/CI.yml | 11 ++++++++ test/debug_utils.jl | 2 +- test/lkj.jl | 8 +++--- test/runtests.jl | 59 ++++++++++++++++++++++++++++++++++++---- test/sampler.jl | 30 ++++++++++---------- 5 files changed, 84 insertions(+), 26 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index bff53bf64..fce8d9e30 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -17,10 +17,17 @@ 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 }} strategy: + fail-fast: false + matrix: runner: # Current stable version @@ -58,6 +65,9 @@ jobs: os: macos-latest arch: aarch64 num_threads: 2 + test_group: + - Group1 + - Group2 steps: - uses: actions/checkout@v4 @@ -73,6 +83,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/debug_utils.jl b/test/debug_utils.jl index dfa46affc..294364758 100644 --- a/test/debug_utils.jl +++ b/test/debug_utils.jl @@ -199,7 +199,7 @@ @test retype <: Tuple # Just make sure the following is runnable. - @test (DynamicPPL.DebugUtils.model_warntype(model); true) + @test DynamicPPL.DebugUtils.model_warntype(model) isa Any 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)) diff --git a/test/runtests.jl b/test/runtests.jl index 38e6f87ca..d18f4c542 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -35,12 +35,16 @@ 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 + # 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") include("varnamedvector.jl") @@ -50,15 +54,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 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