search

schism-dev / schism

Semi-implicit Cross-scale Hydroscience Integrated System Model (SCHISM)
http://ccrm.vims.edu/schismweb/
Apache License 2.0
93 stars 88 forks source link

Added options in support for both standalone and coupled compilations (e.g. ufs-coastal) #138

Closed pvelissariou1 closed 5 months ago

pvelissariou1 commented 5 months ago

Added the BUILD_TOOLS option in support of "couple" model compilations (e.g ufs-coastal). Modified the executable installation paths if the BLD_STANDALONE option is not active (e.g. ufs-coastal)

josephzhang8 commented 5 months ago

@pvelissariou1 (1) Have you confirmed that the changes in cmake for hercules/orion,/frontera lead to faster performance? (2) Can you explain the changes you made in CMakeList.txt? I see if(BLD_STANDALONE) ...else(BLD_STANDALONE) etc

pvelissariou1 commented 5 months ago

@josephzhang8

(1) Have you confirmed that the changes in cmake for hercules/orion,/frontera lead to faster performance? In recent Intel compilers >=20, the over-optimization (O3) and the use of advanced vector extensions (-axCORE-AVX*) generatated memory errors during compilation of SCHISM, thus why I added the "-mcmodel medium" to overcome the memory 2GB limitation. Performace remains the same. My opinion is to remove -axCORE-AVX2 -O3 and to fall back to "-g -O2 -traceback". In ufs-coastal we don't use cpu specific instructions during compilation.

(2) Can you explain the changes you made in CMakeList.txt? I see if(BLD_STANDALONE) ...else(BLD_STANDALONE) etc In lines 177-193, this if block allows for the installation of the binaries like before (BLD_STANDALONE case) and in the build/bin location when used in ufs-coastal or other coupled system. Important for the ufs-coastal and other users to have the SCHISM binaries installed in the top level build system. I implemented the BUILD_TOOLS option to compile the SCHISM utilities and the pschism executable in ufs-coastal as well (we need the utilities for sure, and occasionaly the pschism* executable in some occasions).

pvelissariou1 commented 5 months ago

@josephzhang8 So these modifications compile SCHISM, standalone or coupled nicely.

josephzhang8 commented 5 months ago

How does the following in CMake* (line 177) make sense? The if() and else(0 conditions are same.

if (BLD_STANDALONE) set ( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin)

set ( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin

CACHE PATH "Output Directory for executables.")

set ( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)

set ( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib

CACHE PATH "Output Directory for all static libraries.")

set ( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)

set ( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib

CACHE PATH "Output Directory for all shared libraries.")

else(BLD_STANDALONE) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) endif(BLD_STANDALONE)

pvelissariou1 commented 5 months ago

@josephzhang8 The difference is between PROJECT_BINARY_DIR and CMAKE_BINARY_DIR. in: if (BLD_STANDALONE) set ( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin) . . . else(BLD_STANDALONE) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) . . . endif(BLD_STANDALONE) In the PROJECT_BINARY_DIR case the executables are installed in schism/src/bin/ (do we want this?) while in the CMAKE_BINARY_DIR case the executables are installed in build/bin/. This is the difference. My preference would be to replace the whole if block by only:

set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)

that works for both cases. What do you think?

josephzhang8 commented 5 months ago

My question is: the else() part will never be executed as its condition is identical to if()?

pvelissariou1 commented 5 months ago

@josephzhang8 In cmake you can put tags in the if blocks (and all other contitional, loops, etc ...) to identify the opening and closing statements) in this case if (BLD_STANDALONE) contains the conditional and else(BLD_STANDALONE) simply says if NOT BLD_STANDALONE do the following and endif(BLD_STANDALONE) says close the if BLD_STANDALONE conditional. This is for clarification reasons only. The if block: if (BLD_STANDALONE) . else() . endif() works exactly the same as well. This is a CMake approach.

pvelissariou1 commented 5 months ago

@josephzhang8 I can remove the else(BLD.. and endif(... if you think it is confusing.

josephzhang8 commented 5 months ago

Thx @pvelissariou1 for the explanation. Yes plz use else() to avoid confusion.

I'm using intel>20 on Hercules:

Currently Loaded Modules: 1) intel-oneapi-compilers/2022.2.1 4) hdf5/1.12.2 7) libxkbcommon/1.4.0 10) qt/5.15.8 2) intel-oneapi-mpi/2021.7.1 5) netcdf-c/4.9.0 8) zlib/1.2.13 11) cmake/3.26.3 3) netcdf-fortran/4.6.0 6) glx/1.4 9) sqlite/3.39.4 12) python/3.10.8

But did not have compile or execution errors. I'm wondering why you got that.

pvelissariou1 commented 5 months ago

@josephzhang8 On our side we faced this problem when Soroosh was running his ensemble simulations. Let me recompile and I'll report back

josephzhang8 commented 5 months ago

RE: "-mcmodel medium", I just tested on Hercules with 2 runs at same time, 1 with and the other without this in compile. There is 1 min difference between the 2 runs for each simulation day (11-12 min vs 12-13min) which is not trivial.

pvelissariou1 commented 5 months ago

