Add option to use NetCDF output instead of binary for point output #1230
Conversation
things varied from binary version to add warnings etc
turning on tests
|
I'm running the matrix. I'll keep you informed |
|
@ukmo-ccbunney @mickaelaccensi - Please let me know if you are running into any issues with this that I might be able to help with debugging. |
|
matrix is done but I have both an error in develop and bin2nc branches for ww3_tp2.10 for output point processing due to the fact that the only requested output point (B45005) is considered on land. It is a SMC grid test so it is not linked to this PR. @ukmo-ccbunney and @ukmo-jianguo-li can probably double check this problem. I've checked at ww3_tp2.2 for the format of the out_pnt.ww3.nc, it would be great to have at least the time variable defined in the correct calendar format and standard instead of a 2d array of integers. I think using the functions in w3timemd.ftn it should not bet too complicated.. Anyway, I'm now running the matrix.comp. Once done I'll keep you informed. |
differences sound ok. tp2.17, ww2.21 and ww3_tp2.6 are due to pdlib not b4b Except the format of time variable in out_pnt.ww3.nc, I'm ok with this PR |
|
This recent SMC grid update has activated an old conversion factor VSC to reverse the depth values stored in the SMC grid cell array before they are assigned to the WW3 ZB variable:
!/SMC !Li Minimum DMIN depth is used as well for SMC.
!/SMC !! ZB(ISEA)= - MAX( DMIN, FLOAT( IJKCel(5, ISEA) ) )
!/SMC !Li Allow land cell to be defined by ZLIM value and only reset
!/SMC !Li MAPST* land values for sea points. JGLi03Nov2023
!/SMC ZB(ISEA)= DVSMC*FLOAT( IJKCel(5, ISEA) )
The DVSMC is a copy of the VSC conversion factor, which is specified in the w3_grid.inp file on the old depth input file line:
-0.1 10.0 30 -1. 1 1 '(....)' 'NAME' 'S36125Depth.dat'
Where the fourth argument -1.(0) Is for the VSC. This factor is not defined in the namelist input file and it results in all wet points are converted into land points. The namelist input file in tp2.10 test needs to be updated for this factor. If the old ww3_grid.inp file is used, it should be fine.
From: Mickael Accensi ***@***.***>
Sent: Friday, May 31, 2024 8:17 AM
To: NOAA-EMC/WW3 ***@***.***>
Cc: JianGuo Li ***@***.***>; Mention ***@***.***>
Subject: Re: [NOAA-EMC/WW3] Add option to use NetCDF output instead of binary for point output (PR #1230)
matrix is done but I have both an error in develop and bin2nc branches for ww3_tp2.10 for output point processing due to the fact that the only requested output point (B45005) is considered on land. It is a SMC grid test so it is not linked to this PR. @ukmo-ccbunney<https://github.com/ukmo-ccbunney> and @ukmo-jianguo-li<https://github.com/ukmo-jianguo-li> can probably double check this problem.
I've checked at ww3_tp2.2 for the format of the out_pnt.ww3.nc, it would be great to have at least the time variable defined in the correct calendar format and standard instead of a 2d array of integers. I think using the functions in w3timemd.ftn it should not bet too complicated..
Anyway, I'm now running the matrix.comp. Once done I'll keep you informed.
-
Reply to this email directly, view it on GitHub<#1230 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANGV3EHWHKKWZLMEAUVVJOLZFAPXFAVCNFSM6AAAAABHUSEINOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBRGM3TCOJUGI>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
|
thanks @ukmo-jianguo-li for the exclanation. I'll let you create an issue for this so. |
@mickaelaccensi The time format is what WW3 is expecting to read in and then use in the next routines. I'm thinking we just add an additional time variable as you requested instead of changing this one, thoughts? |
Thanks Mickael - I'll raise an Issue for this. |
It seems to be a solution. To do it, you would need to rename the current time variable as another name (like time_ww3) and create a new time variable with the calendar format. It would be great for future use the out_grd.ww3.nc |
|
@mickaelaccensi and @ukmo-ccbunney I have added the extra time variable as @mickaelaccensi requested. Let me know what you think. @ukmo-ccbunney I think I have addressed your concerns of things not working, but I still see changes requested, so if I missed something or things are still not working for you please let me know! |
|
@JessicaMeixner-NOAA, thanks first to have added the TIME variable. There is just an issue in the TIME values. See screenshot
|
|
Okay i just went to debug this and check this out and i somehow deleted a few lines of code. Should have a fix here in an hour or so. Thanks for catching this @mickaelaccensi ! |
|
@mickaelaccensi time should be fixed now! Please let me know if you see issues still. |
|
I confirm the bugfix ! |
|
@ukmo-ccbunney - are things working for you on your end? |
|
All regtests ran ok with just the usual differences in mww3_test_03. 👍 |
|
Thanks @ukmo-ccbunney! @MatthewMasarik-NOAA - I think this is ready for your final review now! |
Great, thanks for confirming, I have the review in progress now. |
There was a problem hiding this comment.
Code review
Pass
Testing
Pass
- Compared with
developbranch:
**********************************************************************
********************* 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_UQ_MPI_e (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_e (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2 (17 files differ)
mww3_test_03/./work_PR1_MPI_d2 (14 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c (13 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c (16 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2 (15 files differ)
mww3_test_03/./work_PR2_UQ_MPI_d2 (15 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_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.1/./work_unstr_b (1 files differ)
ww3_ufs1.1/./work_unstr_a (1 files differ)
ww3_ufs1.1/./work_unstr_c (1 files differ)
ww3_ufs1.3/./work_a (1 files differ)
**********************************************************************
************************ identical cases *****************************
**********************************************************************
matrixCompFull.txt
matrixDiff.txt
matrixCompSummary.txt
- Compared with
feature/pointbinary2nc_hardcodedbranch:
hardcoded.matrixCompFull.txt
hardcoded.matrixDiff.txt
hardcoded.matrixCompSummary.txt
Approved.
|
A big thank you to @JessicaMeixner-NOAA, who took over the reins and saw this work to completion, and @edwardhartnett who initiated the work and laid the foundation. This is a great contribution in converting WW3 binary outputs to netcdf! Thanks also goes to @mickaelaccensi and @ukmo-ccbunney for testing, and giving feedback that improved the final PR. |
* feature/gpu/w3srce_refactor: Enable doxygen documentation in the cmake build system (NOAA-EMC#1281) Simplify MPI ifdefs in subroutine W3MPIO (NOAA-EMC#1266) Add depth scaling value to SMC regression tests. (NOAA-EMC#1264) Updates to NCEP regtests for Orion Rocky9 OS(NOAA-EMC#1263) Fix code stability issue in ww3_outp (NOAA-EMC#1258) Fix GNU regtest CI failure (NOAA-EMC#1253) Add option to use NetCDF output instead of binary for point output (NOAA-EMC#1230)
Pull Request Summary
Add a switch BIN2NC which will write a NetCDF file instead of a binary file for point output.
Description
When using BIN2NC, the point output will now be written in NetCDF from ww3_shel or ww3_multi. The exes for output post-processing will be able to read these files. Some notes:
Co-authors are @edwardhartnett and @MatthewMasarik-NOAA
Please also include the following information:
Issue(s) addressed
Commit Message
Add option to use NetCDF output instead of binary for point output
Check list
Testing
New tests:
ww3 ww3_tp2.2/work_PR1_MPI_BIN2NC
And updates to ww3_ufs1.1 unstructured tests as they will now use this feature. Note the results do not chnage, just binary -> netcdf for the points, and the switch file in the netcdf output.
I ran two sets of tests, one from this branch compared with develop:
matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt
And then I ran tests hard-coding the option to use BIN2NC by default (https://github.com/JessicaMeixner-NOAA/WW3/tree/feature/pointbinary2nc_hardcoded) to ensure all the various combinations were tested and ignoring "0 diffs" in the output the changes are as expected as well:
matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt