-
Notifications
You must be signed in to change notification settings - Fork 542
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
Update NetCDF comp/link #336
Update NetCDF comp/link #336
Conversation
@JessicaMeixner-NOAA i reviewed it, all look ok to me. Why did you revert 8 to 24 again? are we going to do it in a separate PR? |
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.
I approve it, but please merge it once the matrix is done. Thanks.
@mickaelaccensi I had to update the makefile in regtests/ww3_tp2.14/input/oasis3-mct/util/make_dir/cmplr.tmpl to get this to work. This should work for you too, but thought maybe I should double check? |
@aliabdolali the regtests now all run, I'll compare with the develop branch when I started and then will re-do this with the top of develop as well. Are there other PRs coming in soon that I should wait before I run these again? |
The divider is going in before this one. I will add the regression test comparison in a few minutes. |
Awesome! I will gladly wait for that as it will make this 10000000x easier! Thanks @aliabdolali |
regtests are running now |
Regtests have finished, running matrix.comp now. ufs-weather-model WW3 tests on hera.intel also passed. |
The wind.ww3 binary file is different for test case: ww3_tp2.15 both work_5km and work_MPI_5km Otherwise the differences are as usual on NCEP machines: ********************* non-identical cases **************************** mww3_test_03/./work_PR2_UQ_MPI_d2 (7 files differ) |
@aliabdolali I'm going to run the ww3_tp2.15 test again from the develop and this branch to see if this difference was introduced in this PR or before. |
@JessicaMeixner-NOAA This is introduced in a PR from UKMET and in these cases, we expected changes, but do not expect them to remain. So I'll go ahead and merge your work if you agree and will coordinate with them in that regard. |
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.
I'm not able to compile WW3 anymore and it is related to this PR. I've detailed the problems below
NETCDF_INCLUDE = $(shell $(NETCDF_CONFIG) --includedir) | ||
NETCDF_LIBRARY = $(shell $(NETCDF_CONFIG) --flibs) | ||
NETCDF_INCLUDE = $(shell $(NETCDF_CONFIG) --cflags) | ||
NETCDF_LIBRARY = $(shell $(NETCDF_CONFIG) --flibs) $(shell $(NETCDF_CONFIG) --libs) |
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.
If the netcdf-fortran library is correctly compiled, all the needed dependencies are defined in --flibs.
the option --libs does not exist for all netcdf libraries (like version 4.5.2) so it crashes.
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.
@mickaelaccensi we have an issue now to make the manual specifically say nc-config consistently: #392 and any other checks to make sure things are working for everyone are welcome. Sorry this got pushed through while you were out and is now causing issues.
@@ -30,8 +30,8 @@ COUPLE = $(WWATCH3_DIR)/../regtests/ww3_tp2.14/input/oasis3-mct | |||
ARCHDIR = $(WWATCH3_DIR)/../regtests/ww3_tp2.14/work_oasis3-mct | |||
# | |||
# NetCDF library | |||
NETCDF_INCLUDE = $(shell $(NETCDF_CONFIG) --includedir) | |||
NETCDF_LIBRARY = $(shell $(NETCDF_CONFIG) --flibs) | |||
NETCDF_INCLUDE = $(shell $(NETCDF_CONFIG) --cflags) |
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.
--cflags is for c-compiler
for fortran-compiler, it must be --fflags to be used
@@ -116,7 +116,7 @@ | |||
case $WWATCH3_NETCDF in | |||
NC3) libs="$libs -L$NETCDF_LIBDIR -lnetcdf" ;; | |||
NC4) if [ "$mpi_mod" = 'no' ]; then comp="`$NETCDF_CONFIG --fc`"; fi | |||
libs="$libs `$NETCDF_CONFIG --flibs`" ;; | |||
libs="$libs `$NETCDF_CONFIG --flibs` `$NETCDF_CONFIG --libs`" ;; |
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.
If the netcdf-fortran library is correctly compiled, all the needed dependencies are defined in --flibs.
the option --libs does not exist for all netcdf libraries (like version 4.5.2) so it crashes.
@@ -82,7 +82,8 @@ | |||
case $WWATCH3_NETCDF in | |||
NC3) opt="$opt -I$NETCDF_INCDIR" ;; | |||
NC4) if [ "$mpi_mod" = 'no' ]; then comp="`$NETCDF_CONFIG --fc`"; fi | |||
opt="$opt -I`$NETCDF_CONFIG --includedir`" ;; | |||
#opt="$opt -I`$NETCDF_CONFIG --includedir`" ;; | |||
opt="$opt `$NETCDF_CONFIG --cflags`" ;; |
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.
--cflags is for c-compiler
for fortran-compiler, it must be --fflags to be used
store nodes status for tidal analysis to output the list by NAPOUT
Pull Request Summary
This is a more general way of including the WW3 libraries.
Description
Updated the way NetCDF libraries are including in the comp/link (thanks to @aerorahul help). In addition the matrix_ncep is updated to use the NOAA hpc-stack libraries.
Issue(s) addressed
Check list
Testing