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.

Sign up for GitHub

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

Jump to bottom

Support different I/O types for history and restart output #2754

Merged
merged 11 commits into from
Apr 17, 2024
Merged

Support different I/O types for history and restart output #2754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7be0f75
Adding cmake file for ANL workstation
jayeshkrishna Jan 25, 2024
107da5a
Support for EAMXX/SCREAM testing in ANL GCE
jayeshkrishna Mar 20, 2024
efcc246
Updating SCORPIO to v1.6.1
jayeshkrishna Jan 24, 2024
c3cc055
Option to specify I/O type for hist/rest files
jayeshkrishna Jan 25, 2024
9f822b2
Adding ADIOS2 build for hipcc on Frontier
jayeshkrishna Feb 1, 2024
ceb4336
Reset MPICH GPU support for conversion tool
jayeshkrishna Mar 7, 2024
282405d
Moving SPIO I/O type utils inside scorpio nspace
jayeshkrishna Mar 11, 2024
bdc0c19
Disable BP to NetCDF conversion
jayeshkrishna Mar 12, 2024
67aff45
Add SCORPIO and GPTL includes
jayeshkrishna Mar 20, 2024
fa6c6e8
Updating SCORPIO to v1.6.2 alpha
jayeshkrishna Mar 21, 2024
6b407d0
Ensure PIO calls are not optimized out
jayeshkrishna Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cime_config/customize/case_post_run_io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ def _convert_adios_to_nc(case):
# Load the environment
case.load_env(reset=True)

# Reset MPICH/MPI GPU support, if enabled
is_mpich_gpu_enabled = os.environ.get('MPICH_GPU_SUPPORT_ENABLED')
if int(0 if is_mpich_gpu_enabled is None else is_mpich_gpu_enabled) == 1:
logger.info("Resetting support for GPU in MPICH/MPI library (since its not used by the tool)")
os.environ['MPICH_GPU_SUPPORT_ENABLED'] = str(0);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A CIME config change.
This change is not in E3SM master yet, but we will be adding this change there soon

run_func = lambda: run_cmd(cmd, from_dir=rundir)[0]

# Run the modified case
Expand Down
4 changes: 4 additions & 0 deletions cime_config/machines/config_machines.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1472,6 +1472,10 @@
<env name="OMP_PROC_BIND">spread</env>
<env name="OMP_PLACES">threads</env>
</environment_variables>

<environment_variables compiler="crayclang-scream" mpilib="mpich">
<env name="ADIOS2_ROOT">$SHELL{if [ -z "$ADIOS2_ROOT" ]; then echo /lustre/orion/cli115/world-shared/frontier/3rdparty/adios2/2.9.1/cray-mpich-8.1.26/crayclang-scream-14.0.0; else echo "$ADIOS2_ROOT"; fi}</env>
</environment_variables>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A CIME config change
This change also needs to be ported to E3SM master too

</machine>

<!-- Skylake nodes of Stampede2 at TACC -->
Expand Down
7 changes: 4 additions & 3 deletions cime_config/machines/config_workflow.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<template>template.case.test</template>
<prereq>$BUILD_COMPLETE and $TEST</prereq>
</job>
<job name="case.post_run_io">
<!--job name="case.post_run_io">
<template>template.post_run_io</template>
<dependency>case.run</dependency>
<prereq>case.get_value("PIO_TYPENAME_ATM") == 'adios' or \
Expand All @@ -53,11 +53,12 @@
<runtime_parameters>
<walltime>0:30:00</walltime>
</runtime_parameters>
</job>
</job-->
<job name="case.st_archive">
<template>template.st_archive</template>
<!-- If DOUT_S is true and case.run (or case.test) exits successfully then run st_archive-->
<dependency>(case.run and case.post_run_io) or case.test</dependency>
<!--dependency>(case.run and case.post_run_io) or case.test</dependency-->
<dependency>case.run or case.test</dependency>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A CIME config change
This is a SCREAM repo only change for now. This change disables the ADIOS to NetCDF conversion job from running after the E3SM job completes.

We will need to add a way to disable it via XML changes in E3SM.

