Improve NetCDF detection and linking #1923

rjdave · 2023-10-12T19:01:59Z

TYPE: enhancement

KEYWORDS: NetCDF4, Spack-Stack, gfortran

SOURCE: David Robertson, Rutgers University

DESCRIPTION OF CHANGES:
Problem:

NetCDF4 detection and linking is unreliable and impossible if NetCDF-C and NetCDF-Fortran reside in different directories
gfortran is intolerant of non-initialized variables in mediation_integrate.F.

Solution:

Refactor the detection and configuration of NetCDF dependencies and add the capability to use NetCDF-C and NetCDF-Fortran libraries that don't reside in the same directory by introducing a new variable for the NetCDF-C path.
The fname and n2 intent(out) variables are not assigned under certain circumstances so we should initialize them to empty strings.

   ierr = 0
   fname = ""
   n2 = ""

   ! Note that computation of fname and n2 are outside of the oid IF statement

LIST OF MODIFIED FILES: list of changed files (used git diff --name-status release-v4.5.2 to get formatted list)

M       Makefile
M       arch/Config.pl
M       arch/postamble
M       arch/preamble
M       configure
M       share/mediation_integrate.F

TESTS CONDUCTED:

Compiled and ran with several different configurations of NetCDF, including both shared and static versions of both split (NetCDF-C and NetCDF-Fortran in separate directories) and unified library locations. The shared library versions were from Spack-Stack; the static libraries were custom-built for my systems.
Ran the coupled WRF-ROMS standard test case for Hurricane Irene.
The Jenkins tests are all passing.

RELEASE NOTE: This PR improve the detection and linking with shared and static configurations of the NetCDF-4 libraries (split or unified) inside and outside Spack-Stack.

weiwangncar · 2023-10-12T21:35:49Z

The results of regression tests:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

rjdave · 2023-10-13T19:15:34Z

Moved minor variable initialization in share/mediation_integrate.F into this PR from #1924 as requested.

Should I add the notes about share/mediation_integrate.F to the original PR above and remove from #1924?

Hopefully splitting these PRs as requested multiple times will finally get these necessary changes into WRF and help broaden the environments WRF can be compiled under.

weiwangncar · 2023-10-13T20:06:26Z

@rjdave Thanks for adding the two line change here. Yes, please add relevant info to this PR.

weiwangncar · 2023-10-16T20:43:57Z

@islas Can you review this PR and recommend if it should be included in 4.5.2 release. Thanks!

islas · 2023-10-24T23:10:44Z

configure

@@ -918,7 +882,7 @@ EOF
      echo "           Fortran compiler is $SFC_arch"
      echo "              It will build in $netcdf_arch"
      echo " "
-      if [ -e $NETCDF/bin/nc-config ] ; then
+      if [ -e $NETCDF_C/bin/nc-config ] ; then
        echo "NetCDF version: ${ncversion}"
        echo "Enabled NetCDF-4/HDF-5: `nc-config --has-nc4`"


I believe this will output the results from the wrong nc-config if $NETCDF_C differs from default in PATH

Most of these changes resolve #1742 and having consistent usage of nf-config and nc-config fall under the aim of this PR

Pinging @wkliao to be aware that this should be the PR that resolves issues noted in #1742

You're right, I missed that one. I will push the appropriate change in a few minutes.

Most of these changes resolve #1742 and having consistent usage of nf-config and nc-config fall under the aim of this PR

Pinging @wkliao to be aware that this should be the PR that resolves issues noted in #1742

If this PR #1923 resolved issues raised by PR #1742 and beyond, then #1742 can be closed. Given #1742 was created 1 and 1/2 years ago, I wonder if there is anything it missed, so it could not be merged.

I think the primary thing #1742 did not have was the ability to use both nf-config and nc-config for different library link info. This probably wasn't a high priority then, but netCDF's shift to split the two programs even further as of v4.9.2 will soon be problematic for WRF's current make builds. (see Unidata/netcdf-c#2619 and Unidata/netcdf-c v4.9.2 release notes)

islas · 2023-10-24T23:18:10Z

@islas Can you review this PR and recommend if it should be included in 4.5.2 release. Thanks!

Definitely something to include with 4.5.2 as the latest releases of netCDF further separate nc-config and nf-config, thus builds should aim to follow suit

weiwangncar · 2023-12-08T13:45:17Z

@rjdave Can you add weiwangncar as your collaborator? We need to resolve a code conflict when merging your code into the repository. Thanks.

rjdave · 2023-12-08T21:35:32Z

@rjdave Can you add weiwangncar as your collaborator? We need to resolve a code conflict when merging your code into the repository. Thanks.

I will have to see if we have any spare slots since this is in an organization and not my personal GitHub.

islas · 2023-12-08T21:38:43Z

I will have to see if we have any spare slots since this is in an organization and not my personal GitHub.

There should be an option on the pull request to "Allow edits by maintainers", if not resolving the merge conflict within your repo should be doable as well. The option should be on the righthand column of the PR below all the reviewers, assignees, labels, and notifications.

weiwangncar · 2023-12-09T01:27:49Z

@rjdave If you prefer, you can make the change as it is in PR-1743. It is a one line change for line # 1072 in Makefile.

rjdave · 2023-12-11T15:52:45Z

I see that adding [ -z "$(USENETCDFPAR)" ] || to my if line will resolve the conflict. However, I'm not entirely sure why I have double quotes around $(NETCDF4_DEP_LIB) for a false USENETCDFPAR but not when USENETCDFPAR is true, Let me see if configure completes without the quotes. If not, they will almost certainly be necessary in the else as well. Is it best to make the necessary "quotes" changes with the "Resolve conflicts" button here in this PR or should I commit the changes to my feature branch?

weiwangncar · 2023-12-12T01:45:48Z

@rjdave When you use the 'Resolve Conflicts' button to make a change, the change will be committed to your feature branch.

…and_linking

weiwangncar · 2023-12-12T02:48:14Z

The latest regression test results:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

weiwangncar · 2023-12-12T02:49:13Z

@islas Since this PR has been updated, it requires your review again. Specifically it removed quotes from the next line. Thanks.

weiwangncar · 2023-12-15T12:09:46Z

@islas Are you ok with this PR? Thanks!

TYPE: enhancement KEYWORDS: NetCDF4, Spack-Stack, gfortran SOURCE: David Robertson, Rutgers University DESCRIPTION OF CHANGES: Problem: - NetCDF4 detection and linking is unreliable and impossible if NetCDF-C and NetCDF-Fortran reside in different directories - gfortran is intolerant of non-initialized variables in **`mediation_integrate.F`**. Solution: - Refactor the detection and configuration of NetCDF dependencies and add the capability to use NetCDF-C and NetCDF-Fortran libraries that don't reside in the same directory by introducing a new variable for the NetCDF-C path. - The **fname** and **n2** `intent(out)` variables are not assigned under certain circumstances so we should initialize them to empty strings. ``` ierr = 0 fname = "" n2 = "" ! Note that computation of fname and n2 are outside of the oid IF statement ``` LIST OF MODIFIED FILES: list of changed files (used `git diff --name-status release-v4.5.2` to get formatted list) ``` M Makefile M arch/Config.pl M arch/postamble M arch/preamble M configure M share/mediation_integrate.F ``` TESTS CONDUCTED: 1. Compiled and ran with several different configurations of NetCDF, including both shared and static versions of both split (NetCDF-C and NetCDF-Fortran in separate directories) and unified library locations. The shared library versions were from Spack-Stack; the static libraries were custom-built for my systems. 2. Ran the coupled WRF-ROMS standard test case for Hurricane Irene. 3. The Jenkins tests are all passing. RELEASE NOTE: This PR improve the detection and linking with shared and static configurations of the NetCDF-4 libraries (split or unified) inside and outside Spack-Stack.

Imporve NetCDF detection and linking

afca44f

rjdave requested a review from a team as a code owner October 12, 2023 19:01

hga007 mentioned this pull request Oct 13, 2023

Remove arbitrary limiting of heat fluxes #1924

Merged

Adding minor share/mediation_integrate.F change to branch

3f6dd87

rjdave requested a review from a team as a code owner October 13, 2023 19:09

islas reviewed Oct 24, 2023

View reviewed changes

Two more nc-config path corrections

33e218e

islas previously approved these changes Oct 27, 2023

View reviewed changes

islas added the release-v4.5.2 label Nov 1, 2023

islas mentioned this pull request Dec 7, 2023

Should WRF/Makefile pass the NETCDF4_DEP_LIB to external/io_netcdf/makefile? #1945

Open

weiwangncar previously approved these changes Dec 8, 2023

View reviewed changes

wkliao mentioned this pull request Dec 8, 2023

configure: check nc-config and nf-config #1742

Closed

Merge branch 'release-v4.5.2' into feature/improve_netcdf4_detection_…

ee2889e

…and_linking

rjdave dismissed stale reviews from weiwangncar and islas via ee2889e December 12, 2023 01:50

islas approved these changes Dec 15, 2023

View reviewed changes

weiwangncar approved these changes Dec 16, 2023

View reviewed changes

weiwangncar merged commit bb406d2 into wrf-model:release-v4.5.2 Dec 16, 2023

islas mentioned this pull request Dec 19, 2023

Remove unneeded redundant HDF5 linking #1935

Closed

rjdave deleted the feature/improve_netcdf4_detection_and_linking branch January 3, 2024 15:20

This was referenced Jun 12, 2024

Update wrf.py for 4.5.2 easybuilders/easybuild-easyblocks#3354

Closed

{geo}[intel/2023a] WRF 4.5.2, WPS 4.5 easybuilders/easybuild-easyconfigs#20779

Open

ocaisa mentioned this pull request Nov 29, 2024

pass netCDF-Fortran path via $NETCDFF_DIR in WPS easyblock easybuilders/easybuild-easyblocks#3522

Merged

stefan-wolfsheimer mentioned this pull request Nov 29, 2024

{geo}[foss/2024a] WRF 4.6.1, WPS 4.6.0 easybuilders/easybuild-easyconfigs#21938

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve NetCDF detection and linking #1923

Improve NetCDF detection and linking #1923

rjdave commented Oct 12, 2023 •

edited by weiwangncar

Loading

weiwangncar commented Oct 12, 2023

rjdave commented Oct 13, 2023 •

edited

Loading

weiwangncar commented Oct 13, 2023

weiwangncar commented Oct 16, 2023

islas Oct 24, 2023

islas Oct 24, 2023

rjdave Oct 25, 2023

wkliao Dec 8, 2023

islas Dec 8, 2023

islas commented Oct 24, 2023

weiwangncar commented Dec 8, 2023

rjdave commented Dec 8, 2023

islas commented Dec 8, 2023 •

edited

Loading

weiwangncar commented Dec 9, 2023

rjdave commented Dec 11, 2023

weiwangncar commented Dec 12, 2023

weiwangncar commented Dec 12, 2023

weiwangncar commented Dec 12, 2023 •

edited

Loading

weiwangncar commented Dec 15, 2023

Improve NetCDF detection and linking #1923

Improve NetCDF detection and linking #1923

Conversation

rjdave commented Oct 12, 2023 • edited by weiwangncar Loading

weiwangncar commented Oct 12, 2023

rjdave commented Oct 13, 2023 • edited Loading

weiwangncar commented Oct 13, 2023

weiwangncar commented Oct 16, 2023

islas Oct 24, 2023

Choose a reason for hiding this comment

islas Oct 24, 2023

Choose a reason for hiding this comment

rjdave Oct 25, 2023

Choose a reason for hiding this comment

wkliao Dec 8, 2023

Choose a reason for hiding this comment

islas Dec 8, 2023

Choose a reason for hiding this comment

islas commented Oct 24, 2023

weiwangncar commented Dec 8, 2023

rjdave commented Dec 8, 2023

islas commented Dec 8, 2023 • edited Loading

weiwangncar commented Dec 9, 2023

rjdave commented Dec 11, 2023

weiwangncar commented Dec 12, 2023

weiwangncar commented Dec 12, 2023

weiwangncar commented Dec 12, 2023 • edited Loading

weiwangncar commented Dec 15, 2023

rjdave commented Oct 12, 2023 •

edited by weiwangncar

Loading

rjdave commented Oct 13, 2023 •

edited

Loading

islas commented Dec 8, 2023 •

edited

Loading

weiwangncar commented Dec 12, 2023 •

edited

Loading