mpi: add testing support for openmpi5 #16
Conversation
mpi/inner_script.sh
Outdated
| if [[ $MPI == "openmpi5" ]]; then | ||
| module use -q /usr/global/tools/mpi/toss_4_x86_64_ib/modulefiles/Core/ | ||
| fi |
There was a problem hiding this comment.
There should probably be a comment here about why this module use line is needed.
Should it also be a fatal error if module use fails?
There was a problem hiding this comment.
I don't think module use throws any errors, or, at least, it's very easily fooled. However, if the module isn't loaded, openmpi/5 won't be found and that will kill the script. (lines 20-21)
(s=125,d=0) corona82 ~ $ mkdir junkdir
(s=125,d=0) corona82 ~ $ module use /g/g0/hobbs17/junkdir/
(s=125,d=0) corona82 ~ $ vim junkdir/hello.txt
(s=125,d=0) corona82 ~ $ module use /g/g0/hobbs17/junkdir/
(s=125,d=0) corona82 ~ $ module use /g/g0/hobbs17/junkdir/hello.txt
(s=125,d=0) corona82 ~ $ $?
bash: 0: command not found...
mpi/inner_script.sh
Outdated
| export NAME="$COMPILER"_"$MPI" | ||
|
|
||
| test -n $COMPILER || die "COMPILER (argument 1) not set" | ||
| test -n $MPI || die "MPI (argument 2) not set" | ||
| if [[ $MPI == "openmpi5" ]]; then |
There was a problem hiding this comment.
Would it be simpler to avoid removing the first / from the MPI name and instead just have
if [[ $MPI == "openmpi/5" ]]; thenThis would also address the comment below regarding module load $2
mpi/inner_script.sh
Outdated
| module load $COMPILER || die "Compiler $COMPILER is unavailable on $LCSCHEDCLUSTER" | ||
| module load $MPI || die "MPI implementation $MPI is unavailable on $LCSCHEDCLUSTER" | ||
| module load $2 || die "MPI implementation $MPI is unavailable on $LCSCHEDCLUSTER" |
There was a problem hiding this comment.
A suggestion would be to avoid using the shell position parameters in this part of the script (just for your future sanity).
Maybe use
MPI_MODULENAME=$2
MPI=${MPI_MODULENAME///} # bash-specificSo $MPI_MODULENAME contains the MPI module name and MPI contains a string with / stripped from the name.
However, I don't think this is necessary at this point (see above)
There was a problem hiding this comment.
Well, it's kinda necessary. The module name with a / in it can't be used in line 25 to create a directory (it'll end up trying to create, for example, a subdirectory 5 under clang_openmpi and fail.)
However, I see now on line 13 I'm creating another variable called name for the compiler_mpi name. Duh! I could just sed that, and keep things cleaner later. Fixing.
Problem: flux-pmix is required by openmpi5, which means we should be testing openmpi5 on LC hardware. Utilize modulefiles on Corona to test this. Since loading new modulefiles can cause unexpected behavior in tests for openmpi4, only load those modulefiles when openmpi5 is the requested MPI to test.
|
Thanks @grondo! |
Problem: flux-pmix is required by openmpi5, which means we should be testing openmpi5 on LC hardware.
Utilize modulefiles on Corona to test this. Since loading new modulefiles can cause unexpected behavior in tests for openmpi4, only load those modulefiles when openmpi5 is the requested MPI to test.