-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hooks for overriding surface radiative fluxes seen by land and ocean models #31
Add hooks for overriding surface radiative fluxes seen by land and ocean models #31
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @bensonr. I think that using the existing Coupler
type or a new override data type would be cleaner and avoids needing to change the FV3 state.
Sounds good @lharris4. I'll pick this back up next week. |
@lharris4 @bensonr I went ahead and created a new type to hold this unusual fortran state called For good measure I also made sure to use these potentially overridden values when assigning state to the The model compiles and the tests pass in ai2cm/SHiELD-wrapper#5 with these code updates. Things should be ready for re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for putting this together so quickly and doing such thorough testing. I don't see any problems with your code. I just have a couple of minor cosmetic suggestions and a few things I would be interested in making sure I understand correctly (hence 'minor revisions').
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
437d1a7
to
71bdf66
Compare
71bdf66
to
b227a33
Compare
I think this should be ready for re-review. Spurred on by #31 (comment) and #31 (comment) I have refactored the diagnostics related code to always use the existing diagnostics buckets ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the suggested changes and adding appropriate comments. The logic behind the overrides is now easy to follow.
0f3677b
to
176da07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting in the extra effort to answer my many questions. I think this is good to go.
Excellent, thanks @lharris4! Should we be good to merge this and the related NOAA-GFDL/atmos_drivers#31 now? |
Glad to help :-) I think it should be good but I will defer to Rusty's team
for the final word on the driver modifications.
…On Wed, Nov 1, 2023 at 8:51 AM Spencer Clark ***@***.***> wrote:
Excellent, thanks @lharris4 <https://github.com/lharris4>! Should we be
good to merge this and the related NOAA-GFDL/atmos_drivers#31
<NOAA-GFDL/atmos_drivers#31> now?
—
Reply to this email directly, view it on GitHub
<#31 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVF4J6K2CMTGN5TMF5TYCJAWRAVCNFSM6AAAAAA6I7XSMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBYHEYDGOJVGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This PR updates the dycore, physics, and driver NOAA-GFDL submodules in this repo to their latest `main` branches. Now that - NOAA-GFDL/SHiELD_physics#31 - NOAA-GFDL/atmos_drivers#31 - NOAA-GFDL/atmos_drivers#30 - NOAA-GFDL/GFDL_atmos_cubed_sphere#302 have been merged, all the required features in SHiELD are in place to bring SHiELD-wrapper up-to-par with fv3gfs-wrapper. Note that NOAA-GFDL/SHiELD_physics#31 made changes to the long names of some of the diagnostics output in our regression test, which required a checksum update. I have also taken the liberty to remove the disclaimer from the README that states that changes to SHiELD are required to bring SHiELD-wrapper up-to-par with FV3GFS. That is no longer a limiting factor, and those remaining features are implemented in open PRs in this repo.
This PR ports the ability to override the surface radiative fluxes seen by the land surface model from the wrapper. This was split across two PRs originally in the case of FV3GFS: - ai2cm/fv3gfs-fortran#158 - ai2cm/fv3gfs-wrapper#244 This depends on the fortran changes made in: - NOAA-GFDL/SHiELD_physics#31 - NOAA-GFDL/atmos_drivers#31 which have now been merged, and incorporated into this repo via #11.
Description
This PR adds hooks to enable overriding the surface radiative fluxes seen by the land and ocean models following ai2cm/fv3gfs-fortran#158 in AI2's fork of FV3GFS. This document contains a description of the thought process behind this code in FV3GFS, which has been used in several studies. In short it:
IPD_Data%OverridesFromPythonWrapper
) that are meant to hold data for overriding the surface radiative fluxes (namely the downward longwave, downward shortwave, and net shortwave radiative components).gfs_physics_nml.override_surface_radiative_fluxes
namelist parameter._from_rrtmg
fields).Note that this PR also must be coupled with a PR to NOAA-GFDL/atmos_drivers to follow through with passing the
override_surface_radiative_fluxes
namelist parameter to the diagnostics initialization routine (see NOAA-GFDL/atmos_drivers#31).@lharris4 this should be the last set of PRs needed to obtain feature parity with our wrapper of FV3GFS. Since this does something science-related, I'm guessing it may require a more careful review.
How Has This Been Tested?
These changes have been tested in ai2cm/SHiELD-wrapper#5. There we test that overriding the fluxes changes the solution of the model, and that the diagnostics are updated as expected based on the overridden values. As expected, the answers for the model also do not change in configurations where
gfs_physics_nml.override_surface_radiative_fluxes
is.false.
(the default).Checklist:
Please check all whether they apply or not