Regional decomposition test fix (when nrows_blend > 0) by MicroTed · Pull Request #194 · NOAA-GFDL/GFDL_atmos_cubed_sphere

MicroTed · 2022-05-26T16:51:22Z

Description

Fixes failure of decomposition test for regional case with nrows_blend > 0. Also fixes a bug where nrows_blend=1 caused a crash. Credit to @junwang-noaa for fix in dyn_core.F90
Incidental (unrelated) fix in driver/fvGFS/atmosphere.F90 and fv_sg.F90 to add check for value of hailwat (nwat = 7)

Fixes dcp test failure that was raised in ufs-community/ufs-weather-model#1158

How Has This Been Tested?

Tested on hera with
./opnReqTest.ted -n regional_3km -c dcp -ek
(requires a change to tests/opnReqTests/dcp.sh)

See: https://github.com/MicroTed/ufs-weather-model.git, branch: regional_blend_fix

Checklist:

Please check all whether they apply or not

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

…on corners. Still not working for u and v.

dyn_core.F90 : Fix from Jun Wang to correct sync of u,v fv_regional_bc.F90 : add check for nrows_blend > tile size; fix error when nrows_blend=1

junwang-noaa · 2022-05-27T14:20:00Z

@laurenchilutti @bensonr Would you please review this PR? Thanks

laurenchilutti

Left a few comments regarding commenting out old code.

laurenchilutti · 2022-05-27T17:42:46Z

                                                     call timing_on('COMM_TOTAL')
 #ifndef ROT3
-    if ( it/=n_split)   &
+!    if ( it/=n_split)   &


Is there a reason you are leaving this commented? I would recommend deleting this line of code instead of commenting it out.

Could you delete this commented code?

Weird, I thought I had done that and the next one already... will do.

laurenchilutti · 2022-05-27T17:43:19Z

                                       reg_bc_update_time,it )

-         call mpp_update_domains(u, v, domain, gridtype=DGRID_NE)
+!         call mpp_update_domains(u, v, domain, gridtype=DGRID_NE)


Same comment as above - I would recommend deleting the old code as opposed to commenting it out

Could you delete this commented code?

laurenchilutti · 2022-05-27T17:45:27Z

 !
            i1_blend=i1-nrows_blend_user
            i2_blend=i1-1
+! Original code avoided overlap, but changed this to blend corners from both boundaries


Same comment as above.

I was thinking that these were worth keeping since the intent of the code was changed. In case somebody wants to try it that way again.

Git maintains a history of all changes - if anyone wants or needs to reintroduce the old code, they could still do that via git. I'd still recommend deleting these comments to maintain clean code.

Sure, no problem. I pushed an update to delete the code and just left a comment instead.

laurenchilutti

Thanks! it looks good to me.

laurenchilutti · 2022-06-08T12:47:18Z

@junwang-noaa Please let me know when to commit this.

junwang-noaa · 2022-06-08T13:07:09Z

@MicroTed Would you please create fv3/ufs-weather-model PRs to use your dycore branch with the fix? Thanks

MicroTed · 2022-06-08T13:54:23Z

@junwang-noaa How about if I try adding it to my current PR, since this was the issue holding that up? ufs-community/ufs-weather-model#1158

junwang-noaa · 2022-06-08T14:05:16Z

@MicroTed We are still working on the restart reproducibility capability for PR#1158, we may not be able to commit #1158 until that issue is resolved. So if we can have a new PR, we can at least get this decomposition committed to the UFS WM.

…ere#194

SamuelTrahanNOAA · 2022-06-15T18:50:52Z

By request of @junwang-noaa, this GFS_atmos_cubed_sphere PR will be merged along with a few other bug fixes, in these PRs:

fv3atm (parent PR): NOAA-EMC/ufsatm#553
ufs-weather-model (grandparent PR): ufs-community/ufs-weather-model#1257
ccpp-physics (sibling PR): NCAR/ccpp-physics#942

laurenchilutti · 2022-06-16T11:27:52Z

@SamuelTrahanNOAA @junwang-noaa Does your comment mean that we should merge this now?

