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.

Sign up for GitHub

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

Jump to bottom

Feature/ascii #1089

Merged
merged 7 commits into from
Oct 12, 2023
Merged

Feature/ascii #1089

merged 7 commits into from
Oct 12, 2023

Conversation

mickaelaccensi
Copy link
Collaborator

Pull Request Summary

new feature to output out_grd.ww3, out_pnt.ww3 and mod_def.ww3 both in binary and ascii format using switch ASCII.

Description

new output is created in ascii files out_grd.ww3.txt, out_pnt.ww3.txt and mod_def.ww3.txt when activating cpp key W3_ASCII.

Please also include the following information:

  • Add any suggestions for a reviewer
  • Mention any labels that should be added: bug, documentation, enhancement, new feature
  • Are answer changes expected from this PR? Please describe the changes and the reason why in addition to which of the following labels would apply: mod_def change, out_grd change, out_pnt change, restart file change, Regression test
    -->

Issue(s) addressed

no issue since it is a new feature

Commit Message

new feature to output out_grd.ww3, out_pnt.ww3 and mod_def.ww3 both in binary and ascii format using switch ASCII.

Check list

@JessicaMeixner-NOAA and @MatthewMasarik-NOAA , the version should be updated with this feature, isn't ?

Testing

  • How were these changes tested? matrix
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) ww3_tp2.6 and mww3_test_09
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)? on going
  • Please indicate the expected changes in the regression test output, (Note the list of known non-identical tests.) nothing
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):
    asap

@mickaelaccensi
implement ascii output for
8e151a6 
out_grd.ww3, out_pnt.ww3, mod_def.ww3
using CPP key ASCII
@mickaelaccensi
add regtest in ww3_tp2.6 and mww3_test_09
57e330b 
update matrix.base
@ukmo-ccbunney
Copy link
Collaborator

Nice @mickaelaccensi - this looks very useful for locating bit-4-bit reproducibility issues in output files.

@MatthewMasarik-NOAA
Copy link
Collaborator

Hi @mickaelaccensi, thanks, I look forward to reviewing this. I'm at the Wave workshop right now, so I can start the regtests though the review time frame is the beginning next week.

@ukmo-ccbunney
Copy link
Collaborator

I'm running the regtests on our HPC. I will try the new ASCII regtest on a variety of compilers once the main regtests suite has run.

@JessicaMeixner-NOAA
Copy link
Collaborator

@mickaelaccensi this is awesome and I think will be a great tool in helping us debug b4b issues! Thank you. One comment:

The ascii output is a debugging tool. I'm wondering if it'd be better to just simply locally define the file number for the output instead of adding the optional input arguments into so many routines? I don't know what others thoughts are on this, because the way you implmented it is the more "correct" way it just makes things a bit messier in terms of the optional cpp defined input to routines? Again - this is really exciting.

@JessicaMeixner-NOAA
Copy link
Collaborator

@mickaelaccensi I just kicked off regtests for the NOAA side of things. I'll let you know how it goes.

@ukmo-ccbunney
Copy link
Collaborator

