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.

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

[develop] Added environment variable LDFLAGS on macOS as requested by the corresponding module file #729

Merged
merged 4 commits into from
May 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions devbuild.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggesting changes to the modulefile build_macos_gnu.lua instead of adding conditionals to devbuild.sh
Lines 59-65 of build_macos_gnu.lua to be changed to a single line adding a flag to the existing ones (note "+=" instead of "=", and spaces before "L" and after "lib")

export LDFLAGS+=" -L$MPI_ROOT/lib "

Another change that is needed for build_macos_gnu.lua in lines 49-50:

setenv("CMAKE_Platform", "macosx.gnu")
--setenv("CMAKE_Platform", "macosx.intel")

instead of "macos.<compiler>".
The ufs-weather-model expects "macosx.<compiler>" as a platform name, and it has ufs-weather-model/cmake/configure_macosx.gnu.cmake and configure_macosx.intel.cmake platform-dependent configuration files.

Copy link
Collaborator Author

@ywangwof ywangwof Apr 14, 2023

Choose a reason for hiding this comment

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

  1. export LDFLAGS+=" -L$MPI_ROOT/lib " does not work in a Lua module file. Note that it is a shell command.
  2. If we change the platform name inside the module file, build_macos_gnu.lua, the file name should also be changed to build_macosx_gnu.lua to be consistent. And all other scripts that mentioned build_macos_gnushould also be changed.

It will be involved too many modifications and is error prone. I prefer to leaving it to the next major version update? My current modification is not perfect, but it requests the minimal changes that make the script devbuild.sh work on the macOS platform.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ywangwof -
Thank you for your responses!

  1. Yes, indeed this will not work as in the bash shell. Even if we use Lua syntax, such as
    append_path (”LDFLAGS”, “ -L$MPI_ROOT/lib ", “ ”)
    it will not work either, because the $MPI_ROOT could only be expanded after the modulefile is loaded; it is not yet available inside the modulefile.

Your solution to modify devbuild.sh is then a good option. My only suggestion is to use a conditional based on a $PLATFORM variable, because we may have other compilers Intel, Clang. The line 452 in devbuild.sh then would be:
if [[ "${PLATFORM}" == "macos" ]]; then

  1. Agree. If a platform+compiler naming for the SRW differs from that for the UFS-WM, but still works for the SRW system, it's OK to leave it unchanged.

  2. Modulefiles build_macos_gnu.lua may still need a correction to the syntax for the export LDFLAGS string, suggesting:

if mode() == "load" then
  LmodMsgRaw([===[
   Please export env. variable LDFLAGS after the module is successfully loaded:
        export LDFLAGS+=" -L$MPI_ROOT/lib "
  ]===])
end

Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ CONTINUE=false
VERBOSE=false

# Turn off all apps to build and choose default later
DEFAULT_BUILD=true
DEFAULT_BUILD=true
BUILD_UFS="off"
BUILD_UFS_UTILS="off"
BUILD_UPP="off"
Expand Down Expand Up @@ -263,7 +263,7 @@ fi
RUN_VERSION_FILE="${SRW_DIR}/versions/run.ver.${PLATFORM}"
if [ -f ${RUN_VERSION_FILE} ]; then
. ${RUN_VERSION_FILE}
fi
fi

# set MODULE_FILE for this platform/compiler combination
MODULE_FILE="build_${PLATFORM}_${COMPILER}"
Expand Down Expand Up @@ -352,7 +352,7 @@ if [ "${APPLICATION}" = "ATMAQ" ]; then
cp "${SRW_DIR}/sorc/UFS_UTILS/modulefiles/build.${PLATFORM}.${COMPILER}.lua" "${EXTRN_BUILD_MOD_DIR}/mod_ufs-utils.lua"
fi
if [ "${BUILD_UPP}" = "on" ]; then
cp "${SRW_DIR}/sorc/UPP/modulefiles/${PLATFORM}.lua" "${EXTRN_BUILD_MOD_DIR}/mod_upp.lua"
cp "${SRW_DIR}/sorc/UPP/modulefiles/${PLATFORM}.lua" "${EXTRN_BUILD_MOD_DIR}/mod_upp.lua"
fi
if [ "${BUILD_NEXUS}" = "on" ]; then
cp "${SRW_DIR}/sorc/AQM-utils/parm/nexus_modulefiles/${PLATFORM}.${COMPILER}.lua" "${EXTRN_BUILD_MOD_DIR}/mod_nexus.lua"
Expand Down Expand Up @@ -449,6 +449,9 @@ if [ $USE_SUB_MODULES = true ]; then
else
module use ${SRW_DIR}/modulefiles
module load ${MODULE_FILE}
if [[ "${PLATFORM}" == "macos" ]]; then
export LDFLAGS+=" -L$MPI_ROOT/lib "
fi
fi
module list

Expand Down
2 changes: 1 addition & 1 deletion modulefiles/build_macos_gnu.lua
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ setenv("FFLAGS", " -DNO_QUAD_PRECISION -fallow-argument-mismatch ")
if mode() == "load" then
LmodMsgRaw([===[
Please export env. variable LDFLAGS after the module is successfully loaded:
> export LDFLAGS=\"-L\$MPI_ROOT/lib \" "
> export LDFLAGS+=" -L$MPI_ROOT/lib "
]===])
end

Expand Down