NOAA-EMC / WW3

WAVEWATCH III
Other
267 stars 545 forks source link

Unification of compiler flags within cmake ... #927

Open aronroland opened 1 year ago

aronroland commented 1 year ago

Is your feature request related to a problem? Please describe. Compiler flags are used in any kind of combination and it is like a Monte Carlo experience when looking at all the various flags everybody is using. Some flags are depreciated some flags are "highly" questionable and some flags are just "wrong" in a certain constellation or even misleading. What I expect from the community is to come up with 4 constellation as suggested in cmake and I will do the 1st step and provide some suggestion for that based on the intel compiler.

Describe the solution you'd like Unification of compiler flags within cmake

Describe alternatives you've considered The alternative is that the left hand does not know what right hand is doing.

Additional context none

aronroland commented 1 year ago

ok, so for the intel compiler my suggested setting is the following:

CMAKE_Fortran_FLAGS_DEBUG -g -O0 -ftz -check noarg_temp_created -align all -fno-alias -traceback -debug -nolib-inline -fno-inline-functions -assume protect_parens,minus0 -prec-div -prec-sqrt -check bounds -check uninit -fp-stack-check -ftrapuv -warn nounused,interfaces,nouncalled

CMAKE_Fortran_FLAGS_MINSIZEREL -O1 -axCORE-AVX2 -ffinite-math-only -unroll-aggressive -no-prec-div -qopt-prefetch -g -traceback -no-wrap-margin

CMAKE_Fortran_FLAGS_RELEASE -O3 -ipo -axCORE-AVX2 -ffinite-math-only -unroll-aggressive -no-prec-div -qopt-prefetch -g -traceback -no-wrap-margin

CMAKE_Fortran_FLAGS_RELWITHDEB -traceback -g -check all -warn interfaces,nouncalled -gen-interface -check noarg_temp_created

aronroland commented 1 year ago

Now, we had some questions about when to use which flags and this a very good question, which was not addressed by me.

Thanks @MatthewMasarik-NOAA for pointing this out. Here comes the story and reasoning for the above choice.

When developing new code one should ALWAYS use CMAKE_Fortran_FLAGS_RELWITHDEB, this will show errors during compilation and runtime for new code.

When the code is running without problems or warnings we can test the numerics/physics with CMAKE_Fortran_FLAGS_MINSIZEREL.

If we are happy and we move to production we should use CMAKE_Fortran_FLAGS_RELEASE.

If we have some bug that we are searching for and the code shows unexpected behavior we should use CMAKE_Fortran_FLAGS_DEBUG.

If u do not know how to set the compiler flags with cmake u can follow #822 and use ccmake as explained in this discussion.

MatthewMasarik-NOAA commented 1 year ago

Thank you @aronroland for your explanation on the compile use cases. It is a good clarification of when we should use which set of options.

Regarding supplying cmake flags, this is a different issue. This is no problem correctly supplying the arguments to the cmake call, the issue is the args we are passing (SCOTCH_NOAA_DEBUG_*) are not defined within any CMakeLists.txt, so there is no effect (the absence of the arg in ccmake output confirms as well). This is what @aerorahul had mentioned earlier, and he and @JessicaMeixner-NOAA added a fix using export's prior to the cmake call.

@JessicaMeixner-NOAA provided an updated canned-case with a scotch build script that has these updates. It is now available on the Globus untrusted location shared before (endpoint noaardhpcs#hera_untrusted), newcannedcasescotchnoaa2.tar.gz.

aronroland commented 1 year ago

Thank you @aronroland for your explanation on the compile use cases. It is a good clarification of when we should use which set of options.

Regarding supplying cmake flags, this is a different issue. This is no problem correctly supplying the arguments to the cmake call, the issue is the args we are passing (SCOTCH_NOAA_DEBUG_*) are not defined within any CMakeLists.txt, so there is no effect (the absence of the arg in ccmake output confirms as well). This is what @aerorahul had mentioned earlier, and he and @JessicaMeixner-NOAA added a fix using export's prior to the cmake call.

@JessicaMeixner-NOAA provided an updated canned-case with a scotch build script that has these updates. It is now available on the Globus untrusted location shared before (endpoint noaardhpcs#hera_untrusted), newcannedcasescotchnoaa2.tar.gz.

@MatthewMasarik-NOAA can't we just add the cpp pragma to the flags itself? E.g. if u have like -O3 -g -traceback to add to this line -DSCOTCH_whatever as in the makefile? I never tried.

MatthewMasarik-NOAA commented 1 year ago

@aronroland, do you mean put ifdef definitions for the debug flags in the scotch source files? I can't confirm since I haven't tried, but I don't see why not. I don't see the motivation though. Why would you want to edit files in the repo when you can just export a few variables in the build script?

aronroland commented 1 year ago

@aronroland, do you mean put ifdef definitions for the debug flags in the scotch source files? I can't confirm since I haven't tried, but I don't see why not. I don't see the motivation though. Why would you want to edit files in the repo when you can just export a few variables in the build script?

Hi @MatthewMasarik-NOAA I have updated the text above. I am not a specialist in CMake. Maybe it is not a good idea. However, anyway, we agreed to use GNU make for SCOTCH during the meeting.