Skip to content
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

fix angles on Bu for use by CICE #922

Merged

Conversation

DeniseWorthen
Copy link
Contributor

@DeniseWorthen DeniseWorthen commented Mar 15, 2024

Remove find_anq routine and instead calculate Bu angles using the Ct angle on the opposite side of the seam.
Add angchk to verify CICE internally calculated Ct angle against both MOM6 and CICE model output
Remove resetting of longitudes to 0:360
Add tripole:tripole bilinear mapping for creation of CICE IC downscaling and add required Bu<->Ct mapping on tripole grid
Replace existing unit test on Bu angles by providing 5x5 blocks of input, finding anglet, angle and angchk and comparing known (good) output for MOM6 and CICE6.

TESTS CONDUCTED:

If there are changes to the build or source code, the tests below must be conducted. Contact a repository manager if you need assistance.

  • Compile branch on all Tier 1 machines using Intel (Orion, Jet, Hera, Hercules and WCOSS2). Done using 9caec86.
  • Compile branch on Hera using GNU. Done using 5da3dc6.
  • Compile branch in 'Debug' mode on WCOSS2. Done using 5da3dc6.
  • Run unit tests locally on any Tier 1 machine. Done on Hercules using 5da3dc6.
  • Run relevant consistency tests locally on all Tier 1 machine.

This PR will change baselines for all resolutions.

  • The tripole master file at each resolution will now contain two additional fields, the Bu angle and the angchk array.
  • The previous code added an increment of 360.00 in longitude to the MOM6 longitudes, which have values from -300 to 60. This was done only for convenience, but it was found to produce a slight anomaly in the right-hand pole longitude value.
  • Additional mapping files are generated in order to address the requirement of downscaling CICE ICs from higher to lower resolutions.

A test baseline was created using a feature branch containing only the additional angle variables and the removal of the 360deg increment. The reg tests were run for the feature/fixangleBu branch against this test baseline. The following files did not compare due to the change in the Bu angle fields.

20:Comparing grid_cice_NEMS_mx025.nc........NOT OK
38:Comparing tripole.mx025.nc........NOT OK
58:Comparing grid_cice_NEMS_mx050.nc........NOT OK
74:Comparing tripole.mx050.nc........NOT OK
94:Comparing grid_cice_NEMS_mx100.nc........NOT OK
107:Comparing tripole.mx100.nc........NOT OK
128:Comparing grid_cice_NEMS_mx500.nc........NOT OK
138:Comparing tripole.mx500.nc........NOT OK

DOCUMENTATION:

  • Documentation has been updated. The doxygen was generated on Cactus using 5da3dc6. There we no warnings. It was visualized on rzdm and looked good.

ISSUE:

* remove find_anq routine and instead calculate Bu angles using
the Ct angle on the opposite side of the seam.
* add angchk to verify CICE internally calculated Ct angle against
both MOM6 and CICE model output
* remove resetting of longitudes to 0:360
* add  tripole:tripole bilinear mapping for creation of CICE IC
downscaling and add required Bu<->Ct mapping on tripole grid
@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA I have to rebuild my unit test since I'v re-written the routine it tested.

@GeorgeGayno-NOAA
Copy link
Collaborator

@DeniseWorthen is there anything I can do to help with this PR?

@DeniseWorthen
Copy link
Contributor Author

No, thanks. I'm just trying to get a new unit test running since I've basically replaced the code that the previous unit test was using. I've almost got it working now.

DeniseWorthen and others added 9 commits March 22, 2024 10:43
* tripole, grid_cice and
* grid cice and tripole files are not b4b
* need to allow for non-full dimensioned arrays being
sent to angles routines
* only cice grid files, tripole files change
* need to clean up messaging and angchk test
* 5x5 blocks of grid points are used to calc angle variables
and checked against known output from mom6 and cice6 history
files
* send array bounds to angles routines. This allows non-calculated
values within the unit test to return as values 0.0 instead of
non-sensical values
* baselines pass against a bl created from addanglevar branch
* only cice grid and tripole files differ
@DeniseWorthen DeniseWorthen marked this pull request as ready for review March 25, 2024 17:45
@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA This PR will require all new baselines for all platforms, but it is ready for review.

@GeorgeGayno-NOAA
Copy link
Collaborator

Just a minor comment about an unused variable:

/work2/noaa/da/ggayno/save/UFS_UTILS.denise/sorc/cpld_gridgen.fd/gen_fixgrid.F90(52): remark #7712: This variable has not been used.   [JJ]
  integer :: ii,jj
----------------^

@GeorgeGayno-NOAA
Copy link
Collaborator

@DeniseWorthen do you have a new set of baseline files or should I create them from your branch?

@DeniseWorthen
Copy link
Contributor Author

I need to generate them. I probably have them on hera, but I'd prefer to regenerate.

@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA I've generated the following baselines

