Add face_sw_direct_flux_dn only for AllSkyRadiationWithClearSkyDiagnostics

[Sbozzolo]

face_sw_direct_flux_dn is used only in
AllSkyRadiationWithClearSkyDiagnostics in this block:

NVTX.@annotate function update_sw_fluxes!(
    ::AllSkyRadiationWithClearSkyDiagnostics,
    model,
)
    RRTMGP.RTESolver.solve_sw!(
        model.sw_solver,
        model.as,
        model.lookups.lookup_sw,
        nothing,
        model.lookups.lookup_sw_aero,
    )
    parent(model.face_clear_sw_flux_up) .= parent(model.face_sw_flux_up)
    parent(model.face_clear_sw_flux_dn) .= parent(model.face_sw_flux_dn)
    parent(model.face_clear_sw_direct_flux_dn) .=
        parent(model.face_sw_direct_flux_dn)
    parent(model.face_clear_sw_flux) .= parent(model.face_sw_flux)
    RRTMGP.RTESolver.solve_sw!(
        model.sw_solver,
        model.as,
        model.lookups.lookup_sw,
        model.lookups.lookup_sw_cld,
        model.lookups.lookup_sw_aero,
    )
end

It is not used by other modes, so I don't add it

[szy21]

Actually, we should have it in other radiation modes as well. Right now for other radiation modes, p.radiation.rrtmgp_model.model.face_sw_direct_flux_dn should have the correct values (hopefully). We don't use it right now but the land may want to access it at some point. We just have it explicitly used for AllSkyRadiationWithClearSkyDiagnostics because we are assigning it to face_clear_sw_direct_flux_dn when calling radiation without clouds.

[Sbozzolo]

Actually, we should have it in other radiation modes as well. Right now for other radiation modes, p.radiation.rrtmgp_model.model.face_sw_direct_flux_dn should have the correct values (hopefully). We don't use it right now but the land may want to access it at some point. We just have it explicitly used for AllSkyRadiationWithClearSkyDiagnostics because we are assigning it to face_clear_sw_direct_flux_dn when calling radiation without clouds.

Where is this set/updated? There's no other reference to it besides that function

[szy21]

Where is this set/updated? There's no other reference to it besides that function

If I understand the interface correctly, here we are setting face_sw_direct_flux_dn to be a view of flux_sw. flux_dn_dir (and we set many other things here as well). So after we call e.g. RRTMGP.RTESolver.solve_sw! where flux_sw.flux_dn_dir is updated, face_sw_direct_flux_dn will be updated.

[sriharshakandala]

Actually, we should have it in other radiation modes as well. Right now for other radiation modes, p.radiation.rrtmgp_model.model.face_sw_direct_flux_dn should have the correct values (hopefully). We don't use it right now but the land may want to access it at some point. We just have it explicitly used for AllSkyRadiationWithClearSkyDiagnostics because we are assigning it to face_clear_sw_direct_flux_dn when calling radiation without clouds.

I believe, based on our previous discussion, only the value of flux_dn_dir at surface is needed. Currently, the value at the surface should be correct but we do not guarantee that it is correct/updated at all levels!

[Sbozzolo]

Actually, we should have it in other radiation modes as well. Right now for other radiation modes, p.radiation.rrtmgp_model.model.face_sw_direct_flux_dn should have the correct values (hopefully). We don't use it right now but the land may want to access it at some point. We just have it explicitly used for AllSkyRadiationWithClearSkyDiagnostics because we are assigning it to face_clear_sw_direct_flux_dn when calling radiation without clouds.

I believe, based on our previous discussion, only the value of flux_dn_dir at surface is needed. Currently, the value at the surface should be correct but we do not guarantee that it is correct/updated at all levels!

This is what I see

julia> simulation.integrator.p.radiation.rrtmgp_model.face_sw_direct_flux_dn
11×3456 view(::Matrix{Float32}, 1:11, :) with eltype Float32:
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  …  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  …  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN     NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN
 NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN  …  NaN  NaN  NaN  NaN  NaN  NaN  NaN  NaN

For a allsky case. Is this correct?

[szy21]

I think flux_dn_dir is allocated in FluxSW struct but never gets updated in RRTMGP except for the first level here, and that's maybe why it's all NaNs in atmos. But I thought the first level would have correct values?

[sriharshakandala]

I think flux_dn_dir is allocated in FluxSW struct but never gets updated in RRTMGP except for the first level here, and that's maybe why it's all NaNs in atmos. But I thought the first level would have correct values?

The value at the first level is set here: https://github.com/CliMA/RRTMGP.jl/blob/main/src/rte/shortwave2stream.jl#L233 for the two stream shortwave solver!
Per our earlier discussions, my understanding is that it was only needed at the surface level! I was about to resize the array to only have one level but held off on that change since it would be a breaking change!

[Sbozzolo]

I found the issue and fixed in RRTMGP: CliMA/RRTMGP.jl#550

Closing this