+Partial consolidation of netcdf calls in MOM_io #1311
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Took preliminary steps to consolidate the direct calls to netcdf in MOM_io.F90. Renamed check_grid_def to verify_variable_units and moved it to MOM_io, where it can be used more widely than just in MOM_regridding.F90. Also split get_var_sizes and get_varid out of num_timelevels and made them publicly visible. All answers are bitwise identical, but there are new public interfaces and the renaming of one routine.
Modified get_var_sizes to have the root_PE do the reading and then broadcast this information to the other processors, unless directed not to via the new optional argument all_read. Part of this involved splitting read_var_sizes out from get_var_sizes. Also added the new optional argument alt_units to verify_variable_units, to give some flexibility when checking units without hard coding the "meters" to "m" comparison within verify_variable_units.
When the field_chksum interfaces was added, it returned integers, but the set of mpp_chksum routines from FMS it wraps return 64-bit integers, not 32-bit integers. For small domain sizes (like in the TC tests) this is not a problem, but for much larger problems (like tides_025), this results in a truncation of the results and a change in the checksums, which in turn causes the model to fail if it tries to read a depth-list file. This commit restores the model behavior prior to MOM6 PR#1299 (commit# 797b195..). All answers are bitwise identical in cases that worked with the previous version of the code, but some depth-list files that were created with MOM6 code after that commit may have incorrect verification checksums that lead to spurious fatal errors and should be deleted.
Added the new set of routines read_variable to do a simple read of an entire array from a netCDF file with the root PE, and then broadcast this information to the other PEs. Also added read_attribute to read a global or variable attribute. The option to read dimension names was added to get_var_sizes, and the option for get_varid to indicate whether a variable has been found. As a part of this, separate variants of broadcast were added for 32-bit and 64-bit integers, as were new variants of MOM_read_data for 1-d and 0-d integer fields. All answers are bitwise identical, but there are new options and optional arguments for overloaded interface, and there are two new overloaded interfaces for reading.
Replaced the calls to netCDF in read_depth_list with calls to routines in MOM_io.F90 that accomplish the same thing. Also added trim calls to a number of error messages in write_depth_list_file. All answers and output are bitwise identical.
Eliminated the direct netCDF calls in MOM_shared_initialization, replacing them with calls to routines in MOM_io. All answers are bitwise identical.
Eliminated the direct netCDF calls in read_Z_edges in MOM_tracer_Z_init, replacing them with calls to routines in MOM_io. All answers are bitwise identical.
Added the missing comments describing the new interface read_attribute. All answers are bitwise identical.
|
@Hallberg-NOAA, note that the netCDF library doesn't check for adequate space when reading in attributes. The netCDF library will happily write beyond the end of your array if it is not long enough. This is a pain to debug when it happens. I've yet to see a compiler that catches this, I suspect because the underlying read is happening in C. I think you are more likely to get bitten with char/string attributes than other types, because non-char attributes tend to be scalars in typical netCDF files, not arrays. I suggest comparing the attribute length to the size of Code in the CESM fork of POP that does this for char attributes is at https://github.com/ESCOMP/POP2-CESM/blob/9f2a48328dda6bd98be74f4ccfc13248b1b13868/source/io_netcdf.F90#L232-L258. Note that this code is using the pio API, not the netCDF API. But the issues are the same. |
Modified read_attribute_str to work with allocatable character string arguments to properly handle string attributes of unknown (and possibly enormous) size, following advice from Keith Lindsay. Because this is a change in an argument to a (recently added) publicly visible interface, it required changes to the two places where it was called. All answers are bitwise identical, but an interface has changed.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1311 +/- ##
============================================
+ Coverage 44.05% 45.78% +1.73%
============================================
Files 234 234
Lines 72269 72460 +191
============================================
+ Hits 31837 33177 +1340
+ Misses 40432 39283 -1149
Continue to review full report at Codecov.
|
Added the option to provide a handle to an open netCDF file via the new ncid_in optional arguments to various routines that read from netCDF files. Along with the new interfaces open_file_to_read and close_file_to read in MOM_io these allow for files to be opened and closed once while reading a number of fields or attributes from the same file. If these arguments are not provided, the files are opened and closed with each call as before. All answers are bitwise identical, although there are new optional arguments and new public interfaces.
|
Added more commits to PR#1311 to further concentrate I/O calls in MOM_io.F90, and to respond
With these added commits, I consider PR#1311 to be complete, and have no plans for any additions. |
|
This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/12001. |
marshallward
approved these changes
Feb 4, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Took preliminary steps to consolidate the direct calls to netcdf in
MOM_io.F90. Renamed check_grid_def to verify_variable_units and moved it to
MOM_io, where it can be used more widely than just in MOM_regridding.F90. Also
split get_var_sizes and get_varid out of num_timelevels and made them publicly
visible. All answers are bitwise identical, but there are new public interfaces
and the renaming of one routine.