fix for 4diau with iau_filter_increments=T #167
Conversation
There was a problem hiding this comment.
I tested the changes for both 4DIAU and 3DIAU. The fix works for 4DIAU, but not for 3DIAU. When running 3DIAU, nfiles = 1 and IPD_Control%iaufhrs(1) = IPD_Control%iaufhrs(nfiles). The 'if' condition in line 318 is never satisfied (iau_state%wt is always 0), and the if condition in line 336 is always satisfied (IAU_Data%in_interval is always false).
|
The new changes works for 3DIAU now, but not 4DIAU. The problem is the change in line 357. The t2 here has different meaning. It's no longer IPD_Control%iaufhrs(nfiles). If reverting the change of line 357, it works for 4DIAU. I have another recommendation for 3DIAU. Please consider removing the 'if' condition 'if (IPD_Control%iau_filter_increments) ' in line 348. Otherwise, if iau_filter_increments = .false., setiauforcing will not be called, the iau increment will not be normalized by rdt. |
There was a problem hiding this comment.
The new changes works for 3DIAU now, but not 4DIAU. The problem is the change in line 357. The t2 here has different meaning. It's no longer IPD_Control%iaufhrs(nfiles). If reverting the change of line 357, it works for 4DIAU.
I have another recommendation for 3DIAU. Please consider removing the 'if' condition 'if (IPD_Control%iau_filter_increments) ' in line 348. Otherwise, if iau_filter_increments = .false., setiauforcing will not be called, the iau increment will not be normalized by rdt.
|
Please ignore my second recommendation. I saw that setiauforcing is called in IAU_initialize. |
|
@MingjingTong-NOAA do you think this PR is acceptable or do you still think there are changes that should be made? |
|
Please make sure line 355 'if (IPD_Control%fhour < t1 .or. IPD_Control%fhour >= t2) then' is reverted to 'if (IPD_Control%fhour < IPD_Control%iaufhrs(1) .or. IPD_Control%fhour >= IPD_Control%iaufhrs(nfiles)) then'. There is no other changes needed. |
|
@MingjingTong-NOAA thanks for reviewing this. I see the problem you found is related to the variable t2 being redefined. Instead of your suggested fix (which would have worked fine) I created a different variable tnext and preserved the t2 variable to use in the if test. 4DIAU should work now. |
|
The fix looks fine to me. The pull request can be merged. Thanks! |
|
@laurenchilutti @bensonr All the RT passed in ufs-weather-model PR#990, would you please commit this PR? Thanks |
* Add get_nth_domain_info subroutine * Deallocate local arrays in fv_dyn_bundle_setup * adding back a line that was mistakenly deleted in a previous commit (NOAA-GFDL#166) * Do not print debug messages to stderr * fix for 4diau with iau_filter_increments=T (NOAA-GFDL#167) * fix for 4diau with iau_filter_increments=T * fix time interval for 3DIAU * fix typo in comment * fix bug found in review by @MingjingTong-NOAA * change tnext to integer variable itnext Co-authored-by: jswhit2 <Jeffrey.S.Whitaker@noaa.gov> * Use mpp_error instead of write statements in model/fv_regional_bc.F90 * Attempt at integrating fixes on top of dev/emc branch. (NOAA-GFDL#173) Co-authored-by: Dusan Jovic <dusan.jovic@noaa.gov> Co-authored-by: laurenchilutti <60401591+laurenchilutti@users.noreply.github.com> Co-authored-by: Jeff Whitaker <jswhit@fastmail.fm> Co-authored-by: jswhit2 <Jeffrey.S.Whitaker@noaa.gov> Co-authored-by: MatthewPyle-NOAA <48285220+MatthewPyle-NOAA@users.noreply.github.com>
* Add get_nth_domain_info subroutine * Deallocate local arrays in fv_dyn_bundle_setup * adding back a line that was mistakenly deleted in a previous commit (NOAA-GFDL#166) * Do not print debug messages to stderr * fix for 4diau with iau_filter_increments=T (NOAA-GFDL#167) * fix for 4diau with iau_filter_increments=T * fix time interval for 3DIAU * fix typo in comment * fix bug found in review by @MingjingTong-NOAA * change tnext to integer variable itnext Co-authored-by: jswhit2 <Jeffrey.S.Whitaker@noaa.gov> * Use mpp_error instead of write statements in model/fv_regional_bc.F90 * Attempt at integrating fixes on top of dev/emc branch. (NOAA-GFDL#173) * Support for cloud microphysics hail species (NOAA-GFDL#171) * Incorporated Tim Supinie's updates to handle a hail category (nwat=7) * Added print for hail max/min (non-debug section) * Add hallwat to !OMP shared variables in main loop Addresses compile failure when OMP is turned on (e.g., on Hera) * Update to states as used in SFE2021 (Removed SFE code for now) * Cleanup after rebase * Redo removal of alt. namelist read * Removed tools/module_diag_hailcast.F90 * Fixes from Jili Dong * Add hailwat to tools/external_ic.F90 * Set logic in fv_regional_bc.F90 to match current code; Change logic in external_ic.F90 to check value of nwat * Recommended changes * Check actual index (hailwat) instead of assuming a positive value based on 'nwat' value * Split nwat=7 bits into separate loops (see if it affects nwat=6) Co-authored-by: Larissa Reames <52886575+LarissaReames-NOAA@users.noreply.github.com> Co-authored-by: LarissaReames-NOAA <larissa.reames@noaa.gov> Co-authored-by: Dusan Jovic <dusan.jovic@noaa.gov> Co-authored-by: laurenchilutti <60401591+laurenchilutti@users.noreply.github.com> Co-authored-by: Jeff Whitaker <jswhit@fastmail.fm> Co-authored-by: jswhit2 <Jeffrey.S.Whitaker@noaa.gov> Co-authored-by: MatthewPyle-NOAA <48285220+MatthewPyle-NOAA@users.noreply.github.com> Co-authored-by: Ted Mansell <37668594+MicroTed@users.noreply.github.com> Co-authored-by: Larissa Reames <52886575+LarissaReames-NOAA@users.noreply.github.com> Co-authored-by: LarissaReames-NOAA <larissa.reames@noaa.gov>
Description
This PR fixes a bug in the 4DIAU code when the parameter iau_filter_increments=T. The time bounds for the IAU interval were computed incorrectly, leading to incorrect filter weights that were applied outside the designated time range. The 4DIAU with constant weights, and 3DIAU (with a single increment file specified) were not affected by this bug.
Fixes ufs-community/ufs-weather-model#989
How Has This Been Tested?
Regression tests pass on hera, cycling DA experiment with iau_filter_increments=T and hourly increment files ran on orion.
Checklist:
Please check all whether they apply or not