SamuelTrahanNOAA · 2022-06-16T11:31:46Z

No, you should wait until the final regression testing is complete. Otherwise, we won't know if it works on all platforms.

SamuelTrahanNOAA · 2022-06-16T11:32:50Z

You should hear from one of the NOAA-EMC fv3atm code managers when it is time to merge this PR.

jkbk2004 · 2022-06-17T13:37:58Z

@junwang-noaa @laurenchilutti can we merge in this PR? We tested ok ufs-community/ufs-weather-model#1257

Regional decomposition test fix (when nrows_blend > 0) (NOAA-GFDL#194)

* Regional bc blend changes to extend into interior halos and overlap on corners. Still not working for u and v. dyn_core.F90 : Fix from Jun Wang to correct sync of u,v fv_regional_bc.F90 : add check for nrows_blend > tile size; fix error when nrows_blend=1 Conflicts (not taken during chery-pick): driver/SHiELD/atmosphere.F90 model/fv_sg.F90

* Regional bc blend changes to extend into interior halos and overlap on corners. Still not working for u and v. dyn_core.F90 : Fix from Jun Wang to correct sync of u,v fv_regional_bc.F90 : add check for nrows_blend > tile size; fix error when nrows_blend=1 Conflicts (not taken during chery-pick): driver/SHiELD/atmosphere.F90 model/fv_sg.F90 Co-authored-by: Ted Mansell <37668594+MicroTed@users.noreply.github.com>

* updated fv_regional_bc.F90 to read levsp from GFS_ctl file (#152) * Updating a continue statement in fv_nudge (#153) * update in fv_nudge to fix 666 label logic * fix logic errors (#138) * update to external_ic::remap_scale to skip remapping non-existent IC tracers (#159) * Fix nudge logic (#157) * fix logic errors * fix answer changes * adding back a line that was mistakenly deleted in a previous commit (#166) * Release 042022 (#184) * Merge of updates from GFDL Weather and Climate Dynamics Division (202204): *add license header to missing files and fix typo in header *updates needed for fv3_gfsphysics to have access to bounded_domain *remove obsoleted driver/SHiELD files *updating to fix bug where long_name and units attributes were not being captured in the RESTARTS *remove unused function fv_diagnostics::max_vorticity_hy1 *remove avec timer remnants *adding ability to specify prefix and directory when reading and writing restarts *remove old style namelist read in favor of read from internal character variable *Added option for a mean wind *The planetary radius and rotation rate are now re-scalable by a namelist parameter (small_earth_scale) instead of using exclusively the hard-coded FMS constant. *fv_mapz: Cleanup and added helpful annotations to avoid getting lost so easily * remove duplicate code and fix lstatus on all grids depending on gfs_data and gfs_data.tile1 * New idealized tests *Makes the non-hydrostatic restart variables optional for reads to allow hydrostatic ICs *Fix the hydrostatic TE remapping; Add GMAO cubic for TE remapping, which is used if kord_tm=0 and remap_te=.true. *Add a TE remapping option (kord_tm=0) *Addressing GNU Warnings *Add the L75 vertical config from HAFS * clean up fms_mp_mod and remove mp_bcst * cherry pick 5193c6b from dev/emc * Attempt at integrating fixes on top of dev/emc branch. (#173) * remove outdated GFDL_tools/fv_diag_column.F90 which exists as the result of an improper merge. The correct file is tools/fv_diag_column.F90 (#191) * various io fixes (#192) * fixes io performance issues by making everyone a reader when io_layout=1,1 adds capability to use FMS feature to ignore data integrity checksums in restarts * rename enforce_rst_cksum to ignore_rst_cksum and change the default value for compatibility * fix index error (#196) * New notebooks (#190) * Moving source files into source directory * Added advection notebook * Fixed subplot spacing * New 3D case notebooks * New idealized tests Updated mtn_wave_tests_1km to restore missing graphics. * first try at held-suarez * Updated H-S superrotation notebook * New level placement tool in an interactive note * Minor updates to notebooks; deleted fv_eta binary. * Regional decomposition test fix (when nrows_blend > 0) (#194) (#200) * Regional bc blend changes to extend into interior halos and overlap on corners. Still not working for u and v. dyn_core.F90 : Fix from Jun Wang to correct sync of u,v fv_regional_bc.F90 : add check for nrows_blend > tile size; fix error when nrows_blend=1 Conflicts (not taken during chery-pick): driver/SHiELD/atmosphere.F90 model/fv_sg.F90 Co-authored-by: Ted Mansell <37668594+MicroTed@users.noreply.github.com> * Implementing CI (#207) * CI Parallelworks update (#211) * Update call to read_input_nml and remove unnecessary code. (#161) * Removing use of INTERNAL_FILE_NML and cleaning up read_namelist_test_cases to remove unused argument * deleting duplicate call to read_namelist_test_case_nml in fv_control * removing commented code in fv_control Co-authored-by: Rusty Benson <6594772+bensonr@users.noreply.github.com> Co-authored-by: menzel-gfdl <43218622+menzel-gfdl@users.noreply.github.com> Co-authored-by: Rusty Benson <Rusty.Benson@noaa.gov> Co-authored-by: MatthewPyle-NOAA <48285220+MatthewPyle-NOAA@users.noreply.github.com> Co-authored-by: lharris4 <53020884+lharris4@users.noreply.github.com> Co-authored-by: Ted Mansell <37668594+MicroTed@users.noreply.github.com>

@BinLiu-NOAA

* fixes io performance issues by making everyone a reader when io_layout=1,1 adds capability to use FMS feature to ignore data integrity checksums in restarts * rename enforce_rst_cksum to ignore_rst_cksum and change the default value for compatibility * updated UFS/GFS atmosphere.F90 driver as per @BinLiu-NOAA and @junwang-noaa * Regional decomposition test fix (when nrows_blend > 0) (NOAA-GFDL#194) * Add missing instance for hailwat * Regional bc blend changes to extend into interior halos and overlap on corners. Still not working for u and v. * atmosphere.F90 : add hailwat to check dyn_core.F90 : Fix from Jun Wang to correct sync of u,v fv_regional_bc.F90 : add check for nrows_blend > tile size; fix error when nrows_blend=1 * Explanatory comment added * Removed commented code * Clean old code * In fv_fill.F90, use kind_phys for kind_phys instead of hard-coding 8 byte reals. (NOAA-GFDL#193) * Expose remap_scalar and remap_dwinds to fv3-jedi (NOAA-GFDL#199) * changed interface to public * added public * removed source * mods for jedi build * Transfer changes from PR NOAA-GFDL#202 to NOAA-GFDL#199 Made small changes from PR NOAA-GFDL#202 manually. * returned ignore checksum * fixed ignore checksum * Fix several bugs in fv_regional_bc.F90 relating to uninitialized or incorrectly initialized memory. (NOAA-GFDL#219) * fixes and workarounds for uninitialized memory in fv_regional_bc * remove workarounds and fix remaining known bugs in ps_reg * a few more surface pressure bug fixes; now the test case runs in debug mode * workarounds and bug fixes from gnu compiler testing * remove -9999999 commented-out code * quiet the NaNs passed to Atmp%ps * simplify comments and explain snan * use i-1 & j-1 for two-point averages, when available * Replace many changes with PR NOAA-GFDL#220 Co-authored-by: Rusty.Benson <rusty.benson@noaa.gov> Co-authored-by: Ted Mansell <37668594+MicroTed@users.noreply.github.com> Co-authored-by: Rusty Benson <6594772+bensonr@users.noreply.github.com> Co-authored-by: Samuel Trahan (NOAA contractor) <39415369+SamuelTrahanNOAA@users.noreply.github.com> Co-authored-by: Mark Potts <33099090+mark-a-potts@users.noreply.github.com>

NOAA-GFDL#200) * Regional bc blend changes to extend into interior halos and overlap on corners. Still not working for u and v. dyn_core.F90 : Fix from Jun Wang to correct sync of u,v fv_regional_bc.F90 : add check for nrows_blend > tile size; fix error when nrows_blend=1 Conflicts (not taken during chery-pick): driver/SHiELD/atmosphere.F90 model/fv_sg.F90 Co-authored-by: Ted Mansell <37668594+MicroTed@users.noreply.github.com>

MicroTed added 5 commits May 1, 2022 12:19

Add missing instance for hailwat

c372898

Regional bc blend changes to extend into interior halos and overlap …

b5bcdca

…on corners. Still not working for u and v.

atmosphere.F90 : add hailwat to check

4da0dba

dyn_core.F90 : Fix from Jun Wang to correct sync of u,v fv_regional_bc.F90 : add check for nrows_blend > tile size; fix error when nrows_blend=1

Merge remote-tracking branch 'origin/dev/emc' into regional_bc_test

8f5d1f8

Explanatory comment added

1580749

laurenchilutti requested review from bensonr, junwang-noaa and laurenchilutti May 27, 2022 17:41

laurenchilutti reviewed May 27, 2022

View reviewed changes

MicroTed added 2 commits June 1, 2022 15:59

Removed commented code

51ddcf1

Clean old code

442d61d

laurenchilutti approved these changes Jun 3, 2022

View reviewed changes

bensonr approved these changes Jun 3, 2022

View reviewed changes

This was referenced Jun 6, 2022

Add a new suite definition file to FV3 (FV3_WoFS_v0) for SRW release ufs-community/ufs-weather-model#1168

Closed

Some different MPI layouts in regional does not reproduce results ufs-community/ufs-weather-model#649

Closed

junwang-noaa approved these changes Jun 8, 2022

View reviewed changes

This was referenced Jun 9, 2022

fixes for regional decomposition test NOAA-EMC/ufsatm#549

Closed

fixes for regional decomposition test ufs-community/ufs-weather-model#1263

Closed

Much slower timings in model init with the latest ufs-weather-model ufs-community/ufs-weather-model#801

Closed

SamuelTrahanNOAA mentioned this pull request Jun 15, 2022

Fixes for reproducibility, failed builds, and CI. More hrrr tests, shorter rap&hrrr forecast lengths. ufs-community/ufs-weather-model#1257

Merged

17 tasks

SamuelTrahanNOAA added a commit to SamuelTrahanNOAA/ufs-weather-model that referenced this pull request Jun 15, 2022

Point to regional decomposition bugfix NOAA-GFDL/GFDL_atmos_cubed_sph…

ead2781

…ere#194

SamuelTrahanNOAA mentioned this pull request Jun 17, 2022

Fix RAP (Grell Freitas) decomp b4b issues NOAA-EMC/ufsatm#553

Merged

laurenchilutti merged commit 49f30ee into NOAA-GFDL:dev/emc Jun 17, 2022

MicroTed deleted the regional_bc_test branch June 21, 2022 17:12

JiliDong-NOAA added a commit to JiliDong-NOAA/GFDL_atmos_cubed_sphere that referenced this pull request Jun 23, 2022

Merge pull request #13 from NOAA-GFDL/dev/emc

949ca85

Regional decomposition test fix (when nrows_blend > 0) (NOAA-GFDL#194)

laurenchilutti mentioned this pull request Jul 7, 2022

Cherry-pick of PR 194 to main #200

Merged

6 tasks

Conversation

MicroTed commented May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junwang-noaa commented May 27, 2022

Uh oh!

laurenchilutti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laurenchilutti left a comment

Choose a reason for hiding this comment

Uh oh!

laurenchilutti commented Jun 8, 2022

Uh oh!

junwang-noaa commented Jun 8, 2022

Uh oh!

MicroTed commented Jun 8, 2022

Uh oh!

junwang-noaa commented Jun 8, 2022

Uh oh!

SamuelTrahanNOAA commented Jun 15, 2022

Uh oh!

laurenchilutti commented Jun 16, 2022

Uh oh!

SamuelTrahanNOAA commented Jun 16, 2022

Uh oh!

SamuelTrahanNOAA commented Jun 16, 2022

Uh oh!

jkbk2004 commented Jun 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

MicroTed commented May 26, 2022 •

edited

Loading