CI Integration Example SpeedyWeather.jl

[maximilian-gelbrecht]

@vchuravy suggested I add an example using SpeedyWeather.jl to the integration tests.

The test is pretty much the same thing we do in the paper. It's a sensitivity analysis of a single grid point and it checks that this runs without errors and the gradient makes physical sense, so the gradient is localised around the selected grid point and small far away from it.

[github-actions]

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic main) to apply these changes.

Click here to view the suggested changes.

diff --git a/test/integration/SpeedyWeather/runtests.jl b/test/integration/SpeedyWeather/runtests.jl
index ea3e354c..c606e9fd 100644
--- a/test/integration/SpeedyWeather/runtests.jl
+++ b/test/integration/SpeedyWeather/runtests.jl
@@ -6,7 +6,7 @@
 using SpeedyWeather, Enzyme, Test
 
 spectral_grid = SpectralGrid(trunc = 32, nlayers = 8)             # define resolution
-model = PrimitiveWetModel(; spectral_grid, physics=false)         # construct model
+model = PrimitiveWetModel(; spectral_grid, physics = false)         # construct model
 # physics = false to accelate the test
 simulation = initialize!(model)
 initialize!(simulation)

[vchuravy]

Thanks Max!

[vchuravy]

@giordano can we make the filtering more aggressive and only run CI on the integration test being changed?

[giordano]

We could generate the matrix dynamically, but only if test/integration/ files only have been modified, otherwise I think we want to run all the tests, right?

[giordano]

BTW, this is missing adding SpeedyWeather to

Enzyme.jl/.github/workflows/Integration.yml

Lines 47 to 56 in ab96c82

	package:
	- Bijectors
	- DifferentiationInterface
	- Distributions
	- DynamicExpressions
	- Lux
	- SciML
	- KernelAbstractions
	- Molly
	- MPI

[maximilian-gelbrecht]

Oh, yes. I just added it.

[giordano]

Besides the fact Test isn't loaded, the jobs are timing out. Any way to have something lighter to test? All other jobs finish in less than 20 minutes (most of them are a lot faster)

[maximilian-gelbrecht]

Yeah, in our own CI a very similar test takes about 35 minutes. I hoped it's a bit faster now after recent Enzyme patches, but it doesn't seem to be the case.

If 20 min is the limit (didn't know that), then I can turn off some of the parametrizations and see if that's sufficient. I am out of time to really experiment with that for today though.

[giordano]

Timeout is 45 minutes

Enzyme.jl/.github/workflows/Integration.yml

Line 32 in ab96c82

timeout-minutes: 45

but all other jobs take less than half of that.

[maximilian-gelbrecht]

I mean the gradient compile time is what it is currently 🤷
On my laptop and my HPC it's a bit faster, but in the GitHub Actions CI it's really quite slow.

[maximilian-gelbrecht]

I updated the example to mirror more closely the one we already use in your CI in Speedy. It should be faster now, and there it always finishes < 40 min.

However, it also seems that Enzymev0.13.105 broke something for us, as we are getting fails from our CI tests on this since Friday. So this example here will also currently fail.

[wsmoses]

well thats certainly a reason for getting the integration tests in!

[wsmoses]

that said the erring part is a bit too complex/baked in, would you be able to construct a MWE of?

[codecov]

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.78%. Comparing base (30e0519) to head (0b438af).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/rules/customrules.jl	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2784      +/-   ##
==========================================
- Coverage   67.79%   67.78%   -0.01%     
==========================================
  Files          58       58              
  Lines       20723    20726       +3     
==========================================
  Hits        14050    14050              
- Misses       6673     6676       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wsmoses]

