-
Notifications
You must be signed in to change notification settings - Fork 548
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
R21B in IC4 #1176
Conversation
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.
@ErickRogers Thanks for making this PR! A few things to note:
|
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
The conflict is resolved, @JessicaMeixner-NOAA |
I will add the new regtests to matrix.base, @JessicaMeixner-NOAA |
Adding IC4M8 and IC4M9 regtests (one each) to matrix.base
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.)
6bf12c9
to
e02ea98
Compare
@JessicaMeixner-NOAA @MatthewMasarik-NOAA, I think it is ready to go now. |
@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. |
Right Jessica, sorry @MathieuDutSik please disregard, I meant to tag @MatthewMasarik-NOAA |
@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. |
@ErickRogers can you confirm that this looks as expected, with the differences being:
And full log files: The "unexpected" differences are:
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. |
@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. < 7.424531, 7.883871, 8.097491, 8.154648, 8.00843, 7.535123, 6.881038, 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? |
@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 |
@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. |
Quick sanity check that the new tests reproduce themselves passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ErickRogers !
* 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]>
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:
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