ClimaCoupler downstream test is failing

[Sbozzolo]

https://github.com/CliMA/ClimaAtmos.jl/actions/runs/9753198215

ERROR: LoadError: type NamedTuple has no field idealized_insolation
Stacktrace:
  [1] getproperty
    @ ./Base.jl:37 [inlined]
  [2] set_surface_albedo!
    @ ~/work/ClimaAtmos.jl/ClimaAtmos.jl/ClimaCoupler.jl/experiments/ClimaEarth/components/atmosphere/climaatmos.jl:86 [inlined]
  [3] macro expansion
    @ ~/work/ClimaAtmos.jl/ClimaAtmos.jl/src/callbacks/callbacks.jl:173 [inlined]
  [4] rrtmgp_model_callback!(integrator::ClimaTimeSteppers.DistributedODEIntegrator{…})
    @ ClimaAtmos ~/.julia/packages/NVTX/pfSOQ/src/macro.jl:194
  [5] AtmosCallback
    @ ~/work/ClimaAtmos.jl/ClimaAtmos.jl/src/solver/types.jl:461 [inlined]
  [6] #237
    @ ~/work/ClimaAtmos.jl/ClimaAtmos.jl/src/callbacks/callback_helpers.jl:49 [inlined]
  [7] initialize!(u::ClimaCore.Fields.FieldVector{…}, t::Float64, integrator::ClimaTimeSteppers.DistributedODEIntegrator{…}, any_modified::Bool, c::SciMLBase.DiscreteCallback{…}, cs::SciMLBase.DiscreteCallback{…})
    @ DiffEqBase ~/.julia/packages/DiffEqBase/c8MAQ/src/callbacks.jl:13
  [8] initialize!(::ClimaCore.Fields.FieldVector{…}, ::Float64, ::ClimaTimeSteppers.DistributedODEIntegrator{…}, ::Bool, ::SciMLBase.DiscreteCallback{…}, ::SciMLBase.DiscreteCallback{…}, ::Vararg{…})
    @ DiffEqBase ~/.julia/packages/DiffEqBase/c8MAQ/src/callbacks.jl:14
  [9] initialize!(::ClimaCore.Fields.FieldVector{…}, ::Float64, ::ClimaTimeSteppers.DistributedODEIntegrator{…}, ::Bool, ::SciMLBase.DiscreteCallback{…}, ::SciMLBase.DiscreteCallback{…}, ::Vararg{…})
    @ DiffEqBase ~/.julia/packages/DiffEqBase/c8MAQ/src/callbacks.jl:14
 [10] initialize!(::ClimaCore.Fields.FieldVector{…}, ::Float64, ::ClimaTimeSteppers.DistributedODEIntegrator{…}, ::Bool, ::SciMLBase.DiscreteCallback{…}, ::SciMLBase.DiscreteCallback{…}, ::Vararg{…})
    @ DiffEqBase ~/.julia/packages/DiffEqBase/c8MAQ/src/callbacks.jl:14
 [11] initialize!
    @ ~/.julia/packages/DiffEqBase/c8MAQ/src/callbacks.jl:14 [inlined]
 [12] initialize!(cb::SciMLBase.CallbackSet{…}, u::ClimaCore.Fields.FieldVector{…}, t::Float64, integrator::ClimaTimeSteppers.DistributedODEIntegrator{…})
    @ DiffEqBase ~/.julia/packages/DiffEqBase/c8MAQ/src/callbacks.jl:7
 [13] __init(::SciMLBase.ODEProblem{…}, ::ClimaTimeSteppers.RosenbrockAlgorithm{…}; dt::Float64, tstops::Tuple{}, saveat::Vector{…}, save_everystep::Bool, callback::SciMLBase.CallbackSet{…}, advance_to_tstop::Bool, save_func::ClimaTimeSteppers.var"#36#38", dtchangeable::Bool, stepstop::Int64, tdir::Float64, kwargs::@Kwargs{…})
    @ ClimaTimeSteppers ~/.julia/packages/ClimaTimeSteppers/lF2K5/src/integrators.jl:129
 [14] __init
    @ ~/.julia/packages/ClimaTimeSteppers/lF2K5/src/integrators.jl:71 [inlined]
 [15] #init_call#40
    @ ~/.julia/packages/DiffEqBase/c8MAQ/src/solve.jl:530 [inlined]
 [16] init_call
    @ ~/.julia/packages/DiffEqBase/c8MAQ/src/solve.jl:503 [inlined]
 [17] #init_up#43
    @ ~/.julia/packages/DiffEqBase/c8MAQ/src/solve.jl:564 [inlined]
 [18] init_up
    @ ~/.julia/packages/DiffEqBase/c8MAQ/src/solve.jl:551 [inlined]
 [19] #init#41
    @ ~/.julia/packages/DiffEqBase/c8MAQ/src/solve.jl:544 [inlined]
 [20] macro expansion
    @ ~/work/ClimaAtmos.jl/ClimaAtmos.jl/src/solver/type_getters.jl:719 [inlined]
 [21] macro expansion
    @ ./timing.jl:503 [inlined]
 [22] macro expansion
    @ ~/work/ClimaAtmos.jl/ClimaAtmos.jl/src/utils/utilities.jl:331 [inlined]
 [23] get_simulation(config::ClimaAtmos.AtmosConfig{…})
    @ ClimaAtmos ~/work/ClimaAtmos.jl/ClimaAtmos.jl/src/solver/type_getters.jl:718
 [24] atmos_init(atmos_config::ClimaAtmos.AtmosConfig{…})
    @ Main ~/work/ClimaAtmos.jl/ClimaAtmos.jl/ClimaCoupler.jl/experiments/ClimaEarth/components/atmosphere/climaatmos.jl:35
 [25] top-level scope
    @ ~/work/ClimaAtmos.jl/ClimaAtmos.jl/ClimaCoupler.jl/experiments/ClimaEarth/run_amip.jl:188
