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

Warn differences in OpenMP settings. #1382

Merged
merged 1 commit into from Apr 30, 2018

Conversation

Projects
None yet
2 participants
@rcurtin
Member

rcurtin commented Apr 26, 2018

If the user is compiling their application with OpenMP but mlpack was not compiled with OpenMP (or vice versa) we issue a warning. This situation can cause disaster with recent versions of Armadillo.

This solves #1358. (Or, at least, it tells the user what is wrong in situations that can lead to #1358.)

Warn differences in OpenMP settings.
If the user is compiling their application with OpenMP but mlpack was not
compiled with OpenMP (or vice versa) we issue a warning.  This situation can
cause disaster with recent versions of Armadillo.
@rcurtin

This comment has been minimized.

Member

rcurtin commented Apr 26, 2018

Example output:

$ g++ -o test test.cpp -std=c++11 -lmlpack -larmadillo -lboost_program_options  -fopenmp
In file included from include/mlpack/prereqs.hpp:113:0,
                 from include/mlpack/core.hpp:250,
                 from test.cpp:1:
include/mlpack/core/util/arma_config_check.hpp:47:23: note: #pragma message: mlpack was compiled without OpenMP support, but you are compiling with OpenMP support (either -fopenmp or another option).  This will almost certainly cause irreparable disaster.  Either compile your application *without* OpenMP support (i.e. remove -fopenmp or another flag), or, recompile mlpack with OpenMP support.
       #pragma message "mlpack was compiled without OpenMP support, but you are \
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 compiling with OpenMP support (either -fopenmp or another option).  This will \
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 almost certainly cause irreparable disaster.  Either compile your application \
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 *without* OpenMP support (i.e. remove -fopenmp or another flag), or, recompile \
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 mlpack with OpenMP support."
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

or

$ g++ -o test test.cpp -std=c++11 -lmlpack -larmadillo -lboost_program_options 
In file included from include/mlpack/prereqs.hpp:113:0,
                 from include/mlpack/core.hpp:250,
                 from test.cpp:1:
include/mlpack/core/util/arma_config_check.hpp:55:23: note: #pragma message: mlpack was compiled with OpenMP support, but you are compiling without OpenMP support.  This will almost certainly cause irreparable disaster.  Either enable OpenMP support in your application (e.g., add -fopenmp to your compiler command line), or, recompile mlpack *without* OpenMP support.
       #pragma message "mlpack was compiled with OpenMP support, but you are \
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 compiling without OpenMP support.  This will almost certainly cause \
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 irreparable disaster.  Either enable OpenMP support in your application (e.g., \
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 add -fopenmp to your compiler command line), or, recompile mlpack *without* \
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 OpenMP support."
 ~~~~~~~~~~~~~~~~ 
@zoq

This comment has been minimized.

Member

zoq commented Apr 26, 2018

I don't think there is anything wrong with the style, if you agree I'll go ahead and whitelist this file, to avoid any issue with further PR's.

@zoq

zoq approved these changes Apr 26, 2018

Looks good to me, no comments from my side.

@rcurtin

This comment has been minimized.

Member

rcurtin commented Apr 30, 2018

Agreed, I think we have to whitelist the file unfortunately. I'll go ahead and merge this now then.

@rcurtin rcurtin merged commit f1fb935 into mlpack:master Apr 30, 2018

3 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Style Checks Build finished.
Details
Memory Checks
Details
Static Code Analysis Checks Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@rcurtin rcurtin deleted the rcurtin:fopenmp-warning branch Apr 30, 2018

zoq added a commit to mlpack/jenkins-conf that referenced this pull request Apr 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment