-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
Defect: master, src/mpi/CMakeLists.txt 3rd argument of CHECK_SYMBOL_EXISTS #457
Comments
Here is an update. I had some time available over the weekend, so I went forward with the implementation with Intel-MPI working the same way as MPICH on Linux (using mpirun, mpicc, and mpifort bash scripts). Here is my current configuration for native Win64/MSYS/GCC/Intel-MPI/OpenCoarrays-1.9.2:
Here are my changes to 1.9.2:
With a little work, these changes to build_opencoarrays.sh may be suitable for implementation in master. For example, using uname or cmake to set a variable for the -G option for all systems, and also the items with $MPI_HOME as shown.
Here is output from install.sh run from the OpenCoarrays-1.9.2 directory. Note the mpi.mod in the $MPI_HOME/include directory was built with ifort and not gfortran (and Intel does not seem to provide the .f90 source to build mpi.mod), so the test for MPI_Fortran_MODULE_COMPILES fails. Also note that the second attempt at finding SIGKILL works, as discussed above in item 3 src/mpi/CMakeLists.txt changes.
Next, I'll take a look at the symlink creation error, and let you know if I make any additional progress. |
Ah yes, this is indeed a bug thanks to yours truly. (i.e. me!) The problem is that the
👍 If we can get CMake to handle it, then maybe we won't need any major changes to install.sh... Ideally the build logic for locating extant prerequisites and building the package can all be encapsulated with CMake.
Very interesting, although it sounds like it may be a bit of a yack shave...
The point limiting to GCC < 7 (via
Seems odd, but I might chalk that up to a peculiarity of MSYS/Windows
This concerns me. I wonder if there's a bug (on our end) somewhere.
🎉 💯 🥇 Nice work! This seems like a great starting point, and like there are a number of your changes that we can incorporate. Feel free to open a Pull Request if I don't get to this soon. |
Update on the symlink creation error. The cmake documentation clearly states that -E create_symlink is "available only on UNIX". The following example shows a way to create a symlink using the Windows cmd.exe command, mklink. The example creates a symlink, build/foo -> src/bar, using the same CMakeLists.txt for both MSYS and Linux. src/CMakeLists.txt:
On Windows/MSYS, cmake 3.9.2:
On Linux, cmake 3.9.4:
Next, figure out how to get CMAKE_CURRENT_BINARY_DIR and CMAKE_CURRENT_SOURCE_DIR formatted using Windows backslash in path, instead of MSYS/Linux foward slash; as was done manually for the my_link and my_target variables used with mklink in the example CMakeLists.txt.
I'd say needing -G is more of a cmake on Windows peculiarity, where cmake defaults to the generator for visual studio.
Thanks for the comment, as I didn't pick up on the override of the version check. |
Here is the updated CMakeLists.txt that uses STRING(REPLACE to replace "/" with "\" in my_link and my_target before using mklink to create a Windows symlink (note, must have admin for mklink).
With output.
The web indicates that FILE(TO_NATIVE_PATH does not replace "/" with "\" with the -G "MSYS Makefiles" generator (although it may for the default Windows generator for Visual Studio). |
We will probably will just skip creating the link on windows. It's not critical and the wrapper script points to the actual install location. Admin privs is usually a deal breaker. The whole point of the directory name spacing is to try to allow parallel installs with different versions/compilers (thanks to the extreme lack of portability of .mod files). |
In addition to the previously discussed changes I skipped the symlink creation per the suggestion of @zbeekman, by adding an if(UNIX) / endif() block to src/mpi/CMakeLists.txt.
And voila/eureka/yippee, with the changes documented above, install.sh runs cleanly from the Win7 / MSYS2 / mingw64 / bash prompt. After adding the installation bin folder to the path, the caf and cafrun scripts work as expected, and multi-image CAF runs perform as expected. Now for some more serious testing of this CAF implementation for native Win7 / MSYS2 / Intel-MPI runtime / OpenCoarrays-1.9.2. Other than testing CAF results and performance, I see the next item to work on as the issue of cmake not finding MPI through the install.sh -M option and/or through setting the $MPI_HOME environment variable (currently cmake requires -DMPI_C_LIBRARIES, etc. to be set in prerequisites/install-functions/build_opencoarrays.sh). One idea would be to figure out what it is that cmake is looking for in the mpicc and mpifort scripts. However, I'm already using the same mpicc and mpifort scripts as MPICH 3.2 (with the paths near the beginning as the only changes). I think the key was symlink of the MPI runtime to allow using mpirun/mpicc/mpifort bash scripts. However, if the need for administrator to enable user symlinks is a major problem, adding some double quotes in a few places can probably get everything to work with the default install path of Intel-MPI (under the "Program Files (x86)" folder). If you are willing to assist me with some brief instructions on the right way to perform testing, I will compare results from this build to Linux on the same system (dual boot) and document. In #435 you mentioned needing a Windows CAF solution for your "PDE solver project starting soon"... I'm feeling pretty optimistic about gfortran/OpenCoarrays/Intel-MPI ("free" as in beer MPI runtime) for native Windows builds, based on recent progress. |
@jbmaggard Thanks for all of your tireless efforts on this! |
@jbmaggard can you send me a patch with your changes, or submit a pull request? I can manually go through your comments, but I'm worried I'll miss something or that one file will be out of sync with another. Thanks! |
Win7/MSYS2/mingw64/GCC-7.2.0/Intel-MPI/OpenCoarrays-1.9.2. OpenCoarrays-1.9.2 was built as described above. ctest works correctly with a small change to OpenCoarrays-1.9.2/CMakeLists.txt. Note mingw64/ctest is a native windows application. The change was needed so that ctest would execute the cafrun bash script under the MSYS/bash shell. This change will be documented along with the others when the suggested pull request is performed. Tests 1-46 Passed.
The image_fail... group of tests did not pass, most likely because Intel-MPI does not support needed MPI features (as @zbeekman discussed in #435). I would be interested in checking further into this, when I learn which MPI procedures and compile defines are required (the current libcaf_mpi.dll was compiled with -DUSE_FAILED_IMAGES). The test-installation-scripts.sh test did not pass because modifications to OpenCoarrays-1.9.2/CMakeLists.txt would be required to execute this script under the bash shell. |
@zbeekman Here is the patch file you requested. You may decide that most of this can be done in a way that is benign to the Linux build. For parts that are not, the UNIX, WIN32, and MSYS cmake variables are available. I installed with -i, -c -C -f and -M options of install.sh, with $MPI_HOME environment variable set, and $MPI_HOME/bin in the path (for mpirun/mpicc/mpifort bash scripts). I'm expect you'll be able to so something better than using $MPI_HOME when executing cmake in build_opencoarrays.sh. As an example, using cmake option -G "Unix Makefiles" for Linux (UNIX) versus -G "MSYS Makefiles" (WIN32 and MSYS) might be considered benign. Let me know if I can be of assistance with the concern you mentioned regarding cmake find_package(MPI) not working (which is why I added -DMPI_C_LIBRARIES, etc.).
|
Based on comments from @zbeekman, I moved ahead with an MSYS2 implementation that can work with the default installation location for Intel-MPI (which includes spaces in the path), and does not require admin permissions to implement native Win7-64 CAF using MSYS2/mingw64/GCC (7.2.0), Intel-MPI (2018.0), and OpenCoarrays-1.9.2. In addition to the previously discussed changes, double quotes were added in a few spots to allow for spaces in the default installation path for Intel-MPI (a complete set of patch files from 1.9.2 is provided below). $MPI_HOME points to the location of Intel-MPI. $HOME/mpi points to the bin folder for bash scripts (mpirun, mpicc, and mpifort), lib folder for the import library to impi.dll (called libmpi.a), and the include folder for the needed header files (mpi.h, mpif.h, mpio.h, and mpiof.h) from the Intel-MPI SDK. With both $MPI_HOME/bin and $HOME/mpi/bin in $PATH, here is how I ran install.sh for a clean build.
After adding the bin folder from the OpenCoarrays install to the path (and editing cafrun, see after 5. below), ctest gives the same results reported previously (with only the image_fail tests and installation-scripts test not passing). Here are patches for the modified files:
This last one, on cafrun.in isn't really a patch, but it shows where the problem is. If I manually edit (after install.sh completes) both the install bin/cafrun script and the prerequisites/builds/opencoarrays/1.9.2/bin/cafrun script to say Any advice on fixing @zbeekman also previously mentioned concerns about having to use Updates:
|
@jbmaggard thanks for all your tireless efforts and hard work! Sorry I haven't gotten to this, I'm pretty slammed at the moment. Fortunately, I now have a Windows 10 VM with VS 2015, Intel Compilers and MSYS2 so I can try to test, and reproduce your work once I have the time. Just wanted to let you know I haven't forgotten and that your efforts are appreciated. |
No worries. I think I'll soon be able to say the MSYS2 build is done, if you are OK with the "diff -Nu" output on the 5 files changed for the MSYS2 build of OpenCoarrays-1.9.2 (rather than as a pull request) First however, I want to take another look at the issue of cmake finding MPI without needing to add -DMPI_C_LIBRARIES, -DMPI_C_INCLUDE_PATH, -DMPI_Fortran_LIBRARIES, and -DMPI_Fortran_INCLUDE_PATH in build_opencoarrays.sh. Let me know if you need any help getting up to speed with MSYS2. |
I found the problem in FindMPI.cmake that prevents The fix to this problem is similar to the change discussed above at line 578 of OpenCoarrays-1.9.2/CMakeLists.txt, to have ctest execute the cafrun bash script under the bash shell. In FindMPI.cmake, execute_process() is used to run Here is my patch to cmake-3.9.2, FindMPI.cmake.
Here is the output from my test case.
The The I've reviewed all OpenCoarrays issues that mention "MPI_HOME", and think my application here is consistent, in that $MPI_HOME is used to find the MPI tree, as shown above (and allowing everything to work, even without admin access to the $I_MPI_ROOT installation folder of Intel-MPI for Windows). Please make a suggestion if you think I should use a name other than $MPI_HOME. |
Have you submitted a bug report/PR to kitware? You def should. I know that module was under active development recently. I'm incorporating a bunch of your patches. Most are just bugs that should be fixed, because spaces in paths should be handled correctly. |
@jbmaggard The cafrun issue is strange... this strikes me as potentially being a CMake bug... Can you show me what the line looks like before you manually edit it? |
I'm so confused about the mpiexec issue... @mpiexec@ should get expanded inside the single quotes. This means that there should be single quotes around a path without spaces or variables on the RHS of the assignment. Either CMake is throwing away the quotes when it expands, or something else is fishy in FindMPI.cmake. |
The 1.9.2 folder is from the released .tar.gz. My changes for the MSYS2 build are in the 1.9.2.MSYS folder.
|
oh, so you're saying that CMake was embedding a variable ($HOME or similar) in the contents of @mpiexec@? And that switching to double quotes solved it? |
From the installed cafrun script.
The cmake package installed is the mingw64 subsystem version, so it is a native Win64 application, so I think that is why it wants to expand to "C:/msys64/home" from "/home"; running under MSYS2/mingw64 bash prompt. For example, when build_opencoarrays.sh executes the cmake command, it has I don't quite know how to explain to kitware the issue with ctest and findMPI being OK with executable files, but needing to run scripts like cafrun or mpicc under bash (on MSYS2). |
Sorry, to clarify my questions about cafrun and @mpiexec@, last we left it you had said:
So I'm trying to figure out what the fix for you is. If it's replacing my single quotes with double quotes then great! But it's unclear to me from your description what the actual observed behavior was before you fixed it. |
When it was I think the fix is just using double quotes. |
I'm thinking that build_opencoarrays.sh could use the current (1.9.2) cmake command line if $MPI_HOME is not set (or install.sh -M not used), and use FYI, cmake changed from using ...INCLUDE_PATH (3.9.2) to ...INCLUDE_DIRS (master), in FindMPI.cmake. |
@jbmaggard I need to look into the FindMPI work around still. The other change that I have not incorporated yet is switching the script to call CMake with the windows makefile generator, because that will fail on non-windows machines. Some logic is needed to detect the OS and adjust accordingly. |
For introspection at the bash script level to call cmake correctly, uname output should work. It might be reasonable to use either Previously in this thread, I discussed some relevant cmake variables that might be useful for introspection (perhaps check WIN32 and MSYS).
My experience is that without |
Great thanks for the info. We could even call CMake in script mode if uname
is not working well.
…On Mon, Nov 13, 2017 at 10:47 AM jbmaggard ***@***.***> wrote:
For introspection at the bash script level to call cmake correctly, uname
output should work. It might be reasonable to use either -G "Unix
Makefiles", or -G "MSYS Makefiles", depending on uname output.
Previously in this thread, I discussed some relevant cmake variables that
might be useful for introspection (perhaps check WIN32 and MSYS).
message("---- Begin: CMAKE Variables")
message("---- SYSTEM=" ${CMAKE_SYSTEM} ", SYSTEM_NAME=" ${CMAKE_SYSTEM_NAME} ", SYSTEM_VERSION=" ${CMAKE_SYSTEM})
message("---- UNIX=" ${UNIX} ", WIN32=" ${WIN32} ", APPLE=" ${APPLE} ", MINGW=" ${MINGW} ", MSYS=" ${MSYS} ", CYGWIN=" ${CYGWIN})
message("---- BORLAND=" ${BORLAND} ", WATCOM=" ${WATCOM} ", MSVC=" ${MSVC})
message("---- C_COMPILER_ID=" ${CMAKE_C_COMPILER_ID} ", COMPILER_IS_GNUCC=" ${CMAKE_COMPILER_IS_GNUCC})
message("---- End: CMAKE Variables")
My experience is that without -G "MSYS Makefiles", cmake (MSYS2/mingw64)
will default to looking for Visual Studio.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#457 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAREPEGLVhmxQUBvGIoRYUeZylfxkStgks5s2ICxgaJpZM4P5nct>
.
|
Defect/Bug Report
Observed Behavior
Line 52 of src/mpi/CMakeLists.txt: Unless the 3rd argument of
CHECK_SYMBOL_EXISTS
is a new, unused variable, the function does not seem to be called (or perhaps is called, but does not produce any output or change the value of the 3rd argument).Expected Behavior
Verify SIGKILL is defined, with -D_POSIX required for GCC, MinGW-w64 on Windows (including MSYS).
Steps to Reproduce
A small test example:
SIGKILL
is found after theTrying -D_POSIX
message. SwitchingHAVE_SIGKILL_NEW
to the previously used variableHAVE_SIGKILL
results in the following output:With
SIGKILL
not found after theTrying -D_POSIX
message.Similar behavior with 3rd argument was reproduced on Linux (-G "Unix Makefiles") with cmake 3.9.3; for example, attempting to reuse the
HAVE_SIGINT
variable with one of the other calls toCHECK_SYMBOL_EXISTS
.The simple work-around for src/mpi/CMakeLists.txt would be to use a new variable for each call to
CHECK_SYMBOL_EXISTS
.I didn't see anything about this cmake feature in the documentation or during a brief web search. However, I am a very much a novice with cmake.
@zbeekman
I am continuing to progress on implementation of CAF using the native Windows MSYS gfortran, the Intel-MPI runtime, and building OpenCoarrays.
Adding a couple of symlinks and the previously discussed import library to the MPI runtime installation (#435), I have implemented bash scripts for mpirun (which simply executes the Intel mpiexec.exe), mpicc, and mpifort (mpicc and mpifort are taken from MPICH 3.2, with only the path variables near the top changed) all working together in a fashion similar to MPICH on Linux.
Minor changes to (cmake command line near end of) prerequisites/install-functions/build_opencoarrays.sh, allows install.sh to make it to the first cmake command, which is where I'm at right now. I'm currently learning about MPI builds with cmake. I'll keep you updated if/when I get to the point where it is more feasible to consider adding to install.sh.
I'm considering a way that does not require use of the header files (mpi.h, mpif.h, mpio.h, and mpiof.h) from the Intel-MPI SDK. Since Intel-MPI is ABI compatible with MPICH, it may be possible to use the MPICH header files (with MPI_Aint of type long long on Windows). With ABI compatibility, it may even be possible to build and use the mpi.mod from MPICH source code, to interface with the Intel-MPI runtime.
I recall an article at intel.com discussing using code compiled for Intel-MPI but executing under MSMPI (and/or vice versa); implying that if the runtime has the necessary MPI functions for OpenCoarrays (MSMPI currently does not, but Intel-MPI does), then the MPI functions should be accessible through the same ABI (and therefore, theoretically by using the same .h files {and/or same source code for mpi.mod}) as the MPICH runtime.
The text was updated successfully, but these errors were encountered: