Add rainfall_flux diagnostics at day/mon/6hr frequencies (issue #667) - #680
Add rainfall_flux diagnostics at day/mon/6hr frequencies (issue #667)#680Zubair Maalick (zmaalick) wants to merge 1 commit into
Conversation
…fice#667) Implements three output configurations for the rainfall_flux diagnostic (UM STASH m01s05i214, lbproc=128): - Daily mean over land: new Group A field processed__rainfall_flux_land (= processed__total_rain * surface__land_fraction) added to the daily averaged group in file_def_diags_gal_clim.xml. - Monthly mean: processed__total_rain added to a new monthly output file (lfric_diagnostics_monthly, output_freq=1mo) in file_def_diags_gal_clim.xml. - 6-hourly mean: processed__total_rain added to a new lfric_gl_std_levs_diags_6hr file block in file_def_diags_oper_nwp_gl.xml. Registry-only change; no Fortran/kernel/algorithm edits. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hello Zubair Maalick (@zmaalick)! 👋 Thank you for your contribution. Since this is your first time contributing to this repository, we ask that you sign our Contributor Licence Agreement (CLA). To agree to the CLA, please add your details (GitHub username, Real Name, Affiliation, and Date) to the CONTRIBUTORS.md file (create one, if required) in the development branch for this PR. After signing the CLA, you won't need to do this again for future PRs. |
iboutle
left a comment
There was a problem hiding this comment.
Hi Zubair,
I've asked Jon for clarification, but he's away at the moment. Adding the new land-rainfall diagnostic to field_def_diags is fine.
However, I don't think you should be editing anything currently in the rose-stem suite, as none of these jobs are relevant for the CMIP diagnostics which you are adding. I've asked Jon whether we want to create a new test for the CMIP diagnostics, or whether we are happy that since they are just basic time processing of existing diagnostics, we don't need bespoke testing, it's just something that will need setting up in the actual CMIP standalone suites.
But for now, please revert any changes to the rose-stem directory.
Thanks!
It also occurs to me that the land rainfall flux diagnostic might be wrong - what is produced here is the land fraction multiplied by the rainfall flux. Whereas I'm wondering if what is actually produced in the UM is simply the rainfall falling on any land points (i.e. the field is zeroed over sea). I'm not sure though, as the stash request is just for the total rainfall, so there must be some post-processing or other processing happening here. So actually please check if this should just be a masked version of the rainfall diagnostic (and again then probably worth a discussion with Jon whether this is best done in-model or in post-processing) |
|
Hi iboutle thanks for all of the advice on this one. We suspect that there's a bug in UKNCSP/CDDS-CMIP7-mappings#763, which was used to generate the mapping spreadsheet. The variable should actually be output on all points. The land mask is only applied in post-processing, outside of the model. Zubair Maalick (@zmaalick) would you be able to update the PR in light of iboutle's comment and the bug in the mappings so that the output is on all model points please? |
In which case I don't think there is anything for you to do here, because the rainfall_flux is already available on all points, so nothing new needs adding. As an aside, as you mentioned this morning, it would certainly be possible to put the masking function into the model rather than as a post-processing step. |
Implements three output configurations for the rainfall_flux diagnostic (UM STASH m01s05i214, lbproc=128):
Registry-only change; no Fortran/kernel/algorithm edits.
PR Summary
Sci/Tech Reviewer:
Code Reviewer: allynt
Code Quality Checklist
Testing
trac.log
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review