Bugzilla – Full Text Bug Listing |
Summary: | openmpi invalid suffix on literal | ||
---|---|---|---|
Product: | ns-3 | Reporter: | Tom Henderson <tomh> |
Component: | mpi | Assignee: | George Riley <riley> |
Status: | REOPENED --- | ||
Severity: | normal | CC: | joshpelkey, ns-bugs, pdbarnes |
Priority: | P5 | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | All |
Description
Tom Henderson
2016-06-11 17:21:53 EDT
pushed workaround in changeset 12157:ffd079a4b2e4 For the record, this appears to have been fixed in upstream OpenMPI-1.7 (feature), released Apr 2, 2013, and OpenMPI-1.8 (stable) Mar 31, 2014. https://github.com/open-mpi/ompi-release/blob/v1.7/opal/include/opal/opal_portable_platform.h OpenMPI-1.10 is current, released Aug 2015. Instead of the patch pushed in r12157:ffd079a4b2e4 which disables a warning for all versions, should we instead require OpenMPI >= 1.7? From the shell: $ ompi_info | grep "Open MPI:" | cut -d ':' -f 2 | cut -d '.' -f 1-2 1.10 gets me the major.minor version number. I need help Pythonizing this into src/mpi/wscript. The conditionals around detection of OpenMPI then MPICH will have to be refactored a bit too. (In reply to Peter Barnes from comment #2) > For the record, this appears to have been fixed in upstream OpenMPI-1.7 > (feature), released Apr 2, 2013, and OpenMPI-1.8 (stable) Mar 31, 2014. > > https://github.com/open-mpi/ompi-release/blob/v1.7/opal/include/opal/ > opal_portable_platform.h > > OpenMPI-1.10 is current, released Aug 2015. > > Instead of the patch pushed in r12157:ffd079a4b2e4 which disables a warning > for all versions, should we instead require OpenMPI >= 1.7? > > From the shell: > > $ ompi_info | grep "Open MPI:" | cut -d ':' -f 2 | cut -d '.' -f 1-2 > 1.10 > > gets me the major.minor version number. I need help Pythonizing this into > src/mpi/wscript. The conditionals around detection of OpenMPI then MPICH > will have to be refactored a bit too. I believe that r12157 is not sufficient anyway since a user reported to me that clang does not support -Wno-literal-suffix. My only reservation for the version detection such as above is whether enough systems have OpenMPI >= 1.7 (for instance, Ubuntu 14.04 is still on 1.6), and whether ompi_info is typically installed (on Fedora, it is not provided by openmpi or openmpi-devel; I didn't even find it in openmpi-debuginfo) The first OpenMPI version which lists ompi_info in the docs is v1.3.4 https://www.open-mpi.org/doc/v1.3/ We're not above telling folks to update truly obsolete packages :) https://groups.google.com/d/msg/ns-3-users/4ZFbFhDP-wM/E1VCE2GLCwAJ This may qualify. If you're sophisticated enough to use MPI, you ought to be sophisticated enough to use your package manager. In any case, I can confirm that the patch doesn't work with Apple LLVM version 7.0.2 (clang-700.1.81) (In reply to Peter Barnes from comment #4) > The first OpenMPI version which lists ompi_info in the docs is v1.3.4 > https://www.open-mpi.org/doc/v1.3/ > > We're not above telling folks to update truly obsolete packages :) > https://groups.google.com/d/msg/ns-3-users/4ZFbFhDP-wM/E1VCE2GLCwAJ I agree and I don't have a problem to tell people to upgrade MPI. I have no problem reverting my workaround patch. The question I have is how to gracefully handle the configure version checking when ompi_info is not installed. I couldn't find a package in Fedora providing it. If we find ompi_info is provided by the packages, we can list this as a prerequisite. Or, is there another way to version check? It looks like this stanza in mpi.h was introduced in released version 1.1 /* Major, minor, and release version of Open MPI */ #define OMPI_MAJOR_VERSION 1 #define OMPI_MINOR_VERSION 10 #define OMPI_RELEASE_VERSION 3 A little conf.check_nonfatal(fragment=... and were there. I reverted the original patch and agree that we should pursue a conf.check-based solution. |