in expression starting at /home/runner/work/ClimaAtmos.jl/ClimaAtmos.jl/ClimaCoupler.

This is probably related to some recent changes in insolation. @szy21 @cmschmitt519

[szy21]

Yes, we are aware of this and mark #3150 as a breaking change. But in general what should we do when this test is failing?

[Sbozzolo]

When this test is failing, this means that main of ClimaAtmos and main of ClimaCoupler are incompatible. In turn, this means that releasing ClimaAtmos will break ClimaCoupler. The greener we keep this test, the easier it is to update ClimaCoupler. When this test becomes red, it also hides all possible problems that come afterwards, so ideally, when a PR breaks a downstream test, we should open a sibling PR in ClimaCoupler to take care of the incompatibility.

Breaking changes will indeed break this test, but sometimes it is easy to ensure compatibility with previous versions.

A pattern you might consider is

if pkgversion(ClimaAtmos) <= v"0.26.3"
    insolation = my_insolation
else
   insolation = my_new_insolation
end

But this should not become a burden, so just do it if it is easy.

[szy21]

When this test is failing, this means that main of ClimaAtmos and main of ClimaCoupler are incompatible. In turn, this means that releasing ClimaAtmos will break ClimaCoupler. The greener we keep this test, the easier it is to update ClimaCoupler. When this test becomes red, it also hides all possible problems that come afterwards, so ideally, when a PR breaks a downstream test, we should open a sibling PR in ClimaCoupler to take care of the incompatibility.

But we can't merge the ClimaCoupler PR until the atmos is updated, so the downstream test will still fail. Should we just tag a minor release as soon as we have breaking changes? But then we will have many releases...

[Sbozzolo]

If it is easy, you can support both versions. If not, it is not a big deal, just wait until you release ClimaAtmos. This test is mostly to give us an indication of what might go wrong, but it is not a strict test.

[akshaysridhar]

@Sbozzolo This is fixed here: https://github.com/CliMA/ClimaCoupler.jl/pull/889. We may be able to close this issue. In this instance the solution was to update the coupler interface for compatibility with the latest atmos breaking changes.