Allow using the SMIOL I/O library when building with cmake. #1254
Conversation
|
Have you run any tests, that are easily reproducible, to test these changes? It would be interesting to see if the same problem in PR 1234 occurs when using SMIOL. |
| @@ -59,7 +61,9 @@ find_package(OpenMP COMPONENTS Fortran) | |||
| find_package(MPI REQUIRED COMPONENTS Fortran) | |||
| find_package(NetCDF REQUIRED COMPONENTS Fortran C) | |||
| find_package(PnetCDF REQUIRED COMPONENTS Fortran) | |||
| find_package(PIO REQUIRED COMPONENTS Fortran C) | |||
| if(NOT MPAS_USE_SMIOL) | |||
There was a problem hiding this comment.
This doesn't seem to work. When I build with -DMPAS_USE_SMIOL=1 the cmake output still has:
-- Found PIO: /glade/work/epicufsrt/contrib/spack-stack/derecho/spack-stack-1.6.0/envs/unified-env/install/gcc/12.2.0/parallelio-2.5.10-ysh2ljm found components: Fortran C
-- FindPIO:
-- - PIO_PREFIX [/glade/work/epicufsrt/contrib/spack-stack/derecho/spack-stack-1.6.0/envs/unified-env/install/gcc/12.2.0/parallelio-2.5.10-ysh2ljm]
-- - PIO Components Found: Fortran;C;SHARED
as well as
-- The following REQUIRED packages have been found:
...
* PIO
cmake/Functions/MPAS_Functions.cmake
Outdated
| ) | ||
| if(MPAS_USE_SMIOL) | ||
| set(MPAS_FORTRAN_TARGET_COMPILE_DEFINITIONS | ||
| _MPI=1 |
There was a problem hiding this comment.
Seems like we might want to split out common defines and only set relevant ones within the respective if() check
There was a problem hiding this comment.
I moved the common defines out of this if/then clauses.
src/external/SMIOL/CMakeLists.txt
Outdated
| @@ -0,0 +1,32 @@ | |||
|
|
|||
| cmake_minimum_required(VERSION 3.10) | |||
There was a problem hiding this comment.
I don't think this is necessary, especially as parts of this file inherently rely on early parts of the CMake build so this wouldn't be able to be run standalone
There was a problem hiding this comment.
I removed the cmake_minimum_required.
| find_package(PnetCDF REQUIRED COMPONENTS Fortran C) | ||
|
|
||
| # Specify the library source files | ||
| set(SMIOL_C_SOURCES smiol.c smiol_utils.c) |
There was a problem hiding this comment.
SMIOL_C_SOURCES and SMIOLF_SOURCES don't quite follow the same naming convention. Doesn't matter too much, but it might be nice to align the two. My understanding is SMIOLF_SOURCES is meant to be effectively, <TARGET_NAME>_SOURCES and follow the Makefile target name. SMIOL_SOURCES for the C sources follows that same convention, though it does look more ambiguous than the _C_ one.
There was a problem hiding this comment.
Do you mean change SMIOLF_SOURCES to SMIOL_F_SOURCES?
There was a problem hiding this comment.
I think that would also work, but in keeping with the <TARGET_NAME>_SOURCES I propose SMIOL_C_SOURCES => SMIOL_SOURCES. If that's too ambiguous of a variable name, then yes exactly what you said instead.
There was a problem hiding this comment.
I prefer to leave the explicit C naame, esp as c sources are rare.
src/framework/CMakeLists.txt
Outdated
| if (MPAS_USE_SMIOL) | ||
| set(FRAMEWORK_COMPILE_DEFINITIONS | ||
| MPAS_SMIOL_SUPPORT | ||
| mpas=1 |
There was a problem hiding this comment.
Same as before, I think breaking out common defines would be good, leaving only the specifics within the conditional branch
There was a problem hiding this comment.
I moved the common defines out of this if/then clauses.
| enable_language(Fortran) | ||
| mpas_fortran_target(smiolf) | ||
| add_library(${PROJECT_NAME}::external::smiolf ALIAS smiolf) | ||
| target_compile_definitions(smiolf PRIVATE SMIOL_PNETCDF ) |
There was a problem hiding this comment.
When compiling this branch on my local machine, the build fails because the smiol target cannot find mpi.h. I attached the build log for reference.
log.make.txt
The issue is resolved by adding target_include_directories(smiol PRIVATE ${MPI_INCLUDE_PATH}) after target_compile_definitions(smiol PRIVATE SMIOL_PNETCDF SINGLE_PRECISION).
There was a problem hiding this comment.
Thanks for testing and finding this.
This PR removes the restriction of only using the PIO I/O library when building with cmake.
The default behavior will build using PIO.
In order to build and use the SMIOL I/O library provide
-DMPAS_USE_SMIOL=1on the command line when running cmake.Test run include the mpasjedi ctests, as well as the MPAS-Workflow cylc tests:
3dvar_OIE120km_WarmStart_TEST/
3dvar_OIE120km_ColdStart_TEST/
3dvar_O30kmIE60km_ColdStart_TEST/
3denvar_O30kmIE60km_WarmStart_TEST/
4denvar_OIE120km_WarmStart_TEST/
4dhybrid_OIE120km_WarmStart_TEST/
eda_OIE120km_WarmStart_TEST/
getkf_OIE120km_WarmStart_TEST/
ForecastFromGFSAnalysesMPT_TEST/
3denvar_OIE120km_IAU_WarmStart_TEST/
because the test tries to read an HDF5 data file.