-
Notifications
You must be signed in to change notification settings - Fork 153
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 new PBL diagnostic variables dkt and dku #607
Merged
Merged
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
100a646
Cleanup trailing whitespace for some new added/modified source code r…
BinLiu-NOAA a8265eb
Merge pull request #4 from hafs-community/feature/merge_hwrf_physics
climbfuji c13b36d
Merge pull request #2 from climbfuji/update_hafs_from_dtc_hwrf_physic…
climbfuji 8f34789
Merge remote-tracking branch 'remotes/upstream/master' into support/HAFS
BinLiu-NOAA 53fc0ff
In GFS_rrtmg_pre.F90, separate out the imp_physics == 6 option and le…
BinLiu-NOAA 81b1ba2
Merge remote-tracking branch 'remotes/upstream/master' into feature/s…
BinLiu-NOAA 643ede9
Merge remote-tracking branch 'remotes/upstream/master' into feature/s…
BinLiu-NOAA a1f0373
Merge branch 'feature/sync_master_20200703' of https://github.com/haf…
BinLiu-NOAA 0a8bd93
Initialize crtlame properly in physics/samfdeepcnv.f.
BinLiu-NOAA b6dd33b
Merge remote-tracking branch 'remotes/upstream/master' into feature/s…
BinLiu-NOAA 1b400f5
Follow the master branch to change the unit of radians into radian in
BinLiu-NOAA 3a834f9
Merge remote-tracking branch 'upstream/master' into support/HAFS
BinLiu-NOAA d07d53b
Merge remote-tracking branch 'remotes/upstream/master' into support/HAFS
BinLiu-NOAA 2345803
Merge remote-tracking branch 'remotes/upstream/master' into support/HAFS
BinLiu-NOAA 746cb5b
Merge commit '13b8889' used by ufs-weather-model as of 20210120 into …
BinLiu-NOAA c0ac40b
Merge remote-tracking branch 'remotes/upstream/master' into support/HAFS
BinLiu-NOAA 048d605
Merge remote-tracking branch 'remotes/upstream/master' into support/HAFS
BinLiu-NOAA 97f409e
Merge remote-tracking branch 'remotes/upstream/master' into support/HAFS
BinLiu-NOAA 094d39c
Merge remote-tracking branch 'remotes/upstream/master' into support/HAFS
BinLiu-NOAA 6f9c83e
Merge remote-tracking branch 'upstream/master' into support/HAFS
BinLiu-NOAA 9143484
Add dkudiagnostic in moninedmf from Andy Hazelton.
BinLiu-NOAA 2ce81df
Merge remote-tracking branch 'upstream/master' into feature/dkudiagno…
BinLiu-NOAA 8c44643
*Rename dkudiagnostic into dku in moninedmf.f and satmedmfvdifq.F.
BinLiu-NOAA 4c84630
Use unique standard_name for the new added dkt and dku diagnostic var…
BinLiu-NOAA 28c815a
Unify the usage of Diag%dkt and Interstitial%dkt and change the stand…
BinLiu-NOAA 7b7eb85
Merge remote-tracking branch 'remotes/upstream/master' into feature/d…
BinLiu-NOAA File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Please remove
_from_gfspbl
here and also for[dku]
. Same inphysics/satmedmfvdifq.meta
and inGFS_typedefs.meta
. There is no reason why these variables should be specific toGFS PBL
.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.
@climbfuji We actually were using the names without adding "_from_gfspbl" in the dkt/dku stand_names originally, However, the model build failed due to the complaining that there are duplicated standard names of "atmosphere_heat_diffusivity" in ccpp/data/GFS_typedefs.meta.
As you can see, there is already a dkt under the GFS_interstitial_type section with a standard name of "atmosphere_heat_diffusivity". After looking into it a little more, we saw some examples on how to use different names for the same/similar variables in this GFS_typedefs.meta file, for example:
standard_name = instantaneous_atmosphere_heat_diffusivity
standard_name = atmosphere_heat_diffusivity_background
standard_name = atmosphere_heat_diffusivity_background_maximum
standard_name = atmosphere_heat_diffusivity_from_shoc
standard_name = atmosphere_heat_diffusivity_for_mynnpbl
standard_name = atmosphere_heat_diffusivity
With that, we introduced the
standard_name = atmosphere_heat_diffusivity_from_gfspbl
for dkt (did the same for dku). After that the build worked fine. Agree that, this is not an elegant way and sometime confusing to use different standard_names for the same variable. It would be nice to standardize or unify this from the model and ccpp interface side. Please let us know if you have any comments/suggestions. Thanks!
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.
Oh, I see. Not your fault at all, just terrible precedence. In this case, I suggest to leave the PR as-is, let's clean this up afterwards.
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.
Yeah, some confusion about what was going on with these similarly-named variables is why I made a new one "dkudiagnostic" for momentum diffusivity, but that's obviously not the best solution memory-wise. Definitely would be good if we can help clean this up in the future hopefully.
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.
@climbfuji, With some further discussions and code digging with @ChunxiZhang-NOAA, it looks like our current solution for diagnostic dkt variable will probably break the the existing interstitial dkt usage in physics/moninedmf.f[meta] and in ccpp/data/GFS_typedefs.F90[meta]. Currently, we were thinking of the following two potential solutions:
Appreciate it if you could provide any insight/recommendation on how to move forward with this change. Thanks!
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.
@climbfuji Yes, this is also a good solution. I searched out the usage of "Interstitial%dkt" inside ufs-weather-model. These are the locations the Interstitial%dkt was used:
So, if we move the dkt into the Diag%dkt section. Then the above will be the places need to be replaced by Diag%dkt, right? Of course, in this case the dkt and dku can then use the names of atmosphere_heat_diffusivity and atmosphere_momentum_diffusivity. Do you think this will work? Thanks!
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 think so, yes. The whole point moving it from
Interstitial%
toDiag%
is to make it persistent so that it can be written out. InGFS_debug.F90
, it will need to go from moduleGFS_interstitialtoscreen
toGFS_diagtoscreen
, but that should be pretty much self-explanatory in the file.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 @climbfuji! I have just made and pushed the changes into this branch and in the FV3atm side as well. Please help to check/review the changes when you get a chance. Thanks!
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.
Ok, let's get started with testing this today, your PR will go next after the current LAM post commit.
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.
Ok. Just synced this branch with the latest master to get it ready for model level RT.