[MRG] OpenMP support #260

kguerda-idris · 2021-06-11T10:58:38Z

Added : OpenMP support
Restored : Epsilon and Debug mode
Replaced : parmap => deprecated since emd solver can use OpenMP

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Motivation and context / Related issue

How has this been tested (if it applies)

The speedup has been tested on multiple problems sizes.
A problem of size 120k has been solved in 57 minutes and 51 seconds using 24 CPUs

Checklist

The documentation is up-to-date with the changes I made.
I have read the CONTRIBUTING document.
All tests passed, and additional code has been covered with new tests.

Restored : Epsilon and Debug mode Replaced : parmap => multiprocessing is now replace by multithreading

rflamary · 2021-06-15T15:22:48Z

Hello @kguerda-idris this is awesome, you nearly have it and all the tests seem to be working.

Still the documenation does not seem to be building. Could you run the example plot_optim_OTreg.py on your side, it seems to be stuck when building the doc?

…hreads

rflamary · 2021-07-05T09:56:31Z

ot/lp/__init__.py

@@ -173,7 +181,7 @@ def estimate_dual_null_weights(alpha0, beta0, a, b, M):
    return center_ot_dual(alpha, beta, a, b)


-def emd(a, b, M, numItermax=100000, log=False, center_dual=True):
+def emd(a, b, M, numItermax=100000, log=False, center_dual=True, numThreads="max"):


set default to 1 as we discussed (no loss of performance on simple machines)

codecov · 2021-07-05T11:18:48Z

Codecov Report

Merging #260 (8e86752) into master (7dde9e8) will decrease coverage by 0.42%.
The diff coverage is 78.84%.

@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
- Coverage   93.07%   92.64%   -0.43%     
==========================================
  Files          17       19       +2     
  Lines        3681     3754      +73     
==========================================
+ Hits         3426     3478      +52     
- Misses        255      276      +21

rflamary · 2021-07-15T14:39:11Z

Hello,

The etst are passing now but teher is a large loss in code coverage.

Could you kease design some new tests to check that all the varaints of the solver works ?

rflamary

A few more comments on the code

rflamary · 2021-07-15T14:44:42Z

ot/helpers/openmp_helpers.py

+from pre_build_helpers import compile_test_program
+
+
+def get_openmp_flag(compiler):


Add short documentation to this function and describe what it does.

The function should be tested

rflamary · 2021-07-15T14:44:53Z

ot/helpers/openmp_helpers.py

+    return ['-fopenmp']
+
+
+def check_openmp_support():


test this function also

rflamary · 2021-07-15T14:45:29Z

ot/helpers/pre_build_helpers.py

+from numpy.distutils.command.config_compiler import config_cc
+
+
+def _get_compiler():


test this function or at least the function that is calling it

rflamary · 2021-07-15T14:46:38Z

ot/lp/EMD_wrapper.cpp

+    // beware M and C are stored in row major C style!!!
+
+    using namespace lemon;
+    int n, m, i, cur;

    typedef FullBipartiteDigraph Digraph;


remove the typedef if you don't use it anymore?

rflamary · 2021-07-15T14:48:11Z

ot/lp/__init__.py

@@ -25,6 +26,13 @@
 __all__ = ['emd', 'emd2', 'barycenter', 'free_support_barycenter', 'cvx',
           'emd_1d', 'emd2_1d', 'wasserstein_1d']

+def check_number_threads(numThreads):


add documentation to the function

rflamary · 2021-07-15T14:50:24Z

ot/lp/__init__.py

@@ -349,6 +362,9 @@ def emd2(a, b, M, processes=multiprocessing.cpu_count(),
    center_dual: boolean, optional (default=True)
        If True, centers the dual potential using function
        :ref:`center_ot_dual`.
+    numThreads: int or "max", optional (default=1)


Not clear, be clear that by defalt no OpenMP is used but if numThread>1 then attempt to use OpenMP if available

rflamary · 2021-07-15T14:51:09Z

ot/lp/__init__.py


    return res


 def free_support_barycenter(measures_locations, measures_weights, X_init, b=None, weights=None, numItermax=100,
-                            stopThr=1e-7, verbose=False, log=None):
+                            stopThr=1e-7, verbose=False, log=None, numThreads="max"):


default to 1

rflamary · 2021-07-15T14:52:05Z

ot/lp/emd_wrap.pyx

+        if numThreads == 1:
+            result_code = EMD_wrap(n1, n2, <double*> a.data, <double*> b.data, <double*> M.data, <double*> G.data, <double*> alpha.data, <double*> beta.data, <double*> &cost, max_iter)
+        else:
+            result_code = EMD_wrap_omp(n1, n2, <double*> a.data, <double*> b.data, <double*> M.data, <double*> G.data, <double*> alpha.data, <double*> beta.data, <double*> &cost, max_iter, numThreads)


what happens if numThreads==2 and omp is not present? Is there a bug or thit is compiled without OMP and just works?