WCOSS2: /lfs/h2/emc/stmp/denise.worthen/CPLD_GRIDGEN/BASELINE
HERA: /scratch1/NCEPDEV/stmp4/Denise.Worthen/CPLD_GRIDGEN/BASELINE
HERCULES: /work/noaa/stmp/dworthen/CPLD_GRIDGEN/BASELINE
ORION: /work/noaa/stmp/dworthen/CPLD_GRIDGEN/BASELINE

For Jet, I got a failure the first time which I seemed like a system error? I'm re-running Jet now.

@DeniseWorthen
Copy link
Contributor Author

Baselines on Jet were successful on 2nd try

/lfs4/HFIP/h-nems/Denise.Worthen/CPLD_GRIDGEN/BASELINE/

@GeorgeGayno-NOAA
Copy link
Collaborator

Baselines on Jet were successful on 2nd try

/lfs4/HFIP/h-nems/Denise.Worthen/CPLD_GRIDGEN/BASELINE/

Great. I will host the files in the official location, then merge.

* gnu and gnu debug triggered several errors previously not
caught using intel and intel debug
@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA I've pushed a couple of minor fixes from testing w/ GNU on hercules. I've also created a new baseline on hercules at /work/noaa/stmp/dworthen/CPLD_GRIDGEN/20240402.BASELINE. I've run against that baseline twice and was able to reproduce.

I spot-checked the mx100 and mx050 against the ufs_utils.hercules/reg_tests/cpld_gridgen files and I do see some differences, but small. Could you see if you have any trouble reproducing the baseline I created?

@GeorgeGayno-NOAA
Copy link
Collaborator

@GeorgeGayno-NOAA I've pushed a couple of minor fixes from testing w/ GNU on hercules. I've also created a new baseline on hercules at /work/noaa/stmp/dworthen/CPLD_GRIDGEN/20240402.BASELINE. I've run against that baseline twice and was able to reproduce.

I spot-checked the mx100 and mx050 against the ufs_utils.hercules/reg_tests/cpld_gridgen files and I do see some differences, but small. Could you see if you have any trouble reproducing the baseline I created?

I am able to reproduce your new baseline! Please recreate your baselines on Orion, Jet, Hera and WCOSS2. Then, I can do a final test., then merge.

@DeniseWorthen
Copy link
Contributor Author

Great. I'm doing one final consistency check and then I'll start the remaining platforms.

@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA I've satisfied myself that the angchk, anglet and angle variables for the new baselines are as expected compared to the current develop branch. So I'll go ahead and start generating on the other platforms.

@DeniseWorthen
Copy link
Contributor Author

Hera baseline : /scratch1/NCEPDEV/stmp4/Denise.Worthen/CPLD_GRIDGEN/BASELINE
Orion baseline : /work/noaa/stmp/dworthen/CPLD_GRIDGEN/BASELINE

@DeniseWorthen
Copy link
Contributor Author

JET: /lfs4/HFIP/h-nems/Denise.Worthen/CPLD_GRIDGEN/BASELINE
WCOSS2: /lfs/h2/emc/stmp/denise.worthen/CPLD_GRIDGEN/BASELINE/

@GeorgeGayno-NOAA
Copy link
Collaborator

Using 993c670 I am able to reproduce your baselines on Hercules, WCOSS2 and Hera. However, I am still getting those annoying small differences on Jet and Orion. And on Jet, I get occasional seg faults.

Location of your branch:

  • Jet: /lfs4/HFIP/emcda/George.Gayno/ufs_utils.git/UFS_UTILS.denise/reg_tests/cpld_gridgen
  • Orion: /work/noaa/da/ggayno/save/ufs_utils.git/UFS_UTILS.denise/reg_tests/cpld_gridgen

I moved your baselines here:

  • Jet: /lfs4/HFIP/hfv3gfs/emc.nemspara/role.ufsutils/ufs_utils/reg_tests/cpld_gridgen/baseline_data/*.new
  • Orion: /work/noaa/nems/role-nems/ufs_utils/reg_tests/cpld_gridgen/baseline_data/*new

@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA This is puzzling. Let me do some work on Orion and Jet and see if I can reproduce on those platforms.

@DeniseWorthen
Copy link
Contributor Author

DeniseWorthen commented Apr 8, 2024

@GeorgeGayno-NOAA I'm seeing a lot of intermittent issues. For example, on Orion even if the baseline for one resolution passes, the following job, at the next lower resolution, is never submitted. It seems to disappear into the ether. But by launching the rt.sh again, the job succeeds. On Jet, the first time I ran this morning the job hung.

Right now, I'm testing bumping the memory up on Jet and increasing the wall clock time everywhere. I don't see that either of these should really be necessary, but at least on Jet I've been able to run against the baseline several times now. Is this all down to Rocky8? I'll keep you posted.

@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA Just to be sure, the mom6 fix files used right now are set in the cpld_gridgen.sh rt.sh script. For example,

export MOM6_FIXDIR=/scratch1/NCEPDEV/global/glopara/fix/mom6/20220805

There is a second directory there (20231219) but those files should not be any different. I don't think that is causing any issue, but I'm wondering if I should update those to the later directory.

@DeniseWorthen
Copy link
Contributor Author

On Jet, I have been able to reproduce the baseline at least 4 times using

diff --git a/reg_tests/cpld_gridgen/rt.sh b/reg_tests/cpld_gridgen/rt.sh
index 70d11c04..63b0fcc9 100755
--- a/reg_tests/cpld_gridgen/rt.sh
+++ b/reg_tests/cpld_gridgen/rt.sh
@@ -108,9 +108,9 @@ TESTS_FILE="$PATHRT/rt.conf"
 export TEST_NAME=

 # for C3072 on hera, use WLCLK=60 and MEM="--exclusive"
-WLCLK_dflt=20
+WLCLK_dflt=35
 export WLCLK=$WLCLK_dflt
-MEM_dflt="--mem=12g"
+MEM_dflt="--mem=16g"
 export MEM=$MEM_dflt

 cd $PATHRT
@@ -170,7 +170,7 @@ elif [[ $target = jet ]]; then
     NCCMP=nccmp
     PARTITION=xjet
     ulimit -s unlimited
-    WLCLK=25
+    WLCLK=35
     SBATCH_COMMAND="./cpld_gridgen.sh"
 fi

@DeniseWorthen
Copy link
Contributor Author

I am going to see if I can reproduce everywhere using the bump in memory and time.

@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA On WCOSS2, I don't think the new baselines haven't been staged correctly. They look to be pointing to the Dec 20 baselines.

lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:34 025 -> 025.8639547
lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:34 050 -> 050.8639547
lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:34 100 -> 100.8639547
lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:35 500 -> 500.8639547

@GeorgeGayno-NOAA
Copy link
Collaborator

@GeorgeGayno-NOAA On WCOSS2, I don't think the new baselines haven't been staged correctly. They look to be pointing to the Dec 20 baselines.

lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:34 025 -> 025.8639547
lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:34 050 -> 050.8639547
lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:34 100 -> 100.8639547
lrwxrwxrwx 1 emc.nems nems   11 Apr  5 19:35 500 -> 500.8639547

Try it now.

@GeorgeGayno-NOAA
Copy link
Collaborator

I just retested your branch (9caec86) on Jet and Orion. I still get these annoying differences. The location of your checked out branch is:

Jet: /lfs4/HFIP/emcda/George.Gayno/ufs_utils.git/UFS_UTILS.denise/reg_tests/cpld_gridgen
Orion: /work/noaa/da/ggayno/save/ufs_utils.git/UFS_UTILS.denise/reg_tests/cpld_gridgen

I would like to point to your executable from my work space. And I wonder if we are loading different libraries at runtime.

@DeniseWorthen
Copy link
Contributor Author

I've been running a sanity check this morning everywhere using a clean checkout of the branch. I've gotten jet,orion and hercules and hera to pass. I haven't started WCOSS2 yet.

On orion, my latest checkout and test are in:

/work/noaa/marine/dworthen/utils_dw
/work/noaa/stmp/dworthen/CPLD_GRIDGEN/rt_106867

Both Hercules and Orion write tests to the same location. So my latest Hercules test is

/work/noaa/nems/dworthen/utils_dw/
/work/noaa/stmp/dworthen/CPLD_GRIDGEN/rt_1639913

@DeniseWorthen
Copy link
Contributor Author

DeniseWorthen commented Apr 10, 2024

@GeorgeGayno-NOAA So the differences are in the masked ocean mask files. Are we pointing to different orog fix files?

In /work/noaa/stmp/ggayno/CPLD_GRIDGEN/rt_443522/025, grid.nml shows fv3dir=/work/noaa/da/ggayno/save/ufs_utils.git/UFS_UTILS.denise/fix/orog'

which is

ls -l /work/noaa/da/ggayno/save/ufs_utils.git/UFS_UTILS.denise/fix/orog
lrwxrwxrwx 1 ggayno da 43 Feb 15  2023 /work/noaa/da/ggayno/save/ufs_utils.git/UFS_UTILS.denise/fix/orog -> /work/noaa/global/glopara/fix/orog/20220805

In my /work/noaa/stmp/dworthen/CPLD_GRIDGEN/rt_106867/025, grid.nml shows
fv3dir='/work/noaa/marine/dworthen/utils_dw/fix/orog'

which is

ls -l /work/noaa/marine/dworthen/utils_dw/fix/orog
lrwxrwxrwx 1 dworthen marine 43 Apr 10 06:48 /work/noaa/marine/dworthen/utils_dw/fix/orog -> /work/noaa/global/glopara/fix/orog/20231027

@GeorgeGayno-NOAA
Copy link
Collaborator

@DeniseWorthen I was pointing to some wrong 'fix' files. Everything passes on all machines for me. Sorry for the confusion. I will merge.

Copy link
Collaborator

@GeorgeGayno-NOAA GeorgeGayno-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Able to successfully run the regression tests against the new baselines. Everything looks good. Will merge.

@DeniseWorthen
Copy link
Contributor Author

@GeorgeGayno-NOAA Great. Thanks for working through it with me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rotation angles on corner grid points are wrong in cpld_gridgen
2 participants