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

V5.1.1 IO update #426

Merged
merged 5 commits into from
Nov 15, 2019
Merged

V5.1.1 IO update #426

merged 5 commits into from
Nov 15, 2019

Conversation

kafitzgerald
Copy link
Contributor

TYPE: enhancement?

KEYWORDS: IO, netCDF, compile option, WRF

SOURCE: Katelyn (NCAR)

DESCRIPTION OF CHANGES:

  • Removes WRFIO_NCD_LARGE_FILE_SUPPORT compile time option. This is no longer the WRF default (it's been replaced with a flag that does the opposite).
  • Rather than implement this other flag these netCDF3 outputs were updated to netCDF4, which is already required elsewhere.
  • Updates nf_create function to nf90_create
  • Changes deprecated NF90_HDF5 mode to NF90_NETCDF4

ISSUE: Addresses issue #230

TESTS CONDUCTED: Test suite via testing container. Fails testing where netCDF file type has changed.

NOTES:

  • Relevant to v5.1.1 (for integrating with WRF) and master
  • No huge rush to merge this (other than the pending community release)
  • May want to consider other approaches (e.g. implementing the same flag as WRF and maintaining some netCDF3 outputs)
  • We're using the netCDF4 output format (already in some places and in more after this PR), but as far as I can tell don't require features from the netCDF4 enhanced data model. Given the state of support for features in the enhanced data model (not great) in various analysis + display tools, we might consider setting the NF90_CLASSIC_MODEL flag to enforce the classic data model.

Checklist

@kafitzgerald kafitzgerald added the I/O Input/Output related issues label Nov 13, 2019
@kafitzgerald kafitzgerald added this to the v5.1.1 milestone Nov 13, 2019
@hellkite500
Copy link
Contributor

"We're using the netCDF4 output format (already in some places and in more after this PR), but as far as I can tell don't require features from the netCDF4 enhanced data model. Given the state of support for features in the enhanced data model (not great) in various analysis + display tools, we might consider setting the NF90_CLASSIC_MODEL flag to enforce the classic data model."

The enhanced data model is backwards compatible, and tooling IS adding support for new features (some faster than others). Also, with older versions of the netcdf C library, issues exist in working with the classic model on some tools (see pydata/xarray#2822, for example). I see no reason to strictly enforce the classic model, when it is a compatible subset of the enhanced. Also, adopting the enhanced model everywhere makes adopting new features simpler in the future.

@kafitzgerald
Copy link
Contributor Author

I didn't implement this anyway, but for what it's worth I wasn't talking about going back to netCDF3 just restricting to the features of the classic model for the sake of compatibility. Per the docs:

Oring the NF90_CLASSIC_MODEL flag with the NF90_NETCDF4 flag causes the resulting netCDF-4/HDF5 file to restrict itself to the classic model - none of the new netCDF-4 data model features, such as groups or user-defined types, are allowed in such a file.

This could easily be changed when there's broader support for the enhanced features.

That said, I'm fine with leaving it is as.

@rcabell
Copy link
Collaborator

rcabell commented Nov 15, 2019

I like that this removes WRFIO_NCD_LARGE_FILE_SUPPORT -- we should add ("add"? 🤔) that to the master branch as well...

@rcabell
Copy link
Collaborator

rcabell commented Nov 15, 2019

This PR passes automated CONUS regression tests for AnA and Long-Range with the expected exception of metadata/file formats. The following are the testing-related messages for these changes:

{'channel_rt': 0, 'chanobs': 0, 'lakeout': 0, 'gwout': 0, 'ldasout': 0, 'restart_hydro': 2, 'restart_lsm': 2, 'restart_nudging': 0}

DIFFER : FILE FORMATS : NC_FORMAT_64BIT <> NC_FORMAT_NETCDF4
/glade/u/home/rcabell/.local/lib/python3.6/site-packages/wrfhydropy/core/simulation.py:263: UserWarning: Model minor versions v5.1.1-beta
   do not match domain minor versions v5.1.0-beta2
  
    domain.compatible_version)

The issue with the domain version should be addressed by another Issue/PR and need not be changed here.

@rcabell rcabell merged commit 3b7a386 into NCAR:v5.1.1 Nov 15, 2019
rcabell added a commit to rcabell/wrf_hydro_nwm_public that referenced this pull request Nov 21, 2019
The one-step compile code cherry-picked from downstream `master` conflicted with PR NCAR#426 but this wasn't apparent until they were merged together in PR NCAR#428. This commit fixes that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I/O Input/Output related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants