Tendency application update#299
Conversation
…emove *_of_new_state (default name refers to 'current' value)
…medmfvdifq return new tendencies and new tendency application block in GFS_PBL_generic_post
- Added tendency variables (`ten_t`, `ten_u`, `ten_v`) for temperature and wind tendencies in SAMF deep and shallow convection schemes. - Updated relevant `.meta` files to reflect changes in variable intent (`inout` -> `in` where appropriate) and added tendencies. - Modified Interstitials (`GFS_DCNV_generic_post`, `GFS_DCNV_generic_pre`, `GFS_SCNV_generic_post`, `GFS_SCNV_generic_pre`) to directly use tendencies instead of saved state variables. - Removed redundant saved variables (`save_u`, `save_v`, `save_t`). - Added `delt` to post interstitials.
…' into feature/tendency_cleanup
…_dimension in GFS_time_vary_pre.scm
…endency paradigm; cleanup saving of states no longer needed
…as removed recently
grantfirl
force-pushed
the
feature/tendency_cleanup
branch
from
320ae9a to
5d02db8
Compare
|
@rhaesung @yangfanglin Here are some updated plots from testing: https://docs.google.com/presentation/d/1NXsgRd_2M-zAZIPlHagYhKa8W7TcJEftaR2sxjRvkfY/edit?slide=id.p#slide=id.p These results were obtained last week, after the code had been updated to the latest develop branch. I've added a test using the conus13km_control because it uses the FV3_HRRR suite with smoke/dust active (this wasn't tested last time). Based on these results, I'm still satisfied that these code changes aren't changing the scientific results at all. The exception is for the RAS scheme, which we're removing anyway, so I have not debugged the failed RTs. |
|
@grantfirl Thanks so much for the great work and for sharing the latest test results — I’ve started taking a look. |
@rhaesung Sure, I emailed you both offline to set up a time. |
rhaesung
left a comment
rhaesung
left a comment
There was a problem hiding this comment.
@grantfirl I've had a chance to go through the convection schemes in this PR. Here is a summary of the key points I've flagged.
- Bugs: found a few indexing errors (
ntr+1vsntr+2), missing/dt, and skippedten_uupdates. - Consistency: the internal state updates (like
qv) and diagnostic units (likeQLCN/QICN) - just want to ensure they all align with the new tendency framework. - Metadata minor cleanup
Thanks and looking forward to your thoughts!
| [pu] | ||
| standard_name = x_wind_of_new_state | ||
| standard_name = x_wind | ||
| long_name = updated x-direction wind |
There was a problem hiding this comment.
removing 'updated' would be more accurate. The same should be applied to pv, pt, and other meta files.
There was a problem hiding this comment.
Good point. I agree and I remember thinking it when I was making metadata edits, but I didn't bother due to long_names not being used to match anything. But, it makes sense to fix them because it can be misleading as-is. Will do.
Thanks @rhaesung for the thorough review. I'll address these right away. |
|
@grantfirl @rhaesung Have the requested changes been addressed? The parent PRs were marked ready for review, so I wanted to check on this. |
No, I'm making the changes now. There will likely be some more testing that needs to happen beyond RTs yet too, so this shouldn't be scheduled for the merge queue yet. |
rhaesung
left a comment
rhaesung
left a comment
There was a problem hiding this comment.
@grantfirl The overall direction of the refactor looks great! To bring GWD fully in line with your new framework, just a couple of points:
- Since
generic postnow handles diagnostic accumulation, please remove the manual updates in the primary schemes to avoid double-counting. - Let's keep temperature tendencies alongside the wind to ensure all variables follow the same logic and maintain physical symmetry.
…omputing diagnostic tendencies in drag_suite
11 participants
Description of Changes:
See https://docs.google.com/presentation/d/1LJbCrCVGYDvm0UPo8SR4KP6fT1VFocg-xLG399qGQ1w/edit?slide=id.g371ddcb3c06_0_238#slide=id.g371ddcb3c06_0_238 for a complete description.
Main points:
Tests Conducted:
See the linked slides. This was tested in the following ways:
Dependencies:
None
Documentation:
TODO
Issue (optional):
None
Contributors (optional):
@grantfirl @VanderleiVargas-NOAA