-
Notifications
You must be signed in to change notification settings - Fork 50
Create new scheme to get fields provided by CDEPS Inline #298
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
Create new scheme to get fields provided by CDEPS Inline #298
Conversation
|
@uturuncoglu Please let us know when this is ready for review by removing the draft label when you're ready. |
|
@grantfirl actually it is ready but @NickSzapiro-NOAA made some comment in FV3 side to make it more generic (NOAA-EMC/ufsatm#988 (comment)). At this point, I am not sure about going further to make those changes since I am not funded with this work and it is hard for me to find time. Maybe merging as it is and making those changes later would be good idea. @NickSzapiro-NOAA let me know what do you think. I also need to add new regression test in the model level but I think I could make it quickly. If it is okay to merge this as it is, then I could mark all the PRs ready to review and add a RT as a last item. |
|
Sounds good to me! Thanks for all of your efforts @uturuncoglu |
|
@NickSzapiro-NOAA Okay. Let me add an extra RT in the model level. It would be available soon. I am also making PRs ready for review. |
rhaesung
left a comment
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.
looks good to me - just a few minor suggestions.
| intent = in | ||
| optional = True | ||
| [fice_dat] | ||
| standard_name = sea_ice_area_fraction_of_sea_area_fraction_from_data |
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.
| standard_name = sea_ice_area_fraction_of_sea_area_fraction_from_data | |
| standard_name = sea_ice_area_fraction_from_data |
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.
@rhaesung I think I also need to change the standard name in FV3 side to make it consistent. Right?
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.
@rhaesung I think this is also fine to be consistent with fice.
…composites_pre.F90
Update of CDEPS inline sync without sfc_data
|
@uturuncoglu Please pull in the latest ufs/dev branch. |
fix bug in GFS_surface_composites_pre.F90
|
Upstream testing finished. This can be merged. |
Description of Changes:
This PR aims to create connection with CDEPS Inline hosted by FV model to update certain surface variables while model is running. Some informative slides about the implementation: https://docs.google.com/presentation/d/1w9-n0NKSC3go3i7Y9WiWOz6cJ5D1VXxnjMDYnyjStn0/edit?slide=id.g2b3156c1e0e_0_249#slide=id.g2b3156c1e0e_0_249
Tests Conducted:
We have a limited test with SRW HRRR configuration to update SST and other sea ice related variables over Great Lakes.
Dependencies:
Add any links to parent PRs (e.g. SCM and/or UFS PRs) or submodules (e.g. rte-rrtmgp). For example:
Documentation:
Does this PR add new capabilities that need to be documented or require modifications to the existing documentation? If so, brief supporting material can be provided here. Contact the CODEOWNERS if your PR requires extensive updates to the documentation. See https://github.com/NCAR/ccpp-doc for Technical Documentation or https://dtcenter.org/community-code/common-community-physics-package-ccpp/documentation for the latest Scientific Documentation.
N/A
Issue (optional):
If this PR is resolving or referencing one or more issues, in this repository or elewhere, list them here. For example, "Fixes issue mentioned in #123" or "Related to bug in NCAR/other_repository#123"
N/A
Contributors (optional):
If others have contributed to this work aside from the PR author, list them here
N/A