okay I think I fixed the custom rule issue immediately here (and a potential PR for more information on the 1.11: EnzymeAD/Enzyme#2582).

However, on a debug julia I pretty much always hit GC errors. @vchuravy I think we really ought debug and fix that MWE speedweather GC error issue before we can land this

[maximilian-gelbrecht]

Thanks for your effort on this already. I already had a debugging session today with this, but it's quite nasty to boil this down to a proper MWE. I know which intermediate function from our code causes the error, but each individual function it calls is fine. This is the function for our simplest model, and here's an example call with Enzyme, but in the next days I'll try to condense it to a proper MWE, it's just not that easy to boil this down so quickly.

[maximilian-gelbrecht]

Is #2818 related to this here? Just let me know if I should invest a bit more time for finding a really proper MWE, or if that's already it.

[wsmoses]

No that's unrelated and an issue from an in progress PR which doesn't occur on main atm

[maximilian-gelbrecht]

The whole issue just confuses me.

Now, I found that just leaving away the kwargs... in the function signature and the @boundscheck in the function makes it work. The individual functions are all fine to differentiate as well, and the custom rules that are used for the _fourier! function also pass their tests and are fine to differentiate in other contexts.

So in the following script, using some of SpeedyWeather's numerics I just present the just slightly different versions where the last one that includes an unused kwargs... and a @boundscheck causes a SegFault.

Julia 1.10, Enzyme v0.13.108, SpeedyWeather main

using Enzyme, SpeedyWeather
 
# this works 
function transform_debug!(
    vor_grid, u_grid, scratch_memory, U, 
    vor, 
    S
)    
    # catch incorrect sizes early
    @boundscheck SpeedyWeather.ismatching(S, vor_grid) || throw(DimensionMismatch(S, vor_grid))
    @boundscheck SpeedyWeather.ismatching(S, vor) || throw(DimensionMismatch(S, vor))

    g_north = scratch_memory.north 
    g_south = scratch_memory.south 

    # this is a FFT, for which we have a custom rule defined 
    SpeedyWeather.SpeedyTransforms._fourier!(vor_grid, g_north, g_south, S)

    # catch incorrect sizes early
    @boundscheck SpeedyWeather.ismatching(S, u_grid) || throw(DimensionMismatch(S, u_grid))
    @boundscheck SpeedyWeather.ismatching(S, U) || throw(DimensionMismatch(S, U))

    g_north = scratch_memory.north 
    g_south = scratch_memory.south 

    SpeedyWeather.SpeedyTransforms._fourier!(u_grid, g_north, g_south, S)

    return nothing
    
end

# this works 
function transform_debug_with_kwargs_no_checks!(
    vor_grid, u_grid, scratch_memory, U, 
    vor, 
    S; 
    kwargs... # now has an unused kwargs...
)   
    # but no @boundschecks 
    g_north = scratch_memory.north 
    g_south = scratch_memory.south 

    # this is a FFT, for which we have a custom rule defined 
    SpeedyWeather.SpeedyTransforms._fourier!(vor_grid, g_north, g_south, S)

    g_north = scratch_memory.north 
    g_south = scratch_memory.south 

    SpeedyWeather.SpeedyTransforms._fourier!(u_grid, g_north, g_south, S)

    return nothing
end

# this gives a segfault 
function transform_debug_with_kwargs!(
    vor_grid, u_grid, scratch_memory, U, 
    vor, 
    S; 
    kwargs... # still has an unused kwargs...
)   
    # boundschecks and kwarg together cause an error here now 
    @boundscheck SpeedyWeather.ismatching(S, vor_grid) || throw(DimensionMismatch(S, vor_grid))
    @boundscheck SpeedyWeather.ismatching(S, vor) || throw(DimensionMismatch(S, vor))

    # that's  a spherical harmonics transforms (KA kernel + FFT)
    g_north = scratch_memory.north 
    g_south = scratch_memory.south 

    # this is a FFT for which we have a custom rule defined
    SpeedyWeather.SpeedyTransforms._fourier!(vor_grid, g_north, g_south, S)

    @boundscheck SpeedyWeather.ismatching(S, u_grid) || throw(DimensionMismatch(S, u_grid))
    @boundscheck SpeedyWeather.ismatching(S, U) || throw(DimensionMismatch(S, U))

    g_north = scratch_memory.north 
    g_south = scratch_memory.south 

    SpeedyWeather.SpeedyTransforms._fourier!(u_grid, g_north, g_south, S)

    return nothing
end

spectral_grid = SpectralGrid(trunc=9, nlayers=1)
S = SpectralTransform(spectral_grid)
#simulation = initialize!(model)

# these are RingGrids.Fields 
vor_grid = rand(spectral_grid.grid, 1)
u_grid = rand(spectral_grid.grid, 1)
v_grid = rand(spectral_grid.grid, 1)
scratch_memory = deepcopy(S.scratch_memory)

# these are LowerTriangularArrays.LowerTriangularArray
vor = rand(ComplexF32, spectral_grid.spectrum, 1)  
U =  rand(ComplexF32, spectral_grid.spectrum, 1)  

# this works 
autodiff(Reverse, transform_debug!, Const, Duplicated(vor_grid, make_zero(vor_grid)), Duplicated(u_grid, make_zero(u_grid)), Duplicated(scratch_memory, make_zero(scratch_memory)), Duplicated(U, make_zero(U)), Duplicated(vor, make_zero(vor)), Const(S))

# this works as well 
autodiff(Reverse, transform_debug_with_kwargs_no_checks!, Const, Duplicated(vor_grid, make_zero(vor_grid)), Duplicated(u_grid, make_zero(u_grid)), Duplicated(scratch_memory, make_zero(scratch_memory)), Duplicated(U, make_zero(U)), Duplicated(vor, make_zero(vor)), Const(S))

# this gives a SegFault
autodiff(Reverse, transform_debug_with_kwargs!, Const, Duplicated(vor_grid, make_zero(vor_grid)), Duplicated(u_grid, make_zero(u_grid)), Duplicated(scratch_memory, make_zero(scratch_memory)), Duplicated(U, make_zero(U)), Duplicated(vor, make_zero(vor)), Const(S))

[maximilian-gelbrecht]

I've updated the example above and I am honestly just really confused by it. This was fine before, but now seemingly some interaction between our custom rules for the FFT, a @boundscheck and an unused kwargs... causes a SegFault, but also just in a configuration like that, with calling _fourier! twice. In other contexts all of this is fine.

The custom rule is actually implemented here