Update subroutine W3IOPO to read/write NetCDF files (point output binary -> netcdf) #682
Comments
|
@JessicaMeixner-NOAA can I have one of the data files that are read/written by w3iopo()? |
|
Some detail from Matt on running the tests:
Outline of basic regtest steps |
@edwardhartnett this comment got lost. @MatthewMasarik-NOAA have you already shared this info or do we need to set up 1-2 regtests for Ed for this? (one for per time-snap writing) |
|
@JessicaMeixner-NOAA, I've shared info with @edwardhartnett for running regtests. I think I will be meeting with him this afternoon for some follow up. |
|
@MatthewMasarik-NOAA sounds good. It might be helpful to pick a single regression test that could be a test to run instead of running all of the regression tests. Have you shared with @edwardhartnett which program makes the output of w3iopo and which reads it as well? Please let me know if you need anything additional from me. |
|
OK, I was able to run (at least one) regression test on my local ubuntu laptop. To do that, follow the steps in the .github/workflows/gnu.yml file to install dependencies with spack and build. Then follow Matthew's steps above to run the regression test. I had to do:
Then I ran one of the tests and got (I believe) the expected output: I would be helpful to know if this particular test uses w3iopo(). Is there any easy way to tell from the output? Or do all tests use w3iopo()? If someone could point me to a reasonably small output file, that would be very helpful. Did my regtest produce one? If so, where would it be? |
|
@edwardhartnett this is great that you were able to run a regression tests. /home/ed/WW3/regtests/mww3_test_05/work_ST4_PR2_UQ_MPI/out_pnt.points is the output of w3iopo . This test isn't running any point output which would then read this file. While we'll eventually want/need all regtests to run, perhaps it'll be easier to choose a very small simple regression test, that we know has point output and is fairly small. @MatthewMasarik-NOAA any ideas which one would be a great candidate for this, otherwise I can look in about an hour. |
|
Yes, I was being loose when I said we went over regression testing. We did pick just one test as a small, single test case. The output Ed posted is from that test case, and it has point output, too. Was there something else you had in mind for the test case? |
|
I only saw gridded post-procssing output, not point post processing in @edwardhartnett log. I might chose a single versus multi-grid test to start with personally, but this test certainly will work. I do think we will eventually need to make sure we get some sort of point post-processing added here. |
|
So we have the full set of regression tests, but personally I think starting with something simpler (ie ww3_shel versus ww3_multi) might be a good idea, plus this test has point output (ie it will also read the output from w3iopo), and this test is:
For multi-grid, we have 2 options:
Another option which is admittedly not well documented is to have each point output write it's own time-snap file. You then have to process each time snap differently as it's own out_pnt.ww3 file renamed from the time-stamped file. This is currently done in the global-wrokflow but not in one of the regression tests here. This is the first step toward improving that process. Please let me know if you have any additional questions or things I can help with. |
|
Thanks for this very helpful info, @JessicaMeixner-NOAA! |
|
OK, here's a simple question, when I run run_cmake_test, what is the actual binary file that is executed? In other words, how do I use a debugger on the regression test code? |
|
OK, so I was able to run Jessica's suggested test: |
|
In the regtest output is says: I didn't compile the model with these switches. Is that a problem? |
Several executables are executed in the run_cmake_test script for the various parts of the code. If it'd be helpful we can create a canned case to run just the executables needed to run to write and then read the binary out_pnt.ww3 files
There are several ways to do this. I usually hard-code that cmake is set to debug instead of release. We can set up a meeting to walk you through this if we do not set-up the more limited canned case for you.
There are likely several data structures that are input to the routine as well as those that are in module files.
The run_cmake_test compiles the code. For what you are doing the switches are not as important so shouldn't matter. Sounds like a meeting would be helpful to discuss these and more questions. |
|
@JessicaMeixner-NOAA I am available to meet at any time you think it would be helpful. Meanwhile, I will try and figure out where the existing code is that reads/writes these data files... |
|
@edwardhartnett and I just tried to meet, unfortunately my GFE mic was acting up, so we had to cancel. We were possiibly going to meet tomorrow now instead. Maybe the 3 of use could meet |
|
Happy to join the meeting |
|
OK, great. Meanwhile any specific information about what is being run with run_cmake_test that is doing the I/O would be most helpful, I am running it now to try and figure out where to start the debugger, but some specific info would be helpful. Is there any documentation of what is happening? |
|
When I try and run the tests again I get: Is there a way to reset them and run them again? |
|
Sure, i can give some info on run_cmake_test |
|
The program see's the work_* directory and skips it. If you remove |
run_cmake_test is doing a complete simulation as in: build all (pre/run/post) executables, then run preprocessing programs, the actual run, post processing. It is all streamlined into one call. To do just the run (which is where w3iopo() is called) we can set you up with a canned case. |
|
Well by all means, set me up with anything you think is helpful. I don't know what you mean by canned case. ;-) I will continue to dig into the code. |
|
OK from a grep I see that some of the read code can be found in model/src/gx_outp.F90. There is some I/O set up going on here. Presumably this code is called by the regtest somewhere. Here's some I/O setup from gx_outp.F90: Looks like these have to be called before w3iopo()... |
|
I just sent some info to the email thread. gx_outp.F90 does call w3iopo(), but that is the standalone GrADS point output routine, and is not the call to w3iopo we are interested in. We are interested in the w3wave call. |
|
From Matt's email, answers to question about what binary is calling the I/O subroutine w3iopo().
|
|
@edwardhartnett try removing ww3_tp1.1/work_PR1 before re-trying. |
|
@edwardhartnett I recommended using the call string @JessicaMeixner-NOAA had posted, You used a different signature that will not look for the MPI switch, so it won't be compiled with MPI, but later in the string the |
|
Duh. That worked better... |
|
@edwardhartnett Just wanted to check in to see if you had any ideas of things for us to try for reading in 2D variables in W3IOPON_READ? |
|
I have made a lot of progress, but am still failing at the end of the tp1.1 regtest due to the ww3_outp code which reads the points file, and then apparently re-reads it for some testing purpose (around line 790). My current work can be see in edwardhartnett#37. Please do not attempt to modify the IO code on other branches, it has changed a bit in the past week. The read of the point data, unlike the write, always reads the entire file, so there is no need to slice the time dimension in the read code, just in the write code, which writes out a timestep at a time. Part of the problem, which I fixed this morning, is that the read code needs to call w3dmo2() to allocate a bunch of the arrays to read into. I have tests running on GitHub but these are slow; for interactive development I'm using a local version of Jenkins and have automated testing running quickly enough to be used in interactive development. What would be helpful would be one or more namelist settings to control whether netCDF is used. Then I could run tests with and without that namelist and directly compare results to ensure the netCDF files contain the correct data. However, I can take a stab at this later when I get tp1.1 working OK... |
|
@edwardhartnett - I will be working on the namelist settings, but before we add that, we plan to test all regression tests with the netCDF and see if things match with the binary as a first set of tests. Sounds like I can grab your branch and potentially try that? And if that goes well I'll start adding in the input control for netcdf vs binary. |
|
@edwardhartnett I'm about to start some tests with your branch, but in the meantime I wanted to follow up about not being able to read slices as it seems like we can only read parts of variables according to https://docs.unidata.ucar.edu/netcdf-fortran/current/f90-variables.html
|
|
NetCDF can read the entire variable at once, as well as slices, or strided slices. In Fortran, just leave out the optional start/count/stride parameters, and netCDF will get (or put) the entire variable at once. |
|
Thanks for clarifying @edwardhartnett ! I think for this routine as it is now, we might want to only grab slices as that's how the binary read worked. I'm running a test with your branch and the ww3_outp job is running, but it's taking much longer than expected. It's taking me about 2 hours (and counting) and with the binary the whole job is only a few minutes (I don't have an exact number). Is this expected? |
|
No that is not expected. Let me look at the binary read code again to see what it does... |
|
@edwardhartnett am about to do the same myself. Let me know if it'd be useful to look at anything together. I'll be sure to chime in with anything I learn here. |
|
@edwardhartnett I'm still getting long run times running your branch, however I pulled in the change from your branch to CALL W3DMO2 and I'm now able to read 2D variables! I pushed my updates here: https://github.com/JessicaMeixner-NOAA/WW3/tree/feature/pointbinary2nc and will keep you up to date on progress. |
|
It's not clear to me whether you are intending to merge your code or my code, but I am only testing my code and don't plan to attempt to merge your code back into mine. There are significant changes to the code. |
|
@edwardhartnett I looked at the differences between our two branches. I couldn't find all of the updates I made in your branch and when i was having trouble figuring out what was going on, I just went back to my branch. I have not yet merged in your changes as it didn't have the time dimension changes I made in the read routine. At this point, the changes from your branch that are not included in mine are mostly the change in error handling, which can be added via merge or other updates. I added time in the reading of my branch, which pointed out that your branch is stuck in an infinite reading loop, and my branch fails with an error when time runs out. I need to add a quick check to see if we have another time step to read, and if not set the flag to -1. I should have that update shortly and will share. |
|
It's not clear that it's helpful to run manual tests on code that is not quite the code I am working on. Some things that would be helpful:
|
|
@edwardhartnett great news on my end - I'm able to run the regression test successfully from my branch! My next steps will be:
I think we're at a great spot and it's really exciting! Thank you so much for all of your help!!! |
|
@edwardhartnett here's my current status:
Will likely not have an update until much later today or early tomorrow, but I'll keep you posted. |
|
@edwardhartnett working on merging your changes to my branch and want to confirm that https://github.com/edwardhartnett/WW3/tree/ejh_develop is the correct branch to merge in. Thanks in advance for the help! |
|
@edwardhartnett I'm running some new tests with the full set of regression tests because I had some issues with input yesterday, but nothing related to this branch itself. I'd like to start working on merging your changes into my branch, so I'm hoping you can confirm which branch has your latest changes. Your help is greatly appreciated! |
|
My latest changes are on ejh_develop. |
|
Thanks @edwardhartnett ! I'll share the branch after I've reply back here after I have merged in the changes and done some testing. |
|
@edwardhartnett More good news! I have merged in your changes to my branch here: https://github.com/JessicaMeixner-NOAA/WW3/tree/feature/pointbinary2nc and found and fixed some FLX5 bugs. A few spot checks look good, so my next steps are to run the full set of regtests overnight on hera, then look into a few clean-up things (remove un-used variables from the write for example), after which I will then re-run regtests and then lastly work on adding the namelist control capability. I'll keep you posted! |
|
@edwardhartnett I ran the regression tests overnight and mostly had good results. There are a few regression tests that failed and I'm looking into their causes as I in parallel work on some clean-up items. |
|
Hi @edwardhartnett good news from regression tests last night: All ran without error and matched the output from the develop branch (excpet known not b4b ones). I'm going to make a couple of minor updates and then add control to turn on/off, but I think we're good to go. Do you plan any other updates for the unit tests? Thanks again for all of your help with this!!! |
* 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 <fabrice.ardhuin@ifremer.fr> * 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 <denise.worthen@noaa.gov> * 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 <jessica.meixner@noaa.gov> * 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 <48915820+ukmo-ccbunney@users.noreply.github.com> Co-authored-by: Mickael Accensi <49198861+mickaelaccensi@users.noreply.github.com> Co-authored-by: Benoit Pouliot <51411504+benoitp-cmc@users.noreply.github.com> Co-authored-by: Matthew Masarik <86749872+MatthewMasarik-NOAA@users.noreply.github.com> Co-authored-by: Ghazal-Mohammadpour <124626872+Ghazal-Mohammadpour@users.noreply.github.com> Co-authored-by: Jessica Meixner <jessica.meixner@noaa.gov> Co-authored-by: Biao Zhao <zhaobiaodeyouxiang@163.com> Co-authored-by: Edward Hartnett <38856240+edwardhartnett@users.noreply.github.com> Co-authored-by: Alex Richert <82525672+AlexanderRichert-NOAA@users.noreply.github.com> Co-authored-by: Fabrice Ardhuin <fabrice.ardhuin@ifremer.fr> Co-authored-by: W. Erick Rogers <156342000+ErickRogers@users.noreply.github.com> Co-authored-by: Denise Worthen <denise.worthen@noaa.gov> Co-authored-by: Camille Teicheira <cteicheira@gmail.com>
|
I have no further plans for testing. |
github-project-automation
bot
moved this from In Progress
to Done
in WW3 Output Tasks for GFSv17
Is your feature request related to a problem? Please describe.
Update the subroutine w3iopo in model/src/w3iopomd.F90 to read/write netcdf instead of binary files.
Describe the solution you'd like
Have point output be netcdf instead of binary from the model forecast.
Describe alternatives you've considered
Have point output files be optionally binary or netcdf. This is not impossible but does add a layer of complexity that is not ideal. At one points binary files were preferred due to size and speed, but perhaps those concerns can be overcome or will be outweighed by the benefit of having more readable files. Additionally there was concerns about creating a requirement to have NetCDF and that some users might not have access to this. However, when this was first mentioned was about 5 years ago and NetCDF is a very common library so perhaps that concern is also no longer a reason to not pursue only NetCDF files.
Additional context
This perhaps makes the post processing in ww3_ounp partially redundant. This should also help in refactoring ww3_outp.
The text was updated successfully, but these errors were encountered: