From a0a3c326414d06e459317c8e93bb13af6e6f482b Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 3 Dec 2024 17:56:14 +0100 Subject: [PATCH 1/4] introduce extended tests --- .github/workflows/julianightly.yml | 3 +- test/runtests.jl | 2 + test/spectral_transform_ad_rules.jl | 213 ++++++++++++++-------------- 3 files changed, 112 insertions(+), 106 deletions(-) diff --git a/.github/workflows/julianightly.yml b/.github/workflows/julianightly.yml index 5e4d14ca0..bd7e882b6 100644 --- a/.github/workflows/julianightly.yml +++ b/.github/workflows/julianightly.yml @@ -1,5 +1,5 @@ name: JuliaNightly -# Nightly Scheduled Julia Nightly Run +# Nightly Scheduled Julia Nightly Run with extended tests on: schedule: - cron: '0 2 * * 0' # Daily at 2 AM UTC every Sunday @@ -28,3 +28,4 @@ jobs: uses: julia-actions/julia-runtest@v1 with: coverage: false + test_args: 'extended_tests' diff --git a/test/runtests.jl b/test/runtests.jl index aa845cbff..547e4302b 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,6 +1,8 @@ using SpeedyWeather using Test +FLAG_EXTENDED_TESTS = "extended_tests" in ARGS ? true : false + # GENERAL include("utility_functions.jl") include("dates.jl") diff --git a/test/spectral_transform_ad_rules.jl b/test/spectral_transform_ad_rules.jl index be26cbcca..4d94f32ac 100644 --- a/test/spectral_transform_ad_rules.jl +++ b/test/spectral_transform_ad_rules.jl @@ -7,7 +7,7 @@ import AbstractFFTs grid_types = [FullGaussianGrid, OctahedralGaussianGrid] # one full and one reduced grid, both Gaussian to have exact transforms grid_dealiasing = [2, 3] -fd_tests = [true, false] # to save CI time, only do FiniteDifferences test for one of the grids +fd_tests = [true, true] # currenlty there's an issue with EnzymeTestUtils not being able to work with structs with undefined fields like FFT plans # https://github.com/EnzymeAD/Enzyme.jl/issues/1992 @@ -56,131 +56,134 @@ end end end - @testset "Complete Transform Enzyme" begin - # make a high level finite difference test of the whole transform - # can't use Enzyme or ChainRule Test tools for tests for that - for (i_grid, grid_type) in enumerate(grid_types) + if FLAG_EXTENDED_TESTS # part of the extented tests, not tested in regular CI - spectral_grid = SpectralGrid(Grid=grid_type, trunc=10, nlayers=1, dealiasing=grid_dealiasing[i_grid]) - S = SpectralTransform(spectral_grid, one_more_degree=true) - dS = deepcopy(S) + @testset "Complete Transform Enzyme" begin + # make a high level finite difference test of the whole transform + # can't use Enzyme or ChainRule Test tools for tests for that + for (i_grid, grid_type) in enumerate(grid_types) - if fd_tests[i_grid] - - # forwards - grid = rand(spectral_grid.Grid{spectral_grid.NF}, spectral_grid.nlat_half, spectral_grid.nlayers) - dgrid = zero(grid) - specs = zeros(LowerTriangularArray{Complex{spectral_grid.NF}}, spectral_grid.trunc+2, spectral_grid.trunc+1, spectral_grid.nlayers) - - # seed - dspecs = zero(specs) - fill!(dspecs, 1+1im) + spectral_grid = SpectralGrid(Grid=grid_type, trunc=10, nlayers=1, dealiasing=grid_dealiasing[i_grid]) + S = SpectralTransform(spectral_grid, one_more_degree=true) + dS = deepcopy(S) - autodiff(Reverse, transform!, Const, Duplicated(specs, dspecs), Duplicated(grid, dgrid), Duplicated(S, dS)) + if fd_tests[i_grid] + + # forwards + grid = rand(spectral_grid.Grid{spectral_grid.NF}, spectral_grid.nlat_half, spectral_grid.nlayers) + dgrid = zero(grid) + specs = zeros(LowerTriangularArray{Complex{spectral_grid.NF}}, spectral_grid.trunc+2, spectral_grid.trunc+1, spectral_grid.nlayers) + + # seed + dspecs = zero(specs) + fill!(dspecs, 1+1im) - # new seed - dspecs2 = zero(specs) - fill!(dspecs2, 1+1im) + autodiff(Reverse, transform!, Const, Duplicated(specs, dspecs), Duplicated(grid, dgrid), Duplicated(S, dS)) - # finite difference comparision, seeded with a one adjoint to get the direct gradient - fd_jvp = FiniteDifferences.j′vp(central_fdm(5,1), x -> transform(x, S), dspecs2, grid) - @test isapprox(dgrid, fd_jvp[1]) + # new seed + dspecs2 = zero(specs) + fill!(dspecs2, 1+1im) - ## now backwards, as the input for spec we use the output of the forward transform + # finite difference comparision, seeded with a one adjoint to get the direct gradient + fd_jvp = FiniteDifferences.j′vp(central_fdm(5,1), x -> transform(x, S), dspecs2, grid) + @test isapprox(dgrid, fd_jvp[1]) - fill!(dspecs,0) - grid = zeros(spectral_grid.Grid{spectral_grid.NF}, spectral_grid.nlat_half, spectral_grid.nlayers) - dgrid = similar(grid) - fill!(dgrid, 1) + ## now backwards, as the input for spec we use the output of the forward transform - autodiff(Reverse, transform!, Const, Duplicated(grid, dgrid), Duplicated(specs, dspecs), Duplicated(S, dS)) + fill!(dspecs,0) + grid = zeros(spectral_grid.Grid{spectral_grid.NF}, spectral_grid.nlat_half, spectral_grid.nlayers) + dgrid = similar(grid) + fill!(dgrid, 1) - # new seed - dgrid2 = similar(grid) - fill!(dgrid2, 1) + autodiff(Reverse, transform!, Const, Duplicated(grid, dgrid), Duplicated(specs, dspecs), Duplicated(S, dS)) - fd_jvp = FiniteDifferences.j′vp(central_fdm(5,1), x -> transform(x, S), dgrid2, specs) + # new seed + dgrid2 = similar(grid) + fill!(dgrid2, 1) - @test isapprox(dspecs, fd_jvp[1]) - end + fd_jvp = FiniteDifferences.j′vp(central_fdm(5,1), x -> transform(x, S), dgrid2, specs) - # test that d S^{-1}(S(x)) / dx = dx/dx = 1 (starting in both domains) - # this only holds for exact transforms, like Gaussian grids + @test isapprox(dspecs, fd_jvp[1]) + end - # start with grid (but with a truncated one) - function transform_identity!(x_out::AbstractGridArray{T}, x::AbstractGridArray{T}, S::SpectralTransform{T}) where T - x_SH = zeros(LowerTriangularArray{Complex{T}}, S.lmax+1, S.mmax+1, S.nlayers) - transform!(x_SH, x, S) - transform!(x_out, x_SH, S) - return nothing - end + # test that d S^{-1}(S(x)) / dx = dx/dx = 1 (starting in both domains) + # this only holds for exact transforms, like Gaussian grids - function transform_identity(x::AbstractGridArray{T}, S::SpectralTransform{T}) where T - x_copy = deepcopy(x) - transform_identity!(x_copy, x, S) - return x_copy - end - - grid = rand(S.Grid{spectral_grid.NF}, S.nlat_half, S.nlayers) - spec = transform(grid, S) - - grid = transform(spec, S) - grid_out = zero(grid) - - transform_identity!(grid_out, grid, S) - @test isapprox(grid, grid_out) - - dgrid = similar(grid) - fill!(dgrid, 1) - - dgrid_out = zero(grid_out) - - autodiff(Reverse, transform_identity!, Const, Duplicated(grid_out, dgrid_out), Duplicated(grid, dgrid), Duplicated(S, dS)) - - @test all(isapprox.(dgrid, 1)) - # TODO: previously this test was broken, with a version that directly mutates in-place. - # FD yields the same non-one values though. - # Not sure why. Do we use such things in our model? - # - #function transform_identity!(x::AbstractGridArray{T}, S::SpectralTransform{T}) where T - # x_SH = zeros(LowerTriangularArray{Complex{T}}, S.lmax+1, S.mmax+1, S.nlayers) - # transform!(x_SH, x, S) - # transform!(x, x_SH, S) - # return nothing - #end - # The FD comparision passes, but it takes a long time to compute, so it's commented out. - #dgrid2 = similar(grid) - #fill!(dgrid2, 1) - #fd_jvp = FiniteDifferences.j′vp(central_fdm(5,1), x -> transform_identity(x, S), dgrid2, grid) - #@test isapprox(dgrid, fd_jvp[1], rtol=0.01) - - # now start with spectral space, exclude for other grid because of https://github.com/SpeedyWeather/SpeedyWeather.jl/issues/626 - if fd_tests[i_grid] - - function transform_identity!(x::LowerTriangularArray{Complex{T}}, S::SpectralTransform{T}) where T - x_grid = zeros(S.Grid{T}, S.nlat_half, S.nlayers) - transform!(x_grid, x, S) - transform!(x, x_grid, S) + # start with grid (but with a truncated one) + function transform_identity!(x_out::AbstractGridArray{T}, x::AbstractGridArray{T}, S::SpectralTransform{T}) where T + x_SH = zeros(LowerTriangularArray{Complex{T}}, S.lmax+1, S.mmax+1, S.nlayers) + transform!(x_SH, x, S) + transform!(x_out, x_SH, S) return nothing end + function transform_identity(x::AbstractGridArray{T}, S::SpectralTransform{T}) where T + x_copy = deepcopy(x) + transform_identity!(x_copy, x, S) + return x_copy + end + + grid = rand(S.Grid{spectral_grid.NF}, S.nlat_half, S.nlayers) spec = transform(grid, S) - spec_copy = deepcopy(spec) - transform_identity!(spec, S) - @test isapprox(spec, spec_copy) - - dspec = similar(spec) - fill!(dspec, 1+im) - autodiff(Reverse, transform_identity!, Const, Duplicated(spec, dspec), Duplicated(S, dS)) + grid = transform(spec, S) + grid_out = zero(grid) + + transform_identity!(grid_out, grid, S) + @test isapprox(grid, grid_out) - @test all(all.([isapprox.(dspec[il,1,:], 1) for il in 1:S.lmax+1])) # m = 0 => Im = 0 + dgrid = similar(grid) + fill!(dgrid, 1) - for i in eachmatrix(dspec) - @test all(isapprox.(dspec[:,i][S.lmax+2:end], 1+im)) - end - end - end + dgrid_out = zero(grid_out) + + autodiff(Reverse, transform_identity!, Const, Duplicated(grid_out, dgrid_out), Duplicated(grid, dgrid), Duplicated(S, dS)) + + @test all(isapprox.(dgrid, 1)) + # TODO: previously this test was broken, with a version that directly mutates in-place. + # FD yields the same non-one values though. + # Not sure why. Do we use such things in our model? + # + #function transform_identity!(x::AbstractGridArray{T}, S::SpectralTransform{T}) where T + # x_SH = zeros(LowerTriangularArray{Complex{T}}, S.lmax+1, S.mmax+1, S.nlayers) + # transform!(x_SH, x, S) + # transform!(x, x_SH, S) + # return nothing + #end + # The FD comparision passes, but it takes a long time to compute, so it's commented out. + #dgrid2 = similar(grid) + #fill!(dgrid2, 1) + #fd_jvp = FiniteDifferences.j′vp(central_fdm(5,1), x -> transform_identity(x, S), dgrid2, grid) + #@test isapprox(dgrid, fd_jvp[1], rtol=0.01) + + # now start with spectral space, exclude for other grid because of https://github.com/SpeedyWeather/SpeedyWeather.jl/issues/626 + if fd_tests[i_grid] + + function transform_identity!(x::LowerTriangularArray{Complex{T}}, S::SpectralTransform{T}) where T + x_grid = zeros(S.Grid{T}, S.nlat_half, S.nlayers) + transform!(x_grid, x, S) + transform!(x, x_grid, S) + return nothing + end + + spec = transform(grid, S) + spec_copy = deepcopy(spec) + transform_identity!(spec, S) + @test isapprox(spec, spec_copy) + + dspec = similar(spec) + fill!(dspec, 1+im) + + autodiff(Reverse, transform_identity!, Const, Duplicated(spec, dspec), Duplicated(S, dS)) + + @test all(all.([isapprox.(dspec[il,1,:], 1) for il in 1:S.lmax+1])) # m = 0 => Im = 0 + + for i in eachmatrix(dspec) + @test all(isapprox.(dspec[:,i][S.lmax+2:end], 1+im)) + end + end + end + end end @testset "Complete Transform ChainRules" begin From 8b2232dda10c52488fc3630607af51eef65c31f4 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 3 Dec 2024 17:59:38 +0100 Subject: [PATCH 2/4] extended tests --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5aabc0cd8..99032ec9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Introduced seperate extended tests that are not run on every commit [#628](https://github.com/SpeedyWeather/SpeedyWeather.jl/pull/628) - One-band longwave radiation [#624](https://github.com/SpeedyWeather/SpeedyWeather.jl/pull/624) - compat entry for FiniteDifferences.jl [#620](https://github.com/SpeedyWeather/SpeedyWeather.jl/pull/620) - Slab ocean and net surface fluxes [#613](https://github.com/SpeedyWeather/SpeedyWeather.jl/pull/613) From 10e25a6381bd66836c911446bf02866ec27b1597 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 3 Dec 2024 18:34:43 +0100 Subject: [PATCH 3/4] print info message when running extended tests --- test/runtests.jl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 547e4302b..6b2bec67d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -3,6 +3,10 @@ using Test FLAG_EXTENDED_TESTS = "extended_tests" in ARGS ? true : false +if FLAG_EXTENDED_TESTS + @info "Running extended test suite" +end + # GENERAL include("utility_functions.jl") include("dates.jl") From f952ad5fb694abed80b01c0ae2e6bb5ff1eae592 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 3 Dec 2024 19:34:33 +0100 Subject: [PATCH 4/4] workflow for extended tests --- .github/workflows/extended_tests.yml | 29 ++++++++++++++++++++++++++++ .github/workflows/julianightly.yml | 3 +-- 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/extended_tests.yml diff --git a/.github/workflows/extended_tests.yml b/.github/workflows/extended_tests.yml new file mode 100644 index 000000000..d7cb5b01d --- /dev/null +++ b/.github/workflows/extended_tests.yml @@ -0,0 +1,29 @@ +name: Extended Test +# Weekly extended tests +on: + schedule: + - cron: '0 2 * * 0' # Daily at 2 AM UTC every Sunday +jobs: + test: + runs-on: ${{ matrix.os }} + strategy: + matrix: + version: + - '1.11' + os: + - ubuntu-latest + arch: + - x64 + steps: + - uses: actions/checkout@v4 + - uses: julia-actions/setup-julia@v2 + with: + version: ${{ matrix.version }} + arch: ${{ matrix.arch }} + - uses: julia-actions/cache@v2 + - uses: julia-actions/julia-buildpkg@v1 + - uses: julia-actions/julia-runtest@v1 + with: + test_args: 'extended_tests' + env: + GITHUB_AUTH: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/julianightly.yml b/.github/workflows/julianightly.yml index bd7e882b6..5e4d14ca0 100644 --- a/.github/workflows/julianightly.yml +++ b/.github/workflows/julianightly.yml @@ -1,5 +1,5 @@ name: JuliaNightly -# Nightly Scheduled Julia Nightly Run with extended tests +# Nightly Scheduled Julia Nightly Run on: schedule: - cron: '0 2 * * 0' # Daily at 2 AM UTC every Sunday @@ -28,4 +28,3 @@ jobs: uses: julia-actions/julia-runtest@v1 with: coverage: false - test_args: 'extended_tests'