Skip to content
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

LSHSearch Parallelization #700

Merged
merged 26 commits into from Jul 8, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
72999dd
Implements parallel query processing for LSH
mentekid Jun 19, 2016
95d417f
Implements parallel query processing for LSH
mentekid Jun 19, 2016
afcc881
Book-keeping of number of threads used
mentekid Jun 19, 2016
5db5423
Adds parallelization to bichromatic search
mentekid Jun 19, 2016
abef504
Adds Bichromatic Parallel Test
mentekid Jun 19, 2016
4cbd43e
Merge branch 'master' into lsh-parallelization
mentekid Jun 24, 2016
a60ff91
Adds parallelism option to CMakeFiles, removes most omp.h dependence
mentekid Jun 27, 2016
6152527
Removes numThreadsUsed variable, changes how maxThreads is initialized
mentekid Jun 27, 2016
7cf77cd
Changes placeholder code for OpenMP
mentekid Jun 27, 2016
2ca48c6
Fixes CMakeFiles and code to make openMP actually transparent
mentekid Jun 27, 2016
a6aca41
CMake trickery to maybe make travis not crash
mentekid Jun 27, 2016
3d536c7
Simplifies OpenMP transparency code
mentekid Jun 27, 2016
c04b073
Adds -Wno-unknown-pragmas
mentekid Jun 27, 2016
65983d1
Commit to switch branch
mentekid Jun 28, 2016
b95a3ce
Changes BaseCase to include loop over candidate set. Changes loop var…
mentekid Jul 1, 2016
0d38271
Removes commented-out old BaseCase code
mentekid Jul 1, 2016
3af80c3
Merges multiprobe LSH
mentekid Jul 1, 2016
b02e2f3
Changes BaseCase to include the loop over candidate set. Places paral…
mentekid Jul 6, 2016
a1e9c28
Merges 3fe0b72
mentekid Jul 6, 2016
ad8e6d3
Modifies CMakeLists to remove -DHAS_OPENMP
mentekid Jul 7, 2016
c4c8ff9
Removes old code and comments from CMakeLists.txt
mentekid Jul 7, 2016
074d726
Restores size_t for openMP loop counters, changes CmakeLists to requi…
mentekid Jul 8, 2016
f982ca5
Removes maxThreads functionality from LSHSearch class
mentekid Jul 8, 2016
1fb998f
Removes HAS_OPENMP definition in CMakeFiles
mentekid Jul 8, 2016
b92d465
Workaround for OpenMP 2.0 (based on dt_utils.cpp)
mentekid Jul 8, 2016
2fee61e
Transforms omp loop to reduction
mentekid Jul 8, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 11 additions & 7 deletions CMakeLists.txt
Expand Up @@ -235,13 +235,17 @@ add_definitions(-DBOOST_TEST_DYN_LINK)
# Detect OpenMP support in a compiler. If the compiler supports OpenMP, the
# flags to compile with OpenMP are returned and added.
if (HAS_OPENMP)
find_package(OpenMP)
if (OPENMP_FOUND)
add_definitions(-DOPENMP_FOUND)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
endif ()
endif ()
add_definitions(-DHAS_OPENMP)
find_package(OpenMP)
if (OPENMP_FOUND)
add_definitions(-DOPENMP_FOUND)
set(HAS_OPENMP "1")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
else ()
set(HAS_OPENMP "0")
endif ()
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, so, with what you have changed here, a user must specify HAS_OPENMP for OpenMP to be used. With the previous code, CMake would determine whether or not OpenMP support was available, and if so, OpenMP would be enabled. I think maybe it is better to use automatic detection here instead of asking the user to specify -D HAS_OPENMP; what do you think?

Copy link
Contributor Author

@mentekid mentekid Jul 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added DHAS_OPENMP for two reasons:

  • debugging without using a different compiler. This way I can know that a test passes both with openMP on and off.
  • so a user can maually "forbid" mlpack from using any openMP code, even if openMP is found on their system. I don't know why anybody would want this.

The flag is on by default, so it shouldn't bother users that don't want to mess with it.

It can definately be changed, I just had so much trouble making the build pass in AppVeyor that I went all out.


# Create a 'distclean' target in case the user is using an in-source build for
# some reason.
Expand Down
28 changes: 12 additions & 16 deletions src/mlpack/methods/lsh/lsh_search_impl.hpp
Expand Up @@ -12,6 +12,15 @@
namespace mlpack {
namespace neighbor {

inline size_t CalculateMaxThreads()
{
#ifdef OPENMP_FOUND
if (HAS_OPENMP)
return omp_get_max_threads();
return 1;
#endif
return 1;
}

// Construct the object with random tables
template<typename SortPolicy>
Expand All @@ -31,12 +40,7 @@ LSHSearch(const arma::mat& referenceSet,
bucketSize(bucketSize),
distanceEvaluations(0)
{

#ifdef OPENMP_FOUND
maxThreads = omp_get_max_threads();
#else
maxThreads = 1;
#endif
maxThreads = CalculateMaxThreads();
// Pass work to training function.
Train(referenceSet, numProj, numTables, hashWidthIn, secondHashSize,
bucketSize);
Expand All @@ -59,11 +63,7 @@ LSHSearch(const arma::mat& referenceSet,
bucketSize(bucketSize),
distanceEvaluations(0)
{
#ifdef OPENMP_FOUND
maxThreads = omp_get_max_threads();
#else
maxThreads = 1;
#endif
maxThreads = CalculateMaxThreads();
// Pass work to training function
Train(referenceSet, numProj, numTables, hashWidthIn, secondHashSize,
bucketSize, projections);
Expand All @@ -82,11 +82,7 @@ LSHSearch<SortPolicy>::LSHSearch() :
distanceEvaluations(0)
{
// Only define maxThreads. Nothing else to do.
#ifdef OPENMP_FOUND
maxThreads = omp_get_max_threads();
#else
maxThreads = 1;
#endif
maxThreads = CalculateMaxThreads();
}

// Destructor.
Expand Down