Switch the C compiler for github windows action for mpiintel to the Intel C Complier #5656
Conversation
byrnHDF
added
the
Component - Testing
byrnHDF
requested review from
derobins,
fortnern,
jhendersonHDF,
qkoziol,
vchoi-hdfgroup,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
| @@ -165,6 +190,16 @@ jobs: | |||
| sudo apt install gcc-12 g++-12 gfortran-12 | |||
| sudo apt install libaec0 libaec-dev | |||
|
|
|||
| # This causes an issue when MPI tries to install likely because | |||
There was a problem hiding this comment.
Could use a comma, but this comment of course means this is a best guess at the problem. However, I'm almost certain that this is due to both the setup-fortran and setup-mpi actions calling the setvars.sh script from OneAPI, which end up conflicting with each other after the first call. I don't believe this is caused by different versions.
|
@ellipses-dev, review PR |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to f4a223a in 2 minutes and 31 seconds. Click for details.
- Reviewed
83lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/cmake-par-script.yml:127
- Draft comment:
Using matrix.fortran for -DHDF5_BUILD_FORTRAN is a neat solution. Ensure the values are always provided as 'ON' or 'OFF' to avoid misconfiguration, especially since the Linux matrix doesn’t set this variable. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_IebYIzLIqauvHB6m
You can customize 
| @@ -120,14 +136,15 @@ jobs: | |||
| set (MODEL "MPI") | |||
| set (GROUP "MPI") | |||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} --log-level=VERBOSE") | |||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DCMAKE_TOOLCHAIN_FILE:STRING=${{ matrix.toolchain }}") | |||
There was a problem hiding this comment.
Avoid passing an empty toolchain flag for non-intel builds. Consider conditionally appending -DCMAKE_TOOLCHAIN_FILE only when matrix.toolchain is non‐empty.
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DCMAKE_BUILD_TYPE=${{ inputs.build_mode }}") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DMPIEXEC_NUMPROC_FLAG:STRING=-n") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DMPIEXEC_MAX_NUMPROCS:STRING=2") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_ENABLE_PARALLEL:BOOL=ON") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_ENABLE_SUBFILING_VFD:BOOL=OFF") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_BUILD_JAVA:BOOL=OFF") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_BUILD_CPP_LIB:BOOL=OFF") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_BUILD_FORTRAN:BOOL=OFF") | ||
| set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_BUILD_FORTRAN:BOOL=${{ matrix.fortran }}") |
There was a problem hiding this comment.
We should give a reason for switching off Fortran for future reference.
Fixes #5652
Important
Switches C compiler to Intel C Compiler for
intelmpion Windows in GitHub Actions workflow.intelmpiincmake-par-script.yml.config/toolchain/intel.cmake) and enables Fortran.oneAPIsetup step forintelmpi.intelmpibut comments outoneAPIsetup due to issues.HDF5_BUILD_FORTRANoption to use matrix variable inHDF5options.cmake.This description was created by
for f4a223a. You can customize this summary. It will automatically update as commits are pushed.