Skip to content
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

Passing additional surface variables into the atmospheric model #92 #93

Merged
merged 5 commits into from
Nov 17, 2023

Conversation

zhihong-tan
Copy link
Contributor

This PR is linked to issue #92.

A new boundary layer scheme (NCEP TKE-based eddy-diffusivity mass-flux scheme) is being implemented in the prototype-AM5 model, and it requires five additional surface variables (shflx, lhflx, wind, thv_atm, and thv_surf) as inputs. Thus, I would like to add a few lines in the surface coupler (full/atm_land_ice_flux_exchange.F90 and shared/surface_flux.F90) to pass these variables into the atmospheric model. My branch contains the proposed changes, and the model results are unchanged with these changes.

Zhihong Tan added 3 commits September 21, 2023 17:13
The shflx, lhflx, wind, thv_atm, thv_surf variables are passed
from the surface coupler to the atmospheric model as required by
the NCEP TKE-EDMF scheme. And some useful diagnostics are added
to output the surface fluxes before the implicit surface coupling.
Changing the names of fms subroutines (register and send
diagnostic variables, and get data from xgrid).
The added diagnostics for surface fluxes before the implcit
surface coupling have been removed to minimize the changes
from the original codes.
@thomas-robinson
Copy link
Member

@zhihong-tan have you tested this with the doubly periodic, aquaplanet, or any other model that uses the simple coupler? Those models use the surface flux file but not the full coupler. It's possible that this code doesn't affect those models, or it may require changes to their respective coupler implementations.

@thomas-robinson
Copy link
Member

@zhihong-tan Please remove any comments with your name and the date. Git will track that information, so we don't need to clutter the code with comments.

@thomas-robinson
Copy link
Member

There are also some compile errors in the CI that need to be addressed.

@rem1776
Copy link
Contributor

rem1776 commented Oct 17, 2023

Looks like the compile errors are coming from the atmosphere null model we're using for the CI.

To fix it would require a change in https://github.com/NOAA-GFDL/atmos_null. The fields added to the land_ice_atmos_boundary_type in AM5 will also need to be added there

@zhihong-tan
Copy link
Contributor Author

@thomas-robinson Thanks for your suggestions! I have not run the doubly-periodic or aquaplanet models with simple coupler, and I think they may not work with my modified surface_flux.F90. I will change the two added output variables from surface_flux subroutine to 'optional' and test whether this is sufficient for compatibility with the simple coupler.

@thomas-robinson
Copy link
Member

@zhihong-tan Do you have an update on this?

Zhihong Tan added 2 commits October 26, 2023 18:45
The same code modifications as the full coupler are now incorporated
into the simple coupler, which is now compatible with the updated
surface_flux subroutine.
@zhihong-tan
Copy link
Contributor Author

zhihong-tan commented Oct 27, 2023

@zhihong-tan Do you have an update on this?

@thomas-robinson I have modified the simple coupler and removed my name tags in the comments as suggested. The updates have been pushed to my branches of the FMScoupler and atmos_drivers repositories. I have verified that the new codes work with both the default AM4 and the simple version under the doubly_periodic repository. The 8-day regression run results are exactly reproduced for the "c96L33_am4p0" experiment in the default AM4 and for the "aqua_c96L33_am4p0_control" experiment in the doubly_periodic version.

@zhihong-tan
Copy link
Contributor Author

Should I also clone the atmos_null repository, modify the codes, and submit a merge request? Or could you do that for me? (I don't know how to do the CI when multiple repositories are changed).

@thomas-robinson
Copy link
Member

@zhihong-tan the fastest approach would be for you to update atmos_null and any other repositories that would require updating. If you want MSD to do it, the time frame would be a few months.

We should also set up a meeting to go over these changes and discuss why they are necessary at the coupler level. I'll reach out via email for that.

@zhihong-tan
Copy link
Contributor Author

@zhihong-tan the fastest approach would be for you to update atmos_null and any other repositories that would require updating. If you want MSD to do it, the time frame would be a few months.

We should also set up a meeting to go over these changes and discuss why they are necessary at the coupler level. I'll reach out via email for that.

Thanks for your suggestion! I have forked and updated the atmos_null to be compatible with the updated coupler, for which I have submitted a merge request. I will explain my proposed updates for these three repositories (FMScoupler, atmos_drivers, and atmos_null) in our meeting tomorrow.

@thomas-robinson
Copy link
Member

I'm not sure if you will also need to update atmos_solo_land on the gfdl gitlab

@bensonr
Copy link
Contributor

bensonr commented Nov 2, 2023

@zhihong-tan @thomas-robinson - I'd really like to see all of the related PRs in different repos, including the internal gitlab ones, be linked in to the PR description here so that we can view the changes as a whole. As I've looked at the various PRs individually, I believe I've seen changes to interfaces in atmos-drivers that will require changes in the AMx physics driver interfaces. My concern right now is exposing these new variables will be good for AM5 development, but not for AM4 and AM3 models without macro-controlled conditional compilation.

@rem1776 rem1776 merged commit 740edc2 into NOAA-GFDL:main Nov 17, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants