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

R21B in IC4 #1176

Merged
merged 6 commits into from
Jan 23, 2024
Merged

Conversation

ErickRogers
Copy link
Contributor

@ErickRogers ErickRogers commented Jan 17, 2024

Pull Request Summary

Introduce IC4M8 and IC4M9 to WW3

Description

This introduces IC4M8 and IC4M9 to WW3.
It should not change outcome of any WW3 runs which do not use the new features. (Reverse compatible.)
However, it does include two non-default options to aspects of IC4 that are not M8 or M9.
Namely:

  • option for background dissipation at low frequencies
  • option for using a larger number of steps in IC4M6 (was 10, and is now 16)

Some comments about "IC4 other than IC4M8/IC4M9 " are also changed/updated in the header of w3sic4md.F90.

Verbose descriptions of IC4M8 and IC4M9 can be found in header of w3sic4md.F90.

regtests to demo the new feature:
cd $REGTESTDIR
./bin/run_test -i input_IC4_M8 -w work_IC4_M8 $MODELDIR ww3_tic1.1
./bin/run_test -i input_IC4_M9 -w work_IC4_M9 $MODELDIR ww3_tic1.1

ER: I need to describe the feature in the manual. I added it to my to-do list.

Issue(s) addressed

This is for issue #1167

Commit Message

Introduce IC4M8 and IC4M9 to WW3

Check list

Testing

  • How were these changes tested? 2 new regression tests were added, see here: d4640ab
  • Are the changes covered by regression tests? Yes, see prior bullet.
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)? No.
  • Please indicate the expected changes in the regression test output. No changes expected.
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt): N/A. Matrix was not run.

I have ported my changes from COAMPS/WW3 (which is v6.07) to my fork, branch "1167-R21B-in-IC4" (which is version 7.14+).
Changes are made to 4 of the WW3 fortran files.'
I needed to do the merge manually, since there are a lot of formatting changes (etc.) going from 6.07 to 7.14.
Adding regtest for IC4M8 and IC4M9
Examples:
cd $REGTESTDIR
./bin/run_test -i input_IC4_M8 -w work_IC4_M8 $MODELDIR ww3_tic1.1
./bin/run_test -i input_IC4_M9 -w work_IC4_M9 $MODELDIR ww3_tic1.1

I also edited the info file.

I did not include ww3_shel.nml for these regtests, since this feature is apparently broken. I did include the other .nml files.
(Specifics about the thing that is broken: the option to have homogeneous input field in ww3_shel.nml. Either it is broken in the code, or it is broken in the examples provided for IC4M1, IC4M2, IC4M3, IC4M4, IC4M5, IC4M7. Regtest IC4M6 *does* run with ww3_shel.nml, since it does not use any homogeneous input field. Failure mode: seg fault)
In any case, regtest with -N with these new regtests will only run if you exclude ww3_shel.nml using -r or -q options of run_test.

Regtests w/out -N *do* work, for all cases.
@JessicaMeixner-NOAA
Copy link
Collaborator

@ErickRogers Thanks for making this PR! A few things to note:

  • there's a conflict with the develop branch, so you'll need to merge in the latest develop into your branch.
  • It looks like we should be adding some new tests to matrix.base but I don't see that here? Let me know if you need any help with that. Also I'm going to add back some testing information in the PR template that we'll get you to fill in as well.

Merge from develop.
There was a conflict in a single line.
Someone added “,END=801,ERR=802,IOSTAT=IERR” on an IC4-related line.
Conflict resolved by incorporating both edits.

Merge remote-tracking branch 'upstream/develop' into 1167-R21B-in-IC4
@ErickRogers
Copy link
Contributor Author

The conflict is resolved, @JessicaMeixner-NOAA

@ErickRogers
Copy link
Contributor Author

I will add the new regtests to matrix.base, @JessicaMeixner-NOAA

Adding IC4M8 and IC4M9 regtests (one each) to matrix.base
@ErickRogers
Copy link
Contributor Author

I added the new regtests to matrix.base, @JessicaMeixner-NOAA

…date the documentation in fortran code and regtest to be consistent with naming convention used in the manual (using the 2021a 2021b convention outside the manual is risky because Latex assigns the a and b and it may not be what one has in mind, so I don't use that anymore.)
@ErickRogers
Copy link
Contributor Author

ErickRogers commented Jan 22, 2024

@JessicaMeixner-NOAA @MatthewMasarik-NOAA, I think it is ready to go now.

@JessicaMeixner-NOAA
Copy link
Collaborator

@ErickRogers I think you meant to tag @MatthewMasarik-NOAA not Matheiu. I actually had started some regression tests last week. Unforutunatley the queue is very busy and I'm still waiting for the last of it to run. I'll post those results when I get them and then will retest with the latest updates you've shared here.

@ErickRogers
Copy link
Contributor Author

Right Jessica, sorry @MathieuDutSik please disregard, I meant to tag @MatthewMasarik-NOAA

@ErickRogers
Copy link
Contributor Author

@JessicaMeixner-NOAA, thanks for starting the runs. Today's changes are just to the manual and comments, so those don't require any re-run. But some of the ones last week could of course affect the matrix outcome. Like my additions to the matrix file. But OTOH, the additions to the matrix file cannot be apply to the baseline branch (dev), since they use the new feature.

@JessicaMeixner-NOAA
Copy link
Collaborator

@ErickRogers can you confirm that this looks as expected, with the differences being:

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR1_MPI_e                     (1 files differ)
mww3_test_03/./work_PR3_UQ_MPI_e_c                     (1 files differ)
mww3_test_03/./work_PR3_UNO_MPI_e                     (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_e                     (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (16 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (10 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (16 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (15 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (12 files differ)
mww3_test_03/./work_PR2_UQ_MPI_d2                     (15 files differ)
mww3_test_03/./work_PR3_UQ_MPI_e                     (1 files differ)
mww3_test_03/./work_PR3_UNO_MPI_e_c                     (1 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (15 files differ)
mww3_test_09/./work_MPI_ASCII                     (0 files differ)
ww3_tic1.1/./work_IC4_M5                     (2 files differ)
ww3_tic1.1/./work_IC4_M6                     (2 files differ)
ww3_tic1.1/./work_IC4_M6L                     (2 files differ)
ww3_tic1.1/./work_IC4_M3                     (2 files differ)
ww3_tic1.1/./work_IC4_M6H                     (2 files differ)
ww3_tic1.1/./work_IC4_M7                     (2 files differ)
ww3_tic1.1/./work_IC4_M4                     (2 files differ)
ww3_tic1.1/./work_IC4_M8                     (directory not found)
ww3_tic1.1/./work_IC4_M2                     (2 files differ)
ww3_tic1.1/./work_IC4_M9                     (directory not found)
ww3_tic1.1/./work_IC4_M1                     (2 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (7 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (4 files differ)
ww3_tp2.6/./work_ST4_ASCII                     (0 files differ)
ww3_ufs1.3/./work_a                     (3 files differ)

And full log files:
matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt

The "unexpected" differences are:

ww3_tic1.1/./work_IC4_M5                     (2 files differ)
ww3_tic1.1/./work_IC4_M6                     (2 files differ)
ww3_tic1.1/./work_IC4_M6L                     (2 files differ)
ww3_tic1.1/./work_IC4_M3                     (2 files differ)
ww3_tic1.1/./work_IC4_M6H                     (2 files differ)
ww3_tic1.1/./work_IC4_M7                     (2 files differ)
ww3_tic1.1/./work_IC4_M4                     (2 files differ)
ww3_tic1.1/./work_IC4_M8                     (directory not found)
ww3_tic1.1/./work_IC4_M2                     (2 files differ)
ww3_tic1.1/./work_IC4_M9                     (directory not found)
ww3_tic1.1/./work_IC4_M1                     (2 files differ)

Which are your 2 new tests plus a few other diffs in tic1.1 tests. These diffs only appear to be the mod_defs and ww3_grid.out. My only curiosity was why just these tests, but likely this is all okay. I will do a quick double check on the two new tests with the version of the code + comment changes as a quick sanity check and to make sure they reproduce themselves. Then we should be ready to merge this in.

@ErickRogers
Copy link
Contributor Author

ErickRogers commented Jan 23, 2024

@JessicaMeixner-NOAA thanks again for running the tests. The changes listed as "unexpected" look OK to me. Re: "why just these tests" ... I added new variables to the IC4 namelist, so they show up as changes to mod-def and grid.out. But the differences only will appear in the regtests where the IC4 switch is used. So that is expected/OK. And as you noticed, they don't affect the actual results, which is also expected.
I'm puzzled about the differences in the .nc files, like /mww3_test_03/work_PR2_UNO_MPI_d2/ww3.196806.nc_diff.txt ... like...

< 7.424531, 7.883871, 8.097491, 8.154648, 8.00843, 7.535123, 6.881038,
> 7.424531, 7.883871, 8.097491, 8.154648, 8.00843, 7.535123, 6.881037,

Yes, these differences are tiny, but I wonder why we see even tiny differences. Is that normal? Is this caused by some randomness in WW3, or in netcdf?

@JessicaMeixner-NOAA
Copy link
Collaborator

@ErickRogers the mww3_test_03/*_d2 are known to have differences (https://github.com/NOAA-EMC/WW3/wiki/How-to-use-matrix.comp-to-compare-regtests-with-develop#4-look-at-results). For more details please see: #144

@ErickRogers
Copy link
Contributor Author

@JessicaMeixner-NOAA ....OK, thanks for the info. I get it now that you were implicitly grouping those as "not unexpected differences" i.e. expected differences.

@JessicaMeixner-NOAA
Copy link
Collaborator

Quick sanity check that the new tests reproduce themselves passed.

Copy link
Collaborator

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

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

Thanks @ErickRogers !

@JessicaMeixner-NOAA JessicaMeixner-NOAA merged commit 9a718fc into NOAA-EMC:develop Jan 23, 2024
miguelsolanocordoba added a commit to wavespotter/WW3 that referenced this pull request Apr 19, 2024
* Bugfix - initialised VD and VS to zero in w3srcemd. (NOAA-EMC#1037)

* More efficient test for binary files in matrix.comp (NOAA-EMC#1035)

* Tidy up of pre-processor directives and unused variables in w3srcemd.F90 (NOAA-EMC#1010)

* Correct typo in w3srcemd.F90 pre-processor directive. (NOAA-EMC#1039)

* minor bugfix for matrix grepping on keywords (NOAA-EMC#1049)

* Stop masking group 1 output where icec > icen (NOAA-EMC#1019)

* Doxygen documentation added, 8th subset.(NOAA-EMC#1046)

* NC4 ,F90 ,XX0 switches removed from ww3_tp2.19 regtest (NOAA-EMC#1054)

* CI:  Fix for Intel scripts. GNU scripts updated. (NOAA-EMC#1064)

* correct the computation of QP parameter, add QKK output parameter, change UST scale factor (NOAA-EMC#1050)

* correct issue with ww3_multi when requesting restart2 and using nml file instead of inp file (NOAA-EMC#1070)

* correct calendar for track netcdf output (NOAA-EMC#1079)

* Fix missing mod_def.ww3 file in multigrid regression tests for track output (NOAA-EMC#1091)

* STAB3: fix cmake build for ST4 or ST3 (NOAA-EMC#1086)

* new feature to output out_grd.ww3, out_pnt.ww3 and mod_def.ww3 both in binary and ascii format using switch ASCII. (NOAA-EMC#1089)

* Update local unit number arrays (NDS, MDS) to be same size of array defined in w3odatmd (size=15). Also, defined unit numbers for NDS(14) and NDS(15). (NOAA-EMC#1098)

* Removed code referencing PHIOC in output section for PHICE in ww3_ounf (NOAA-EMC#1093)

* implementation of the GQM (Gaussian Quadrature Method) to replace the DIA in NL1 or NL2. (NOAA-EMC#1083)

* update logic to ensure you are not accessing uninitialized dates (NOAA-EMC#1114)

* Initialised S and D arrays in W3SDB1 before potential early return if zero energy. (NOAA-EMC#1115)

* ww3_ounp.F90:  x/y units attribute corrected from 'm' to 'km' (NOAA-EMC#1088)

* Bugfix: Assign unit numbers to ASCII gridded/point output in multi-grid mode. (NOAA-EMC#1118)

* correct bugs to run correctly GQM implementation (NOAA-EMC#1127)

* Adding documentation to w3iopo() in preparation for code for NOAA-EMC#682. (NOAA-EMC#1131)

* NCEP regtest module updates: uses spack-stack/1.5.0, includes scotch/7.0.4 (NOAA-EMC#1137)

* Minor update to ncep regtests (NOAA-EMC#1138)

* Updated intel workflow to install oneapi compilers from new location. (NOAA-EMC#1157)

* Add unit test for points I/O code. (NOAA-EMC#1158)

* Update Intel CI (relocate /usr/local; ensure intel-oneapi-mpi; use ubuntu-latest) (NOAA-EMC#1161)

* remove lookup table for ST4 to speed up computation and clean up the ST4 code (NOAA-EMC#1124)

Co-authored-by: Fabrice Ardhuin <[email protected]>

* initialize USSP_WN for mod_def (NOAA-EMC#1165)

* Introduce IC4M8 and IC4M9 to WW3 (NOAA-EMC#1176)

* clean up and add ST4 variables (NOAA-EMC#1181)

* w3fld1md.F90: fix divide by zero in CRIT2 parameter (NOAA-EMC#1184)

* ww3_prnc.F90: fix out-of-scope grid index write statement (NOAA-EMC#1185)

* Bugfix: address potential divide-by-zero in APPENDTAIL (NOAA-EMC#1188)

Co-authored-by: Denise Worthen <[email protected]>

* Provide initial drying of cells with depth < ZLIM for SMC grid. (NOAA-EMC#1192)

* Output OMP threading info to screen when running ww3_shel/ww3_multi compiled with the OMPG switch. Also fixes truncation of build.log when running run_cmake_build. (NOAA-EMC#1191)

* Added screen output showing number of threads when OMP enabled.

* update build to get more info in logs (NOAA-EMC#46)

---------

Co-authored-by: Jessica Meixner <[email protected]>

* update run_cmake_test to catch build errors and exit (NOAA-EMC#1194)

* fix merge conflicts

* Fix gustiness bug, as suggst by Pieter

* Change USTARsigma to WAM implementation

---------

Co-authored-by: Chris Bunney <[email protected]>
Co-authored-by: Mickael Accensi <[email protected]>
Co-authored-by: Benoit Pouliot <[email protected]>
Co-authored-by: Matthew Masarik <[email protected]>
Co-authored-by: Ghazal-Mohammadpour <[email protected]>
Co-authored-by: Jessica Meixner <[email protected]>
Co-authored-by: Biao Zhao <[email protected]>
Co-authored-by: Edward Hartnett <[email protected]>
Co-authored-by: Alex Richert <[email protected]>
Co-authored-by: Fabrice Ardhuin <[email protected]>
Co-authored-by: W. Erick Rogers <[email protected]>
Co-authored-by: Denise Worthen <[email protected]>
Co-authored-by: Camille Teicheira <[email protected]>
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.

None yet

2 participants