CDI-PIO support #485
CDI-PIO support #485
Conversation
…to IO workflow before testing
…rk on attributes functions
- pass std::string by const reference - throw an exception if the I/O mode is invalid - rename m_async to m_initialized (this is a closer match to its meaning) - replace a sequence of if () ... else if () with a std::map (makes the code easier to read) - replace int with unsigned int for n_writers since the number of writers cannot be negative - add an include guard to the header
- replace if conditions with std::map - throw an exception on invalid input
We need to call xt_idxlist_delete() to free resources allocated by xt_idxvec_new(). Also: replace malloc()...free() with a std::vector.
… closing and re-opening files
…nding is supported in CDI-PIO
|
The issue with The code has been tested and it seems to work as expected. |
|
All the output files are now opened at the beginning and closed at the end. It seems to work as expected but more tests are needed. |
|
The CDI-PIO mode is not needed because right now we are supporting only NetCDF files. It should be quite straightforward to add GRIB files support but I think it is better to first validate the implementation for NC files which are currently the only format used within PISM. |
This pull request contains CDI-PIO-related changes from #479, but applied to the
devbranch instead ofmaster. PICO optimizations are already in thedevbranch.The code as it stands has several issues:
only asynchronous writing is supported, making it harder than necessary to test the implementation (in particular: we cannot test this code in a serial run, only with
mpiexec -n N pismr ...for someN > 1)The
asyncflag is useless becausepismrstops with an error message when it is set to "false".It is not clear if appending to an existing file (
-extra_append) is possible. Need to be clarified with CDI-PIO developers if appending files is supported. Reading the CDI documentation it does not seem to be the case.Reading is not implemented. Moreover, it fails silently instead of stopping with an error message.
The code does not follow the RAII (see Wikipedia as well) idiom: the
CDIclass should be responsible for both opening and closing files, ensuring that all open files get closed. For example:CDI::open_impldoes not actually open a file. This is wrong.The
CDIclass needs documenting comments. I understand what the code does, but it is not clear why it is done.Configuration parameters specifying CDI file types and I/O modes should be "keywords", not integers.
Possible file type and I/O mode choices are undocumented.
The
Fileclass keeps track of whether dimensions were written andVariableMetadatakeeps track of the same for variables. This is inconsistent; both tasks should probably be performed inFile.File::is_split()andFile::set_split()do not make sense: oneFileinstance corresponds to one file, never multiple files.YAXT
is written in the GNU dialect of Cuses C99 features that are not a part of the C++ standard and so includingyaxt.h(andcdi.has well) pollutes PISM's code, breaking standard compliance (PISM follows the C++11 language standard). This makes it impossible to compile this using GCC 10 and possibly some other compilers. (Clang works.)PISM's code should compile (with
CMAKE_BUILD_TYPE=Debug) without warnings.We need to free all allocated resources (see
malloc()andfree(),ccs_init_calendar()andccs_free_calendar(), etc).Most of PISM should not have to
#include cdi.h,yaxt.h, etc. Most CDI-PIO and YAXT functions should be called from the "Initializer" class and from the CDI class, but not elsewhere.All changes to
IceModelrelated to avoiding closing and re-opening files should work with all supported I/O libraries and should not be enclosed in#if (Pism_USE_CDIPIO==1) ... #endif.The code initializing and finalizing MPI, PETSc, and CDI-PIO in
pismr.ccneeds more testing.update documentation
update
CHANGES.rstadd regression tests