<prereq>$DOUT_S</prereq>
<runtime_parameters>
<task_count>1</task_count>
Expand Down
1 change: 1 addition & 0 deletions components/eamxx/cime_config/namelist_defaults_scream.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ be lost if SCREAM_HACK_XML is not enabled.
<output_yaml_files type="array(string)"/>
<model_restart>
<filename_prefix>./${CASE}.scream</filename_prefix>
<iotype>default</iotype>
<output_control locked="true">
<Frequency>${REST_N}</Frequency>
<frequency_units>${REST_OPTION}</frequency_units>
Expand Down
12 changes: 12 additions & 0 deletions components/eamxx/cmake/machine-files/anlgce.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
include(${CMAKE_CURRENT_LIST_DIR}/common.cmake)
common_setup()

include (${EKAT_MACH_FILES_PATH}/kokkos/openmp.cmake)
include (${EKAT_MACH_FILES_PATH}/mpi/other.cmake)

# Remove this if you are using a resource manager (slurm etc)
set (EKAT_TEST_LAUNCHER_MANAGE_RESOURCES True CACHE BOOL "")

# EKAT MPI settings
set (EKAT_MPIRUN_EXE "mpiexec" CACHE STRING "mpiexec")
set (EKAT_MPI_NP_FLAG "-n" CACHE STRING "-n")
11 changes: 11 additions & 0 deletions components/eamxx/cmake/tpls/Scorpio.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ macro (CreateScorpioTargets)
option (PIO_ENABLE_FORTRAN "Enable the Fortran library builds" ON)

add_subdirectory (${E3SM_EXTERNALS_DIR}/scorpio ${CMAKE_BINARY_DIR}/externals/scorpio)

set (SCORPIO_Fortran_INCLUDE_DIRS ${CMAKE_BINARY_DIR}/externals/scorpio/src/flib CACHE INTERNAL "SCORPIO Fortran include dirs")
set (C_INCLUDE_DIRS "${E3SM_EXTERNALS_DIR}/scorpio/src/clib")
list(APPEND C_INCLUDE_DIRS "${CMAKE_BINARY_DIR}/externals/scorpio/src/clib")
set (SCORPIO_C_INCLUDE_DIRS "${C_INCLUDE_DIRS}" CACHE INTERNAL "SCORPIO C include dirs")

# Add GPTL from SCORPIO
if (NOT GPTL_PATH)
set (GPTL_PATH ${E3SM_EXTERNALS_DIR}/scorpio/src/gptl CACHE INTERNAL "Path to GPTL library")
endif ()

Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you just add these to the clib/flib/gptl targets? Like

target_include_directories (pioc PUBLIC 
   ${E3SM_EXTERNALS_DIR}/scorpio/src/clib
   ${CMAKE_BINARY_DIR}/externals/scorpio/src/clib
)

target_include_directories (piof PUBLIC
   ${CMAKE_BINARY_DIR}/externals/scorpio/src/flib
)

target_include_directories (gptl PUBLIC
  ${E3SM_EXTERNALS_DIR}/scorpio/src/gptl
)

It seems more CMake3.0 style, and less invaisve (only need to take care of this in this cmake script, rather than throughout the project)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, eventually we will move to just using find_library(scorpio...) mode. The basic pieces are there in SCORPIO now, we will start using it in E3SM/EAMXX in upcoming PRs.

(PS: Note that this hack is needed due to the way EAMXX currently adds SCORPIO source for standalone builds)

EkatDisableAllWarning(pioc)
EkatDisableAllWarning(piof)
EkatDisableAllWarning(gptl)
Expand Down
5 changes: 4 additions & 1 deletion components/eamxx/scripts/machines_specs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@
["mpic++","mpif90","mpicc"],
"srun --mpi=pmi2 -l -N 1 --kill-on-bad-exit --cpu_bind=cores",
"/lcrc/group/e3sm/baselines/chrys/intel/scream"),