@josephzhang8 The errors come from the Utilities (see below) Modules loaded: 1) intel-oneapi-compilers/2022.2.1 2) intel-oneapi-mpi/2021.7.1 3) hdf5/1.12.2 4) netcdf-c/4.9.0 5) netcdf-fortran/4.6.0 6) glx/1.4 7) libxkbcommon/1.4.0 8) zlib/1.2.13
9) sqlite/3.39.4 10) qt/5.15.8 11) cmake/3.26.3 12) python/3.10.8

Command to configure cmake: cmake -S src -B build -C cmake/SCHISM.local.hercules -DUSE_ATMOS=ON -DNO_PARMETIS=OFF -DBLD_STANDALONE=ON

after that: cd build; make

Errors: [ 79%] Linking Fortran executable ../../bin/gen_slope_filter2 /apps/spack-managed/gcc-11.3.1/intel-oneapi-compilers-2022.2.1-z2sjni66fcyqcsamnoccgb7c77mn37qj/compiler/2022.2.1/linux/compiler/lib/intel64_lin/libifcoremt.a(for_init.o): in function for__process_start_time': for_init.c:(.text+0xc): relocation truncated to fit: R_X86_64_PC32 against.bss' /apps/spack-managed/gcc-11.3.1/intel-oneapi-compilers-2022.2.1-z2sjni66fcyqcsamnoccgb7c77mn37qj/compiler/2022.2.1/linux/compiler/lib/intel64_lin/libifcoremt.a(for_init.o): in function for_rtl_init_': for_init.c:(.text+0x57): relocation truncated to fit: R_X86_64_PC32 against.bss' for_init.c:(.text+0x78): relocation truncated to fit: R_X86_64_PC32 against .bss' for_init.c:(.text+0x226): relocation truncated to fit: R_X86_64_PC32 against.bss' for_init.c:(.text+0x235): relocation truncated to fit: R_X86_64_PC32 against .bss' for_init.c:(.text+0x242): relocation truncated to fit: R_X86_64_PC32 against.bss' for_init.c:(.text+0x27d): relocation truncated to fit: R_X86_64_PC32 against symbol for__l_excpt_info' defined in .bss section in /apps/spack-managed/gcc-11.3.1/intel-oneapi-compilers-2022.2.1-z2sjni66fcyqcsamnoccgb7c77mn37qj/compiler/2022.2.1/linux/compiler/lib/intel64_lin/libifcoremt.a(for_init.o) for_init.c:(.text+0x295): relocation truncated to fit: R_X86_64_PC32 against.bss' for_init.c:(.text+0x2a0): relocation truncated to fit: R_X86_64_PC32 against .bss' for_init.c:(.text+0x2ab): relocation truncated to fit: R_X86_64_PC32 against.bss' for_init.c:(.text+0x2af): additional relocation overflows omitted from the output ld: failed to convert GOTPCREL relocation; relink with --no-relax make[2]: [Utility/Pre-Processing/CMakeFiles/gen_slope_filter2.dir/build.make:96: bin/gen_slope_filter2] Error 1 make[1]: [CMakeFiles/Makefile2:1469: Utility/Pre-Processing/CMakeFiles/gen_slope_filter2.dir/all] Error 2 make: *** [Makefile:146: all] Error 2

josephzhang8 commented 5 months ago

OK, do we really need those util compile each time? In any case perhaps we can just add it to util, not main code? Thx/

pvelissariou1 commented 5 months ago

@josephzhang8 If you think that "-axCORE-AVX2" needs to remain, adding the "-mcmodel medium" option doesn't hurt that much. Anyway, in ufs-coastal we don't use cpu optimization instructions for compilations. I don't know how to fully answer this. May be in the Utility folder, we can adjust the cmake compile flags, as you suggested, or fix the offending utility code.

josephzhang8 commented 5 months ago

Thx @pvelissariou1. I can see why gen_slope_filter2 led to the compile error (static arrays). I suggest you turn of compile for util at ufs side first as a work-around, while I work on the fix.

I'm ready to accept the CMak* changes once you revert the other changes in hercules, fronter and orion. Thx.

pvelissariou1 commented 5 months ago

@josephzhang8 I have created an issue for the Intel icc compiler. As of Intel 2024 version, icc is not available and has been replaced by icx so the compilation breaks. I think the cmake/make scripts need to be adjusted accordingly (actually to support both icc and icx). Something to consider.

josephzhang8 commented 5 months ago

Yes we should update the cmake for that.

josephzhang8 commented 5 months ago

@pvelissariou1 : I've adjusted the array dimensions in gen_slope_filter2 in master, and can make all on hercules now. Plz pull and confirm.

pvelissariou1 commented 5 months ago

@josephzhang8 Let me clone master and compile standalone (and in ufs -coastal). I am between meetings. I'll report back as soon as possible.

pvelissariou1 commented 5 months ago

@josephzhang8 I compiled the schism latest commit both "standalone" and in ufs-coastal and everything went as expected and the executables installed in the proper location for both compilations. Thank you Joseph - everything looks good

josephzhang8 commented 5 months ago

gr8; thx @pvelissariou1