If openmp is not supported, then the library is compiled without the omp flag. So every reference to OMP is ignored (pragmas are ignored, and references to omp functions like omp_get_max_threads are replaced). A warning message could be added if necessary, because it is still using the omp code (just not parallelized), which happens to be slower with 1 thread than the older (and not parallelized) version of the solver

Added : OpenMP support

a3d7107

Restored : Epsilon and Debug mode Replaced : parmap => multiprocessing is now replace by multithreading

kguerda-idris changed the title ~~Added : OpenMP support~~ [WIP] OpenMP support Jun 11, 2021

kguerda-idris and others added 6 commits June 11, 2021 12:59

Merge branch 'master' into OTOMP

692960e

Merge branch 'PythonOT:master' into OTOMP

403f74b

Commit clean up

b5f11e2

Number of CPUs correctly calculated on SLURM clusters

d4c1b3d

Corrected number of processes for cluster slurm

d6e5953

Mistake corrected

c84285a

ncassereau and others added 9 commits July 2, 2021 16:38

parmap is now deprecated

b2c958d

Now a different solver is used depending on the requested number of t…

1679a3e

…hreads

Tiny mistake corrected

ad5c0c1

Folders are now in the ot library instead of at the root

88f6aa9

Merge branch 'PythonOT:master' into OTOMP

99e51ed

Helpers is now correctly placed

b6b26cc

Merge branch 'OTOMP' of https://github.com/kguerda-idris/POT into OTOMP

c6892f4

Attempt to make compilation work smoothly

829f181

OS compatible path

b052fc2

rflamary reviewed Jul 5, 2021

View reviewed changes

NumThreads now defaults to 1

3f68dc2

ncassereau added 3 commits July 5, 2021 13:53

Better flags

42b2f42

Mistake corrected in case of OpenMP unavailability

937fec6

Revert OpenMP flags modification, which do not compile on Windows

fd25881

rflamary reviewed Jul 15, 2021

View reviewed changes

rflamary and others added 4 commits September 6, 2021 16:47

Merge branch 'master' into OTOMP

dd0c8a5

Test helper functions

dc9a475

Helpers comments

78c0934

Documentation update

47aedd2

ncassereau and others added 10 commits September 10, 2021 11:59

File title corrected

8a2bbf1

Merge branch 'master' into OTOMP

7d45569

Warning no longer using print

0258f85

Merge branch 'OTOMP' of https://github.com/kguerda-idris/POT into OTOMP

7aee5fc

Last attempt for macos compilation

2dde576

pls work

7c565ad

atempt

f86d406

solving a type error

9df20c5

TypeError OpenMP

b744a01

Compilation finally working on Windows

e3e4c37

rflamary mentioned this pull request Sep 17, 2021

[WIP] OpenMP EMD #192

Closed

7 tasks

Merge branch 'master' into OTOMP

78e4ca4

kguerda-idris changed the title ~~[WIP] OpenMP support~~ [MRG] OpenMP support Sep 20, 2021

ncassereau and others added 6 commits September 21, 2021 17:00

Bug solve, number of threads now correctly selected

71cda54

Merge branch 'OTOMP' of https://github.com/kguerda-idris/POT into OTOMP

8ca091f

64 bits solver to avoid overflows for bigger problems

d2359c5

64 bits EMD corrected

bfbb582

build wheels

0491bfe

Merge branch 'master' into OTOMP

8e86752

rflamary merged commit 1c7e7ce into PythonOT:master Sep 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] OpenMP support #260

[MRG] OpenMP support #260

kguerda-idris commented Jun 11, 2021 •

edited

Loading

rflamary commented Jun 15, 2021

rflamary Jul 5, 2021

codecov bot commented Jul 5, 2021 •

edited

Loading

rflamary commented Jul 15, 2021

rflamary left a comment

rflamary Jul 15, 2021

rflamary Jul 15, 2021

rflamary Jul 15, 2021

rflamary Jul 15, 2021

rflamary Jul 15, 2021

rflamary Jul 15, 2021

rflamary Jul 15, 2021

rflamary Jul 15, 2021

ncassereau Sep 10, 2021

		from pre_build_helpers import compile_test_program


		def get_openmp_flag(compiler):

		from numpy.distutils.command.config_compiler import config_cc


		def _get_compiler():

[MRG] OpenMP support #260

[MRG] OpenMP support #260

Conversation

kguerda-idris commented Jun 11, 2021 • edited Loading

Types of changes

Motivation and context / Related issue

How has this been tested (if it applies)

Checklist

rflamary commented Jun 15, 2021

Choose a reason for hiding this comment

codecov bot commented Jul 5, 2021 • edited Loading

Codecov Report

rflamary commented Jul 15, 2021

rflamary left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kguerda-idris commented Jun 11, 2021 •

edited

Loading

codecov bot commented Jul 5, 2021 •

edited

Loading