"anlgce" : ([". /nfs/gce/software/spack/opt/spack/linux-ubuntu20.04-x86_64/gcc-9.3.0/lmod-8.3-6fjdtku/lmod/lmod/init/sh", "module purge", "module load autoconf/2.69-bmnwajj automake/1.16.3-r7w24o4 libtool/2.4.6-uh3mpsu m4/1.4.19-7fztfyz cmake/3.20.5-zyz2eld gcc/11.1.0-qsjmpcg zlib/1.2.11-p7dmb5p", "export LD_LIBRARY_PATH=/nfs/gce/projects/climate/software/linux-ubuntu20.04-x86_64/mpich/4.0/gcc-11.1.0/lib:$LD_LIBRARY_PATH", "export PATH=/nfs/gce/projects/climate/software/linux-ubuntu20.04-x86_64/mpich/4.0/gcc-11.1.0/bin:/nfs/gce/projects/climate/software/linux-ubuntu20.04-x86_64/netcdf/4.8.0c-4.3.1cxx-4.5.3f-serial/gcc-11.1.0/bin:$PATH", "export NetCDF_ROOT=/nfs/gce/projects/climate/software/linux-ubuntu20.04-x86_64/netcdf/4.8.0c-4.3.1cxx-4.5.3f-serial/gcc-11.1.0", "export PERL5LIB=/nfs/gce/projects/climate/software/perl5/lib/perl5"],
["mpicxx","mpifort","mpicc"],
"",
""),
"linux-generic" : ([],["mpicxx","mpifort","mpicc"],"", ""),
"linux-generic-debug" : ([],["mpicxx","mpifort","mpicc"],"", ""),
"linux-generic-serial" : ([],["mpicxx","mpifort","mpicc"],"", ""),
Expand Down
5 changes: 5 additions & 0 deletions components/eamxx/src/share/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ target_include_directories(scream_share PUBLIC
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_BINARY_DIR}/modules
)

if (GPTL_PATH)
target_include_directories(scream_share PUBLIC ${GPTL_PATH})
endif ()