I am seeing some failures in ww3_trnc (can't find mod_def.ww3 file).
They seem to be only happening in mww3_test_02...
I'll investigate.

@ukmo-ccbunney
Copy link
Collaborator

ukmo-ccbunney commented Oct 4, 2023 •
edited
Loading

I am seeing some failures in ww3_trnc (can't find mod_def.ww3 file). They seem to be only happening in mww3_test_02... I'll investigate.

Actually, I think this might be an existing bug in run_cmake_test - the section for ww3_trnc appears to be missing the code to link in the relevant mod_def.<GRD> file when running in multigrid mode. I am not sure why it was not happening before though! I'll try and confirm this and raise a separate issue.

@JessicaMeixner-NOAA
Copy link
Collaborator

@mickaelaccensi I also got an error for the test:

run_test -b slurm -o all -S -T -s MPI_ASCII -w work_MPI_ASCII -m grdset_a -f -p srun -n 24 ../model mww3_test_09

Processing /scratch1/NCEPDEV/climate/Jessica.Meixner/PR_WW3/pr1089/regtests/mww3_test_09/input/ww3_strt.inp
Screen output routed to /scratch1/NCEPDEV/climate/Jessica.Meixner/PR_WW3/pr1089/regtests/mww3_test_09/work_MPI_ASCII/ww3_strt_Huron.out

ERROR: Error occured during /scratch1/NCEPDEV/climate/Jessica.Meixner/PR_WW3/pr1089/regtests/mww3_test_09/work_MPI_ASCII/exe/ww3_strt execution

The error was:

 *** WAVEWATCH III ERROR IN W3IOGR :
     ERROR IN READING FROM mod_def.ww3 FILE
     IOSTAT =   67     MOD DEF FILE WAS GENERATED WITH A DIFFERENT
     WW3 VERSION OR USING A DIFFERENT SWITCH FILE.
     MAKE SURE WW3_GRID IS COMPILED WITH SAME SWITCH
     AS WW3_SHEL OR WW3_MULTI, RUN WW3_GRID AGAIN
     AND THEN TRY AGAIN THE PROGRAM YOU JUST USED.

I see that you haven't posted the matrix output from your tests, were yours successful? Can you post the matrix output.

@ukmo-ccbunney
Copy link
Collaborator

@JessicaMeixner-NOAA and @mickaelaccensi - do you have any errors regarding ww3_trnc execution in the multigrid tests? It seems that this is not working correctly in develop too.

grep -i error regtests/bin/matrix*.out

../../bin/matrix10.out:ERROR: Error occured during /home/d02/frey/WW3/REGS_GNU/regtests/mww3_test_02/work_PR3_UNO_a_c/exe/ww3_trnc execution
../../bin/matrix10.out:ERROR: Error occured during /home/d02/frey/WW3/REGS_GNU/regtests/mww3_test_02/work_PR3_UQ_a/exe/ww3_trnc execution
../../bin/matrix10.out:ERROR: Error occured during /home/d02/frey/WW3/REGS_GNU/regtests/mww3_test_02/work_PR3_UQ_a_c/exe/ww3_trnc execution

...and lots more...

@JessicaMeixner-NOAA
Copy link
Collaborator

JessicaMeixner-NOAA commented Oct 4, 2023 •
edited
Loading

I do not @ukmo-ccbunney do you get these in the develop branch as well? (I now re-read, you did see these in develop.)

@JessicaMeixner-NOAA JessicaMeixner-NOAA mentioned this pull request Oct 4, 2023
Closed
@ukmo-ccbunney
Copy link
Collaborator

ukmo-ccbunney commented Oct 4, 2023 •
edited
Loading

I do not @ukmo-ccbunney do you get these in the develop branch as well? (I now re-read, you did see these in develop.)

Yes I do. I think I can see what the problem is and I will raise a PR for it.
What confuses me is that you don't see the problem. The issue is that the section in run_cmake_test for running the ww3_trck and ww3_trnc programs in multi-grid mode does not link in the mod_def file, so it fails.

Do you have track.outer and ww3.196806_trck.nc files in mww3_test_02/work_PR3_UQ_a_c?

@mickaelaccensi
Copy link
Collaborator Author

@JessicaMeixner-NOAA , I also had the issue on mww3_test_09, it is not corrected.
I'm running the matrix.comp, I'll add it to the PR once it is done

@JessicaMeixner-NOAA
Copy link
Collaborator

Thanks @mickaelaccensi !

@MatthewMasarik-NOAA
Copy link
Collaborator

Hi @mickaelaccensi, were working on this now. Could you sync this branch since Chris's fix just merged?

@mickaelaccensi
Copy link
Collaborator Author

done. also merged for feature/GQM, if you can merge it also. Then I have the branch feature/ST4 which is waiting for those PR to be accepted. cheers

@MatthewMasarik-NOAA
Copy link
Collaborator

Awesome, thanks Mickael. I'll run the tests for feature/GQM now as well so it will be ready right behind this one.

I accidentally ran the regtests for this branch without the updates from #1091, so they are re-running right now. If there's no unexpected diffs, it should get through shortly today.

@MatthewMasarik-NOAA
Copy link
Collaborator

MatthewMasarik-NOAA commented Oct 10, 2023 •
edited
Loading

Hi @mickaelaccensi, I got the matrix results back, and I'm seeing a couple tests with differences (ww3_tp2.6, ww3_ufs1.2, mww3_test_04), you can see listed below. The differing files are mostly out_grd.*. Should these be expected?

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
## known non-b4b's
mww3_test_03/./work_PR1_MPI_e                     (1 files differ)
mww3_test_03/./work_PR2_UQ_MPI_e                     (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (16 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (7 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (13 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (15 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (13 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)
ww3_tp2.10/./work_MPI_OMPH                     (7 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (4 files differ)
ww3_ufs1.3/./work_a                     (3 files differ)
## 

-- unexpected non-b4b's
ww3_tp2.6/./work_ST0                    (3 files differ)
ww3_tp2.6/./work_ST4                    (3 files differ)
ww3_ufs1.2/./work_a                     (2 files differ)
ww3_ufs1.2/./work_c                     (3 files differ)
ww3_ufs1.2/./work_b                     (2 files differ)
mww3_test_04/./work_PR1_MPI_d           (3 files differ)
mww3_test_04/./work_PR2_UNO_MPI_d       (3 files differ)
mww3_test_04/./work_PR3_UNO_MPI_d_c     (3 files differ)
mww3_test_04/./work_PR3_UNO_MPI_d       (3 files differ)
mww3_test_04/./work_PR3_UQ_MPI_d_c      (3 files differ)
mww3_test_04/./work_PR3_UQ_MPI_d        (3 files differ)
mww3_test_04/./work_PR2_UQ_MPI_d        (3 files differ)
**********************************************************************
************************ identical cases *****************************
**********************************************************************

@JessicaMeixner-NOAA
Copy link
Collaborator

I think we might be seeing answer changes because of these code changes

image

@mickaelaccensi
Copy link
Collaborator Author

good catch. I've removed it and created an issue to check it later because I'm convinced the test on UST in w3iogomd.F90 is not correct. But it is not the goal of this branch

@JessicaMeixner-NOAA
Copy link
Collaborator

@mickaelaccensi thanks for this update. Last night I ran some regression tests that confirmed that part of the code is the cause of the issues we're seeing. I put your updates here: https://github.com/jessicameixner-noaa/ww3/tree/bug/initvalueust for that thest. Based on the diffs we were seeing before, I think you have found something that needs to be updated, but agree that it should be in a separate PR. In the meantime - I think this should resolve the outstanding issues with this branch and after some regtests today we should be able to merge this, which will hopefully help us figure out what's going on with the gqm branch and allow us to merge that soon too.

@MatthewMasarik-NOAA
Copy link
Collaborator

Thanks @mickaelaccensi for the updates, and @JessicaMeixner-NOAA for diagnosing the issue. I'll start the new regtests.

@JessicaMeixner-NOAA JessicaMeixner-NOAA mentioned this pull request Oct 12, 2023
Open
@MatthewMasarik-NOAA
Copy link
Collaborator

@mickaelaccensi there was a small fix that went in yesterday afternoon, could you sync this branch when you have chance?

MatthewMasarik-NOAA
MatthewMasarik-NOAA approved these changes Oct 12, 2023
View reviewed changes
Copy link
Collaborator

@MatthewMasarik-NOAA MatthewMasarik-NOAA left a comment

Choose a reason for hiding this comment

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

Code review
Pass

Feature check
Pass

Setting the ASCII switch will additionally create mod_def.*.txt, out_grd.*.txt, and out_pnt.*.txt alongside the standard binary versions. Two tests have been added to verify this. Sample directory listing from one of these (mww3_test_09/work_MPI_ASCII) demonstrates the functionality is working:

[WW3/regtests]$ file mww3_test_09/work_MPI_ASCII/out_* mww3_test_09/work_MPI_ASCII/mod_def*
mww3_test_09/work_MPI_ASCII/out_grd.Huron:     data
mww3_test_09/work_MPI_ASCII/out_grd.Huron.txt: ASCII text
mww3_test_09/work_MPI_ASCII/out_grd.Michi:     data
mww3_test_09/work_MPI_ASCII/out_grd.Michi.txt: ASCII text
mww3_test_09/work_MPI_ASCII/out_grd.Super:     data
mww3_test_09/work_MPI_ASCII/out_grd.Super.txt: ASCII text
mww3_test_09/work_MPI_ASCII/out_pnt.Huron:     data
mww3_test_09/work_MPI_ASCII/out_pnt.Huron.txt: ASCII text
mww3_test_09/work_MPI_ASCII/out_pnt.Michi:     data
mww3_test_09/work_MPI_ASCII/out_pnt.Michi.txt: ASCII text
mww3_test_09/work_MPI_ASCII/out_pnt.Super:     data
mww3_test_09/work_MPI_ASCII/out_pnt.Super.txt: ASCII text
mww3_test_09/work_MPI_ASCII/mod_def.Huron:     data
mww3_test_09/work_MPI_ASCII/mod_def.Huron.txt: ASCII text
mww3_test_09/work_MPI_ASCII/mod_def.Michi:     data
mww3_test_09/work_MPI_ASCII/mod_def.Michi.txt: ASCII text
mww3_test_09/work_MPI_ASCII/mod_def.Super:     data
mww3_test_09/work_MPI_ASCII/mod_def.Super.txt: ASCII text

Testing
Pass

Matrix results: No changes outside of the known non-b4b cases.

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR3_UNO_MPI_e                     (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (16 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (14 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (15 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (18 files differ)
mww3_test_03/./work_PR2_UQ_MPI_d2                     (13 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)
ww3_tp2.10/./work_MPI_OMPH                     (7 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (4 files differ)
ww3_ufs1.3/./work_a                     (3 files differ)
**********************************************************************
************************ identical cases *****************************
**********************************************************************

@MatthewMasarik-NOAA
Copy link
Collaborator

@mickaelaccensi thank you for this very useful feature!! I think it will make a big impact on debugging going forward.

@MatthewMasarik-NOAA MatthewMasarik-NOAA merged commit eff6686 into NOAA-EMC:develop Oct 12, 2023
20 checks passed
@MatthewMasarik-NOAA MatthewMasarik-NOAA mentioned this pull request Oct 12, 2023
Merged
4 tasks
This was referenced Oct 13, 2023
Closed
Merged
miguelsolanocordoba added a commit to wavespotter/WW3 that referenced this pull request Apr 19, 2024
@miguelsolanocordoba @ukmo-ccbunney
@mickaelaccensi @benoitp-cmc @MatthewMasarik-NOAA @Ghazal-Mohammadpour @JessicaMeixner-NOAA @Biao-Zhao @edwardhartnett @AlexanderRichert-NOAA @ErickRogers @DeniseWorthen @camilleanne
Re-base with NOAA-EMC and fix gustiness (#15)
6334796 
* 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
Reviewers

@JessicaMeixner-NOAA JessicaMeixner-NOAA JessicaMeixner-NOAA left review comments

@MatthewMasarik-NOAA MatthewMasarik-NOAA MatthewMasarik-NOAA approved these changes

@ukmo-ccbunney ukmo-ccbunney Awaiting requested review from ukmo-ccbunney

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mickaelaccensi @ukmo-ccbunney @MatthewMasarik-NOAA @JessicaMeixner-NOAA