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

WIP: Initial work to support new style NETCDF environment variables #1269

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

opoplawski
Copy link

TYPE: enhancement

KEYWORDS: configure, netcdf, ohpc

SOURCE: Orion Poplawski (NWRA)

DESCRIPTION OF CHANGES:
Problem:
Currently configure expects a specific layout for NetCDF and NetCDF Fortran's install. New "modular" installs may not use this layout, but specify locations in environment variable.

Solution:
Use the new environment variables if present.

ISSUE: Fixes #794

LIST OF MODIFIED FILES:

M       Makefile
M       arch/Config.pl
M       arch/conf_tokens
M       arch/configure.defaults
M       arch/postamble
M       configure
M       external/io_netcdf/makefile
M       hydro/Rapid_routing/makefile
M       hydro/Rapid_routing/makefile.cpl
M       hydro/Rapid_routing/makefile.orig
M       var/gen_be/Makefile

TESTS CONDUCTED:
Built with OHPC netcdf-fortran module.

RELEASE NOTE: configure can locate NetCDF installs through newer environment variables.

TODO:
This almost assuredly breaks builds with pNetCDF. I'm not familiar with that though and am not sure what the best way forward there is. There is also some code in configure that I think tries to parse symbolic links in the $NETCDF dir. I'm not sure what the status of that is.

@opoplawski opoplawski requested review from a team as code owners August 13, 2020 19:34
@opoplawski opoplawski marked this pull request as draft August 13, 2020 19:34
@davegill davegill changed the base branch from master to develop August 14, 2020 05:38
Copy link
Contributor

@kafitzgerald kafitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good on the "hydro" side. We'll make sure these changes are reflected in the WRF-Hydro repo as well.

@MichaelLaufer
Copy link
Contributor

MichaelLaufer commented Jan 13, 2022

Any progress with this PR? This has the potential to greatly simplify support for automated build environments such as Spack and Easybuild, as currently patches must be manually generated for each WRF version in order to implement similar logic to this PR.

@davegill
Copy link
Contributor

Any progress with this PR?

The WRF model has been gradually moving away from a paradigm where the NCAR hosts generate large amounts of code to one where the notion is that the community contributions are reviewed, iterated, and cleaned up by the NCAR gate keepers. However, most of the heavy lifting is done by contributors.

There are a number of I/O options available in WRF. Most of those options are required to work with different compile-time builds: chemistry, data assimilation, land surface, hydrology. Additionally the I/O has to work with the existing packages that use the WRF model as part of their couple system.

The WRF model needs to accommodate users that choose old NetCDF installations (a single Fortran library), medium new installations (two Fortran libraries), and newer versions that have a split between C and Fortran locations.

These combinations represent a vast amount of testing and getting an OK from dozens of people that agree to undertake some testing, and in the end this PR does not provide additional capability. We are striving towards making a brand new user feel comfortable with the build and running of WRF via containers. On larger HPC installations, most users have access to modules and the HPC system admins, allowing them to get systems built. We are putting that as where we are comfortable with the state of the WRF system build mechanism.

The PR states "This almost assuredly breaks builds with pNetCDF." We tend to spend quite a bit of time with infrastructure PRs when they do not have known considerations. Because of the likelihood of being a large drain on time with small payback potential, this PR is not going to be addressed or worked on by the NCAR WRF hosts. Should a community member wish to champion this PR and move it forward, that would be fantastic.

@MichaelLaufer
Copy link
Contributor

Hi @davegill, thanks for quick response. In general I am happy to help testing in any way to get this moved forward. I am also the Spack maintainer for the WRF package, so I have a vested interest in this. :)

Also regarding breaking PnetCDF: from looking at the commit, I do not see why this should break builds with PnetCDF. From my understanding these two I/O backends are fully decoupled. Yes it is true that used the same variable name NETCDFPATH in the io_pnetcdf/Makefile and arch/postamble as the NetCDF , but it points to the PnetCDF library, and not to the NetCDF one that has been changed here. I can test this with PnetCDF if no one has verified it as a potential bug if it will be useful here.

I am happy to help.

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.

WRF v4.0.1 configuration warning about NETCDF
4 participants