target_link_libraries(scream_share PUBLIC ekat pioc)
target_compile_options(scream_share PUBLIC
$<$<COMPILE_LANGUAGE:Fortran>:${SCREAM_Fortran_FLAGS}>
Expand Down
8 changes: 8 additions & 0 deletions components/eamxx/src/share/io/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ if (DEFINED ENV{ADIOS2_ROOT})
target_include_directories(scream_io PRIVATE $ENV{ADIOS2_ROOT}/include)
endif ()

if (SCORPIO_Fortran_INCLUDE_DIRS)
target_include_directories(scream_io PUBLIC ${SCORPIO_Fortran_INCLUDE_DIRS})
endif ()

if (SCORPIO_C_INCLUDE_DIRS)
target_include_directories(scream_io PUBLIC ${SCORPIO_C_INCLUDE_DIRS})
endif ()

target_link_libraries(scream_io PUBLIC scream_share piof pioc)

if (SCREAM_CIME_BUILD)
Expand Down
5 changes: 4 additions & 1 deletion components/eamxx/src/share/io/scorpio_input.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,10 @@ void AtmosphereInput::finalize()
/* ---------------------------------------------------------- */
void AtmosphereInput::init_scorpio_structures()
{
scorpio::register_file(m_filename,scorpio::Read);
std::string iotype_str = m_params.get<std::string>("iotype", "default");
int iotype = scorpio::str2iotype(iotype_str);

scorpio::register_file(m_filename,scorpio::Read,iotype);

// Register variables with netCDF file.
register_variables();
Expand Down
2 changes: 2 additions & 0 deletions components/eamxx/src/share/io/scream_io_file_specs.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ struct IOFileSpecs {
bool is_open = false;
std::string filename;

int iotype = 0;

// If positive, flush the output file every these many snapshots
int flush_frequency = std::numeric_limits<int>::max();

Expand Down
10 changes: 7 additions & 3 deletions components/eamxx/src/share/io/scream_output_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,7 @@ setup (const ekat::Comm& io_comm, const ekat::ParameterList& params,
const auto& last_output_filename = get_attribute<std::string>(rhist_file,"last_output_filename");
m_resume_output_file = last_output_filename!="" and not restart_pl.get("force_new_file",false);
if (m_resume_output_file) {

scorpio::register_file(last_output_filename,scorpio::Read);
scorpio::register_file(last_output_filename,scorpio::Read,m_output_file_specs.iotype);
int num_snaps = scorpio::get_dimlen(last_output_filename,"time");
scorpio::eam_pio_closefile(last_output_filename);

Expand Down Expand Up @@ -680,6 +679,11 @@ set_params (const ekat::ParameterList& params,
m_checkpoint_file_specs.ftype = FileType::HistoryRestart;
}
}

// Set the iotype to use for the output file
std::string iotype = m_params.get<std::string>("iotype", "default");
m_output_file_specs.iotype = scorpio::str2iotype(iotype);
m_checkpoint_file_specs.iotype = scorpio::str2iotype(iotype);
}

/*===============================================================================================*/
Expand All @@ -698,7 +702,7 @@ setup_file ( IOFileSpecs& filespecs,
const auto& filename = filespecs.filename;
// Register new netCDF file for output. Check if we need to append to an existing file
auto mode = m_resume_output_file ? Append : Write;
register_file(filename,mode);
register_file(filename,mode,filespecs.iotype);
if (m_resume_output_file) {
eam_pio_redef(filename);
}
Expand Down
60 changes: 47 additions & 13 deletions components/eamxx/src/share/io/scream_scorpio_interface.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ module scream_scorpio_interface
!------------
use pio_types, only: iosystem_desc_t, file_desc_t, var_desc_t, io_desc_t, &
pio_noerr, pio_global, &
PIO_int, PIO_real, PIO_double, PIO_float=>PIO_real
PIO_int, PIO_real, PIO_double, PIO_float=>PIO_real,&
pio_iotype_netcdf, pio_iotype_pnetcdf, pio_iotype_adios
use pio_kinds, only: PIO_OFFSET_KIND

use mpi, only: mpi_abort, mpi_comm_size, mpi_comm_rank
Expand Down Expand Up @@ -79,7 +80,7 @@ module scream_scorpio_interface
eam_update_time, & ! Update the timestamp (i.e. time variable) for a given pio netCDF file
read_time_at_index ! Returns the time stamp for a specific time index

private :: errorHandle, get_coord, is_read, is_write, is_append
private :: errorHandle, get_coord, is_read, is_write, is_append, scream_iotype_to_pio_iotype

! Universal PIO variables for the module
integer :: atm_mpicom
Expand Down Expand Up @@ -193,18 +194,19 @@ module scream_scorpio_interface
!=====================================================================!
! Register a PIO file to be used for input/output operations.
! If file is already open, ensures file_purpose matches the current one
subroutine register_file(filename,file_purpose)
subroutine register_file(filename,file_purpose,iotype)

character(len=*), intent(in) :: filename
integer, intent(in) :: file_purpose
integer, intent(in) :: iotype

type(pio_atm_file_t), pointer :: pio_file

if (.not.associated(pio_subsystem)) then
call errorHandle("PIO ERROR: local pio_subsystem pointer has not been established yet.",-999)
endif

call get_pio_atm_file(filename,pio_file,file_purpose)
call get_pio_atm_file(filename,pio_file,file_purpose,iotype)
end subroutine register_file
!=====================================================================!
! Mandatory call to finish the variable and dimension definition phase
Expand Down Expand Up @@ -444,9 +446,11 @@ subroutine register_variable(filename,shortname,longname,units, &

if (is_write(pio_atm_file%purpose)) then
ierr = PIO_def_var(pio_atm_file%pioFileDesc, trim(shortname), hist_var%nc_dtype, hist_var%dimid(:numdims), hist_var%piovar)
call errorHandle("PIO ERROR: could not define variable "//trim(shortname),ierr)
call errorHandle("PIO ERROR: could not define variable "//trim(shortname)//" in file "//trim(filename),ierr)
ierr=PIO_put_att(pio_atm_file%pioFileDesc, hist_var%piovar, 'units', hist_var%units )
call errorHandle("PIO ERROR: could not set attribute 'units' for variable "//trim(shortname)//" in file "//trim(filename),ierr)
ierr=PIO_put_att(pio_atm_file%pioFileDesc, hist_var%piovar, 'long_name', hist_var%long_name )
call errorHandle("PIO ERROR: could not set attribute 'long_name' for variable "//trim(shortname)//" in file "//trim(filename),ierr)
else
ierr = PIO_inq_varid(pio_atm_file%pioFileDesc,trim(shortname),hist_var%piovar)
call errorHandle("PIO ERROR: could not retrieve id for variable "//trim(shortname)//" from file "//trim(filename),ierr)
Expand Down Expand Up @@ -776,7 +780,10 @@ subroutine eam_update_time(filename,time)
pio_atm_file%numRecs = pio_atm_file%numRecs + 1
call get_var(pio_atm_file,'time',var)
! Only update time on the file if a valid time is provided
if (time>=0) ierr = pio_put_var(pio_atm_file%pioFileDesc,var%piovar,(/ pio_atm_file%numRecs /), (/ 1 /), (/ time /))
if (time>=0) then
ierr = pio_put_var(pio_atm_file%pioFileDesc,var%piovar,(/ pio_atm_file%numRecs /), (/ 1 /), (/ time /))
call errorHandle("PIO ERROR: something went wrong while writing time var on file="//trim(filename),ierr)
endif
end subroutine eam_update_time
!=====================================================================!
! Assign institutions to header metadata for a specific pio output file.
Expand Down Expand Up @@ -867,26 +874,46 @@ function is_eam_pio_subsystem_inited() result(is_it) bind(c)

is_it = LOGICAL(associated(pio_subsystem),kind=c_bool)
end function is_eam_pio_subsystem_inited
!=====================================================================!
function scream_iotype_to_pio_iotype(siotype) result(piotype)
integer, intent(in) :: siotype
integer :: piotype

if(siotype == 0) then
piotype = pio_iotype
else if(siotype == 1) then
piotype = pio_iotype_netcdf
else if(siotype == 2) then
piotype = pio_iotype_pnetcdf
else if(siotype == 3) then
piotype = pio_iotype_adios
else
piotype = pio_iotype
end if
end function scream_iotype_to_pio_iotype
!=====================================================================!
! Create a pio netCDF file with the appropriate name.
subroutine eam_pio_createfile(File,fname)
subroutine eam_pio_createfile(File,fname,iotype)
use pio, only: pio_createfile
use pio_types, only: pio_clobber

type(file_desc_t), intent(inout) :: File ! Pio file Handle
character(len=*), intent(in) :: fname ! Pio file name
integer, intent(in) :: iotype
!--
integer :: retval ! PIO error return value
integer :: mode ! Mode for how to handle the new file
integer :: piotype

mode = ior(pio_mode,pio_clobber) ! Set to CLOBBER for now, TODO: fix to allow for optional mode type like in CAM
retval = pio_createfile(pio_subsystem,File,pio_iotype,fname,mode)
piotype = scream_iotype_to_pio_iotype(iotype)
retval = pio_createfile(pio_subsystem,File,piotype,fname,mode)
call errorHandle("PIO ERROR: unable to create file: "//trim(fname),retval)

end subroutine eam_pio_createfile
!=====================================================================!
! Open an already existing netCDF file.
subroutine eam_pio_openfile(pio_file,fname)
subroutine eam_pio_openfile(pio_file,fname,iotype)
use pio, only: pio_openfile
use pio_types, only: pio_write, pio_nowrite

Expand All @@ -895,13 +922,17 @@ subroutine eam_pio_openfile(pio_file,fname)
!--
integer :: retval ! PIO error return value
integer :: mode ! Mode for how to handle the new file
integer, intent(in) :: iotype

integer :: piotype

if (is_read(pio_file%purpose)) then
mode = pio_nowrite
else
mode = pio_write
endif
retval = pio_openfile(pio_subsystem,pio_file%pioFileDesc,pio_iotype,fname,mode)
piotype = scream_iotype_to_pio_iotype(iotype)
retval = pio_openfile(pio_subsystem,pio_file%pioFileDesc,piotype,fname,mode)
call errorHandle("PIO ERROR: unable to open file: "//trim(fname),retval)

if (is_append(pio_file%purpose)) then
Expand Down Expand Up @@ -1124,6 +1155,7 @@ subroutine eam_pio_finalize()

#if !defined(SCREAM_CIME_BUILD)
call PIO_finalize(pio_subsystem, ierr)
call errorHandle("PIO ERROR: something went wrong when calling PIO_finalize.",ierr)
nullify(pio_subsystem)
#endif

Expand Down Expand Up @@ -1361,12 +1393,13 @@ subroutine lookup_pio_atm_file(filename,pio_file,found,pio_file_list_ptr_in)
end subroutine lookup_pio_atm_file
!=====================================================================!
! Create a new pio file pointer based on filename.
subroutine get_pio_atm_file(filename,pio_file,purpose)
subroutine get_pio_atm_file(filename,pio_file,purpose,iotype)
use pio, only: PIO_inq_dimid, PIO_inq_dimlen

character(len=*),intent(in) :: filename ! Name of file to be found
type(pio_atm_file_t), pointer :: pio_file ! Pointer to pio_atm_output structure associated with this filename
integer,intent(in) :: purpose ! Purpose for this file lookup, 0 = find already existing, 1 = create new as output, 2 = open new as input
integer,intent(in) :: iotype

logical :: found
type(pio_file_list_t), pointer :: new_list_item
Expand Down Expand Up @@ -1404,7 +1437,7 @@ subroutine get_pio_atm_file(filename,pio_file,purpose)
pio_file%purpose = purpose
if (is_read(purpose) .or. is_append(purpose)) then
! Either read or append to existing file. Either way, file must exist on disk
call eam_pio_openfile(pio_file,trim(pio_file%filename))
call eam_pio_openfile(pio_file,trim(pio_file%filename),iotype)
! Update the numRecs to match the number of recs in this file.
ierr = pio_inq_dimid(pio_file%pioFileDesc,"time",time_id)
if (ierr.ne.0) then
Expand All @@ -1417,7 +1450,7 @@ subroutine get_pio_atm_file(filename,pio_file,purpose)
end if
elseif (is_write(purpose)) then
! New output file
call eam_pio_createfile(pio_file%pioFileDesc,trim(pio_file%filename))
call eam_pio_createfile(pio_file%pioFileDesc,trim(pio_file%filename),iotype)
call eam_pio_createHeader(pio_file%pioFileDesc)
else
call errorHandle("PIO Error: get_pio_atm_file with filename = "//trim(filename)//", purpose (int) assigned to this lookup is not valid" ,-999)
Expand Down Expand Up @@ -1464,6 +1497,7 @@ function read_time_at_index(filename,time_index) result(val)
ierr = pio_inq_dimid(pio_atm_file%pioFileDesc,trim("time"),dim_id)
call errorHandle("read_time_at_index ERROR: dimension 'time' not found in file "//trim(filename)//".",ierr)
ierr = pio_inq_dimlen(pio_atm_file%pioFileDesc,dim_id,time_len)
call errorHandle("PIO Error! Something went wrong when inquiring time dim length on file file "//trim(filename)//".",ierr)

if (present(time_index)) then
timeidx = time_index
Expand Down
6 changes: 3 additions & 3 deletions components/eamxx/src/share/io/scream_scorpio_interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ using scream::Int;
extern "C" {

// Fortran routines to be called from C++
void register_file_c2f(const char*&& filename, const int& mode);
void register_file_c2f(const char*&& filename, const int& mode, const int& iotype);
int get_file_mode_c2f(const char*&& filename);
void set_decomp_c2f(const char*&& filename);
void set_dof_c2f(const char*&& filename,const char*&& varname,const Int dof_len,const std::int64_t *x_dof);
Expand Down Expand Up @@ -94,8 +94,8 @@ void eam_pio_finalize() {
eam_pio_finalize_c2f();
}
/* ----------------------------------------------------------------- */
void register_file(const std::string& filename, const FileMode mode) {
register_file_c2f(filename.c_str(),mode);
void register_file(const std::string& filename, const FileMode mode, const int iotype) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you, but since you often call this with iotype=IoType::DefaultIoType, you could consider adding the default value in the fcn signature, so that you don't need to add , scorpio::IOType::DefaultIOType to all register_file calls..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think its a good idea to add the default value here (On the other hand forcing users to specify the args can prevent copy-paste errors :) ). I will make that change

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's not an obvious choice. As I said, up to you. If you think in the vast majority of cases the "default" value is fine, then adding defaults to the signature makes sense. But if the cost of using default when you shouldn't is very high, then we can just force users to always pass it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will modify the code to use default args since there are many instances where the default io type is used (including tests etc)

register_file_c2f(filename.c_str(),mode,iotype);
}
/* ----------------------------------------------------------------- */
void eam_pio_closefile(const std::string& filename) {
Expand Down
Loading