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.

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

Add NETCDFF_CONFIG flag to support NetCDF > 4.9.1 #1072

Conversation

camilleanne
Copy link

Pull Request Summary

Updates executable generation to support NetCDF > 4.9.1

Description

As of netcdf-c 4.9.2 the ability to access nf-config values via nc-config is no longer supported. For example, the commands nc-config --flibs or nc-config --fc no longer work. See this outdated documentation for more examples.

This behavior is currently hardcoded into the generation of the WW3 NetCDF pre- and post-processing executables. This PR adds a new environment variable NETCDFF_CONFIG whose value should be the path of nf-config. This environment variable will be required for versions of NetCDF-c > 4.9.1. If using a version of NetCDF-c < 4.9.1, NETCDFF_CONFIG is optional and it's value will be set to the provided value for NETCDF_CONFIG, and thus this change is backwards compatible.

Issue(s) addressed

No specific issues, although may help with #392.

Commit Message

Add NETCDFF_CONFIG flag to support NetCDF > 4.9.1

Check list

Testing

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

I would appreciate some guidance on how to test these changes -- I have tested this locally extensively by building and running WW3 (specifically by compiling the netcdf executables with ./model/bin/w3_automake ww3_ounf ww3_prnc ww3_ounp as well as ww3_multi with the same command), but I am new to the WW3 test frameworks and am unclear which (if any) of the tests in this repo would appropriate to run.

@JessicaMeixner-NOAA
Copy link
Collaborator

@camilleanne Thanks for making a PR! I have a quick question - we're actually in the process of moving away from the GNU make that your update supports to CMAKE. Have you tried the cmake build with the NetCDF > 4.9.1? Currently we're using NetCDF 4.9.2 with CMAKE and haven't needed any updates.

@camilleanne
Copy link
Author

camilleanne commented Sep 12, 2023

@JessicaMeixner-NOAA I haven't tried that -- I will give it a go! Would be happy not to have to make this change.

Note: I was under the impression that w3_automake was the preferred means of building ww3 according to the manual (v6.07), but I see this issue #697 contradicts that. Are there plans to publish a new manual with the cmake instructions? Or deprecate the manual in favor of the readmes? We're actively developing against WW3 and I've been assuming that the manual is a reasonable source of truth as it's still maintained in this repo. What should we assume is the documentation of current best practices?

@JessicaMeixner-NOAA
Copy link
Collaborator

@camilleanne We do our best to keep the manual up to date, but many times its behind (you can search the issue for "documentation" to find known missing things), the lack of manual documentation is particularly egregious for CMAKE. The original developer who put in CMAKE has sense gone on to other things and so it's actually on me to update the manual and other documents and I'm very behind on this. We haven't fully taken out the GNU make build because the documentation for cmake is not quite complete. We do recommend moving to CMAKE, but from what I know from others GNU still works. Unfortunately it will still be a bit before I can get the CMAKE documentation updated. In the meantime feel free to open a discussion issue with any questions and see: https://github.com/NOAA-EMC/WW3/tree/develop/model#readme for instructions.

@JessicaMeixner-NOAA
Copy link
Collaborator

@camilleanne have you been able to test the cmake build to see if this is working for you? There's some concern that the changes implemented here would break for those not using the latest versions of intel as implemented and so we're hesitant to merge this as is. Please let us know if you need any assistance with cmake.

@camilleanne
Copy link
Author

@JessicaMeixner-NOAA Sorry for the late response, I was OOO. We have had success with the cmake build, although have hit one issue that I'll open a ticket for today. I think we can close this PR unmerged and we'll refocus our efforts on transitioning to cmake.

@MatthewMasarik-NOAA
Copy link
Collaborator

Hi @camilleanne, thanks for the update. I'm replying since Jessica is on leave till Monday. I'm glad to hear you've had some success with the cmake build. Please let us know of any other issues, while we are still working to fill out the cmake documentation. I'll keep an eye out for the new ticket you mentioned. Thanks for your interest.

@camilleanne
Copy link
Author

Here is the ticket I mentioned: #1082

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants