GraphBLAS / LAGraph

This is a library plus a test harness for collecting algorithms that use the GraphBLAS. For test coverage reports, see https://graphblas.org/LAGraph/ . Documentation: https://lagraph.readthedocs.org
Other
228 stars 61 forks source link

Parallel execution #14

Closed nicelhc13 closed 5 years ago

nicelhc13 commented 5 years ago

Hello, At first, thank you for the current parallel implementations.

I am wondering how to execute Test applications in parallel.

According to CMakeFile, it seems that there are two options for parallel execution; 1) openMP and 2) pthread.

In order to use the OpenMP, I compiled with '-DUSER_OPENMP=1' option. While generating makefile, CMake logs showed that it successfully compiled the application with OpenMP library. I also set 'OMP_NUM_THREADS' environment variable as 56.

However, when I execute applications, for example BFS, LAGraph_get_nthreads() returns '1' in the very initial stage; before calling LAGraph_set_nthreads().

To figure out what happened, I tracked the source code and I found some macros from "Source/Utility/LAGraph_get_nthreads.c.": GxB_SUITESPARSE_GRAPHBLAS and _OPENMP. But the application passed 'GxB_SUITESPARSE_GRAPHBLAS' code; so to speak, _OPENMP was not set correctly.

So could you explain how to correctly run the tests in parallel?

Thank you!

DrTimothyAldenDavis commented 5 years ago

It's not enough just to set -DUSER_OPENMP=1. You have to compile with OpenMP enabled. If you have OpenMP, cmake should find it. It will set the compile time flag -fopenmp, for example.

The _OPENMP flag is #define'd only if OpenMP is available. If a code is not compiled with OpenMP, then _OPENMP is undefined.

USER_OPENMP just defines how I do internal synchronization if the user application uses OpenMP. It does not enable OpenMP. It can't; that has to be done with a flag like -fopenmp.

On Mon, Jul 29, 2019 at 5:57 PM Hochan Lee notifications@github.com wrote:

Hello, At first, thank you for the current parallel implementations.

I am wondering how to execute Test applications in parallel.

According to CMakeFile, it seems that there are two options for parallel execution; 1) openMP and 2) pthread.

In order to use the OpenMP, I compiled with '-DUSER_OPENMP=1' option. While generating makefile, CMake logs showed that it successfully compiled the application with OpenMP library. I also set 'OMP_NUM_THREADS' environment variable as 56.

However, when I execute applications, for example BFS, LAGraph_get_nthreads() returns '1' in the very initial stage; before calling LAGraph_set_nthreads().

To figure out what happened, I tracked the source code and I found some macros from "Source/Utility/LAGraph_get_nthreads.c.": GxB_SUITESPARSE_GRAPHBLAS and _OPENMP. But the application passed 'GxB_SUITESPARSE_GRAPHBLAS' code; so to speak, _OPENMP was not set correctly.

So could you explain how to correctly run the tests in parallel?

Thank you!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AEYIIOLTJAWNTOCWVPBY62TQB5YTZA5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HCFEGNQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYIIOIPH6SEACG6JPR5U5TQB5YTZANCNFSM4IHW4YJA .

nicelhc13 commented 5 years ago

Hello Dr. Davis.

Thank you for your response. But it seems that it is compiled with OpenMP.

Below is the CMake logs for liblagraph

-- The C compiler identification is GNU 8.1.0 -- The CXX compiler identification is GNU 8.1.0 -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: ..../gcc-8.1/c7/bin/g++ -- Check for working CXX compiler: ..../gcc-8.1/c7/bin/g++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- Found OpenMP_C: -fopenmp (found version "4.5") -- Found OpenMP_CXX: -fopenmp (found version "4.5") -- Found OpenMP: TRUE (found version "4.5")
-- Looking for pthread.h -- Looking for pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - not found -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - found -- Found Threads: TRUE
-- CMAKE build type: Release -- CMAKE C Flags release: -O3 -DNDEBUG -- CMAKE compiler ID: GNU -- CMAKE thread library: -lpthread -- CMAKE have pthreads: 1 -- CMAKE have Win32 pthreads: -- CMAKE have OpenMP: TRUE -- openmp is used! -- cmake -DUSER_OPENMP=1: insisting on using OpenMP -- Using OpenMP to synchronize user threads -- CMAKE C flags: -std=c11 -lm -Wno-pragmas -O3 -DNDEBUG -fopenmp -DUSER_OPENMP_THREADS -DHAVE_PTHREADS -- Configuring done -- Generating done -- Build files have been written to: ..../build

================================================ and also, the below is pagerank

======================================

-- The C compiler identification is GNU 8.1.0 -- The CXX compiler identification is GNU 8.1.0 -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- Check for working C compiler: ...../gcc-8.1/c7/bin/gcc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: ...../gcc-8.1/c7/bin/g++ -- Check for working CXX compiler: ...../gcc-8.1/c7/bin/g++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- Found OpenMP_C: -fopenmp (found version "4.5") -- Found OpenMP_CXX: -fopenmp (found version "4.5") -- Found OpenMP: TRUE (found version "4.5")
-- Looking for pthread.h -- Looking for pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - not found -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - found -- Found Threads: TRUE
-- CMAKE build type: Release -- CMAKE C Flags release: -O3 -DNDEBUG -- CMAKE compiler ID: GNU -- CMAKE thread library: -lpthread -- CMAKE have pthreads: 1 -- CMAKE have Win32 pthreads: -- CMAKE have OpenMP: TRUE -- cmake -DUSER_OPENMP=1: insisting on using OpenMP -- Using OpenMP to synchronize user threads -- CMAKE C flags: -std=c11 -lm -Wno-pragmas -O3 -DNDEBUG -fopenmp -DUSER_OPENMP_THREADS -DHAVE_PTHREADS -- Configuring done -- Generating done -- Build files have been written to: ...../Test/PageRank/build

================================================

It seems that USER_OPENMP is still not enabled.

Would you recommend using OpenMP? Could I use pthead?

Thank you!

DrTimothyAldenDavis commented 5 years ago

USER_OPENMP is not a flag that gets passed to the compiler. It only appears in the cmake build process. If it is set, then the code gets compiled with the flag:

-DDUSER_OPENMP_THREADS

so the ifdef in the code is #ifdef USER_OPENMP_THREADS.

There's no place in the source code at all where I rely on any #define USER_OPENMP. The #define for GxB_SUITESPARSE_GRAPHBLAS has nothing to do with OpenMP. The reason you see it in LAGraph_get_nthreads is that GxB_get is a GraphBLAS extension that is not in the GraphBLAS standard.

So everything is going as planned in the build.

I don't define _OPENMP. That is defined by the compiler as a result of -fopenmp. There must be something else going wrong with your OpenMP installation.

Can you try adding this in any C program in the LAGraph test?

int nth ; info = GxB_get (GxB_NTHREADS, &nth) ; printf ("info %d nthreads %d error %s\n", info, nth, GrB_error ( )) ;

and then print out both nth and info.

Did try to call LAGraph_get_nthreads before calling GrB_init or LAGraph_init? That will fail.

On Mon, Jul 29, 2019 at 9:14 PM Hochan Lee notifications@github.com wrote:

Hello Dr. Davis.

Thank you for your response. But it seems that it is compiled with OpenMP. Below is the CMake logs for liblagraph

-- The C compiler identification is GNU 8.1.0 -- The CXX compiler identification is GNU 8.1.0 -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: ..../gcc-8.1/c7/bin/g++ -- Check for working CXX compiler: ..../gcc-8.1/c7/bin/g++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- Found OpenMP_C: -fopenmp (found version "4.5") -- Found OpenMP_CXX: -fopenmp (found version "4.5") -- Found OpenMP: TRUE (found version "4.5") -- Looking for pthread.h -- Looking for pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - not found -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - found -- Found Threads: TRUE -- CMAKE build type: Release -- CMAKE C Flags release: -O3 -DNDEBUG -- CMAKE compiler ID: GNU -- CMAKE thread library: -lpthread -- CMAKE have pthreads: 1 -- CMAKE have Win32 pthreads: -- CMAKE have OpenMP: TRUE -- openmp is used! -- cmake -DUSER_OPENMP=1: insisting on using OpenMP -- Using OpenMP to synchronize user threads -- CMAKE C flags: -std=c11 -lm -Wno-pragmas -O3 -DNDEBUG -fopenmp -DUSER_OPENMP_THREADS -DHAVE_PTHREADS -- Configuring done -- Generating done -- Build files have been written to: ..../build

================================================ and also, the below is pagerank

======================================

-- The C compiler identification is GNU 8.1.0 -- The CXX compiler identification is GNU 8.1.0 -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- Check for working C compiler: ...../gcc-8.1/c7/bin/gcc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: ...../gcc-8.1/c7/bin/g++ -- Check for working CXX compiler: ...../gcc-8.1/c7/bin/g++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- Found OpenMP_C: -fopenmp (found version "4.5") -- Found OpenMP_CXX: -fopenmp (found version "4.5") -- Found OpenMP: TRUE (found version "4.5") -- Looking for pthread.h -- Looking for pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - not found -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - found -- Found Threads: TRUE -- CMAKE build type: Release -- CMAKE C Flags release: -O3 -DNDEBUG -- CMAKE compiler ID: GNU -- CMAKE thread library: -lpthread -- CMAKE have pthreads: 1 -- CMAKE have Win32 pthreads: -- CMAKE have OpenMP: TRUE -- cmake -DUSER_OPENMP=1: insisting on using OpenMP -- Using OpenMP to synchronize user threads -- CMAKE C flags: -std=c11 -lm -Wno-pragmas -O3 -DNDEBUG -fopenmp -DUSER_OPENMP_THREADS -DHAVE_PTHREADS -- Configuring done -- Generating done -- Build files have been written to: ...../Test/PageRank/build

================================================

It seems that USER_OPENMP is still not enabled.

Would you recommend using OpenMP? Could I use pthead?

Thank you!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AEYIIOO6KHPAHDXAYTJNGLDQB6PXFA5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3CRA5I#issuecomment-516231285, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYIIOKA4EROA4RYDQJZLIDQB6PXFANCNFSM4IHW4YJA .

DrTimothyAldenDavis commented 5 years ago

I put an error check inside LAGraph_get_nthreads. Do a git pull and try it again.

On Mon, Jul 29, 2019 at 9:55 PM Tim Davis davis@tamu.edu wrote:

USER_OPENMP is not a flag that gets passed to the compiler. It only appears in the cmake build process. If it is set, then the code gets compiled with the flag:

-DDUSER_OPENMP_THREADS

so the ifdef in the code is #ifdef USER_OPENMP_THREADS.

There's no place in the source code at all where I rely on any #define USER_OPENMP. The #define for GxB_SUITESPARSE_GRAPHBLAS has nothing to do with OpenMP. The reason you see it in LAGraph_get_nthreads is that GxB_get is a GraphBLAS extension that is not in the GraphBLAS standard.

So everything is going as planned in the build.

I don't define _OPENMP. That is defined by the compiler as a result of -fopenmp. There must be something else going wrong with your OpenMP installation.

Can you try adding this in any C program in the LAGraph test?

int nth ; info = GxB_get (GxB_NTHREADS, &nth) ; printf ("info %d nthreads %d error %s\n", info, nth, GrB_error ( )) ;

and then print out both nth and info.

Did try to call LAGraph_get_nthreads before calling GrB_init or LAGraph_init? That will fail.

On Mon, Jul 29, 2019 at 9:14 PM Hochan Lee notifications@github.com wrote:

Hello Dr. Davis.

Thank you for your response. But it seems that it is compiled with OpenMP. Below is the CMake logs for liblagraph

-- The C compiler identification is GNU 8.1.0 -- The CXX compiler identification is GNU 8.1.0 -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: ..../gcc-8.1/c7/bin/g++ -- Check for working CXX compiler: ..../gcc-8.1/c7/bin/g++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- Found OpenMP_C: -fopenmp (found version "4.5") -- Found OpenMP_CXX: -fopenmp (found version "4.5") -- Found OpenMP: TRUE (found version "4.5") -- Looking for pthread.h -- Looking for pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - not found -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - found -- Found Threads: TRUE -- CMAKE build type: Release -- CMAKE C Flags release: -O3 -DNDEBUG -- CMAKE compiler ID: GNU -- CMAKE thread library: -lpthread -- CMAKE have pthreads: 1 -- CMAKE have Win32 pthreads: -- CMAKE have OpenMP: TRUE -- openmp is used! -- cmake -DUSER_OPENMP=1: insisting on using OpenMP -- Using OpenMP to synchronize user threads -- CMAKE C flags: -std=c11 -lm -Wno-pragmas -O3 -DNDEBUG -fopenmp -DUSER_OPENMP_THREADS -DHAVE_PTHREADS -- Configuring done -- Generating done -- Build files have been written to: ..../build

================================================ and also, the below is pagerank

======================================

-- The C compiler identification is GNU 8.1.0 -- The CXX compiler identification is GNU 8.1.0 -- Check for working C compiler: ..../gcc-8.1/c7/bin/gcc -- Check for working C compiler: ...../gcc-8.1/c7/bin/gcc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: ...../gcc-8.1/c7/bin/g++ -- Check for working CXX compiler: ...../gcc-8.1/c7/bin/g++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- Found OpenMP_C: -fopenmp (found version "4.5") -- Found OpenMP_CXX: -fopenmp (found version "4.5") -- Found OpenMP: TRUE (found version "4.5") -- Looking for pthread.h -- Looking for pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - not found -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - found -- Found Threads: TRUE -- CMAKE build type: Release -- CMAKE C Flags release: -O3 -DNDEBUG -- CMAKE compiler ID: GNU -- CMAKE thread library: -lpthread -- CMAKE have pthreads: 1 -- CMAKE have Win32 pthreads: -- CMAKE have OpenMP: TRUE -- cmake -DUSER_OPENMP=1: insisting on using OpenMP -- Using OpenMP to synchronize user threads -- CMAKE C flags: -std=c11 -lm -Wno-pragmas -O3 -DNDEBUG -fopenmp -DUSER_OPENMP_THREADS -DHAVE_PTHREADS -- Configuring done -- Generating done -- Build files have been written to: ...../Test/PageRank/build

================================================

It seems that USER_OPENMP is still not enabled.

Would you recommend using OpenMP? Could I use pthead?

Thank you!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AEYIIOO6KHPAHDXAYTJNGLDQB6PXFA5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3CRA5I#issuecomment-516231285, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYIIOKA4EROA4RYDQJZLIDQB6PXFANCNFSM4IHW4YJA .

nicelhc13 commented 5 years ago

Thank you for your response again. I pulled your new source code and tested it; I put the LAGraph_get_nthreads() before GrB_init(). As we expected, the application is immediately finished. But it still prints the number of threads as '1'.

I just think it is my environment's problem. However, I still don't understand; I checked compiler options and version of OpenMP. They seem like they are all normal. I will dig around for more solutions.

So, I gave up and just set the number of threads as several numbers by using LAGraph_set_nthreads(). and it showed performance improvement. To sum up, OpenMP is not used, but maybe pthreads are used. Do I have to use OpenMP? I mean, does it matter if I use pthreads instead before I locate the issue?

Thank you!!

DrTimothyAldenDavis commented 5 years ago

If you want parallelism inside GraphBLAS, then you have to use OpenMP internally, inside GraphBLAS.

But GraphBLAS allows the user application to use either OpenMP or pthreads for its own parallelism. It is thread-safe for user threads based on either model.

So what does omp_get_max_threads () report? That is what LAGraph_get_nthreads is returning.

If GxB_SUITESPARSE_GRAPHBLAS is #define'd, then the GxB_get function exists, and it returns the # of threads from omp_get_max_threads, as follows:

First, in GraphBLAS/Source/GB.h I have this:

if defined ( _OPENMP )

#include <omp.h>
#define GB_OPENMP_THREAD_ID         omp_get_thread_num ( )
#define GB_OPENMP_MAX_THREADS       omp_get_max_threads ( )
#define GB_OPENMP_GET_NUM_THREADS   omp_get_num_threads ( )

else

#define GB_OPENMP_THREAD_ID         (0)
#define GB_OPENMP_MAX_THREADS       (1)
#define GB_OPENMP_GET_NUM_THREADS   (1)

endif

Then in GxB_Global_Option_get, which is GxB_get, I get the # of threads by looking up a global value I keep. That value is initialized in GB_init, to GB_OPENMP_MAX_THREADS. If _OPENMP is defined, that is omp_get_max_threads.

So in either case, LAGraph_get_nthreads is calling omp_get_max_threads. I just have the #ifdef there to allow other GraphBLAS libraries, other than SuiteSparse:GraphBLAS, to get the # of threads.

On Mon, Jul 29, 2019 at 10:47 PM Hochan Lee notifications@github.com wrote:

Thank you for your response again. I pulled your new source code and tested it; I put the LAGraph_get_nthreads() before GrB_init(). As we expected, the application is immediately finished. But it still prints the number of threads as '1'.

I just think it is my environment's problem. However, I still don't understand; I checked compiler options and version of OpenMP. They seem like they are all normal. I will dig around for more solutions.

So, I gave up and just set the number of threads as several numbers by using LAGraph_set_nthreads(). and it showed performance improvement. To sum up, OpenMP is not used, but maybe pthreads are used. Do I have to use OpenMP? I mean, does it matter if I use pthreads instead before I locate the issue?

Thank you!!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AEYIIOJSEWQEW7452FELMFTQB62UHA5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3CVLFQ#issuecomment-516248982, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYIIOJUE5RSFDDZPQ43QVLQB62UHANCNFSM4IHW4YJA .

szarnyasg commented 5 years ago

Commit 5cd40c8a47706c8388100402b212dd571ea8ff58 introduced a compile error for me:

/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c: In function ‘LAGraph_get_nthreads’:
/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c:53:5: error: ‘info’ undeclared (first use in this function); did you mean ‘ynf’?
     info = GxB_get (GxB_NTHREADS, &nthreads) ;
     ^~~~
     ynf
/

The lines in question are: https://github.com/GraphBLAS/LAGraph/blob/5cd40c8a47706c8388100402b212dd571ea8ff58/Source/Utility/LAGraph_get_nthreads.c#L52-L54

Adding the GrB_Info type fixed the error.

DrTimothyAldenDavis commented 5 years ago

oops. ... fixed.

On Tue, Jul 30, 2019 at 9:17 AM Gabor Szarnyas notifications@github.com wrote:

5cd40c8 https://github.com/GraphBLAS/LAGraph/commit/5cd40c8a47706c8388100402b212dd571ea8ff58 introduced a compile error for me:

/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c: In function ‘LAGraph_get_nthreads’:

/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c:53:5: error: ‘info’ undeclared (first use in this function); did you mean ‘ynf’?

 info = GxB_get (GxB_NTHREADS, &nthreads) ;

 ^~~~

 ynf

/

The lines in question are:

https://github.com/GraphBLAS/LAGraph/blob/5cd40c8a47706c8388100402b212dd571ea8ff58/Source/Utility/LAGraph_get_nthreads.c#L52-L54

Adding the GrB_Info fixed the error.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AEYIIOMLU27DT7ODQOK2463QCBEN7A5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3EDXMQ#issuecomment-516438962, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYIIOKMD4JPBVOWQLLFCPDQCBEN7ANCNFSM4IHW4YJA .

nicelhc13 commented 5 years ago

Thank you for your advice. It is helpful. Now I understood what happened to me. I already installed OpenMP libary: libomp, version 4.5. As I reported before, CMake also successfully linked OpenMP during compilation.

However, in order to verify whether test applications were executed in parallel, I just checked LAGraph_get_threads() which checks GxB_SUITESPARSE_GRAPHBLAS flag at first: in order to check the number of active threads. Therefore, _OPENMP condition was not checked because both flags were set.

As you explained it, GB_OPENMP_MAX_THREADS worked for me correctly since I was already using libomp libary. So I swapped the condition check parts of the LAGraph_get_threads() and LAGraph_set_threads() like below: `

if defined (_OPENMP)

...

elif defined (GxB_SUITESPARSE_GRAPHBLAS)

...

else

end

` and now it correctly returns the maximum number of threads, which was set by OMP_NUM_THREADS.

Therefore, the previous performance improvements was came from openMP parallelism. It worked properly.

I appreciate your help.

DrTimothyAldenDavis commented 5 years ago

Oh I see the confusion.

Lagraph get threads is not meant to return the max # of threads. It returns the # of threads that GraphBLAS will use in the subsequent calls. By default, when LAGraph_init starts up, this number is obtained from omp_get_max_threads. But it can change if LAGraph_set_threads has changed it.

So your change actually breaks the code. Or more precisely, it redefines what LAgraph get threads is doing. It returns "the # of threads to use", but I should add more comments to explain this. I see how you misunderstood. The LAGraph documentation is still in progress.

So don't change the code; your fix is broken. See my next git push, with comments and examples added.

On Tue, Jul 30, 2019 at 10:46 AM Hochan Lee notifications@github.com wrote:

Thank you for your advice. It is helpful. Now I understood what happened to me. I already installed OpenMP libary: libomp, version 4.5. As I reported before, CMake also successfully linked OpenMP during compilation.

However, in order to verify whether test applications were executed in parallel, I just checked LAGraph_get_threads() which checks GxB_SUITESPARSE_GRAPHBLAS flag at first: in order to check the number of active threads. Therefore, _OPENMP condition was not checked because both flags were set.

As you explained it, GB_OPENMP_MAX_THREADS worked for me correctly since I was already using libomp libary. So I swapped the condition check parts of the LAGraph_get_threads() and LAGraph_set_threads() like below:

if defined (_OPENMP) ... #elif defined (GxB_SUITESPARSE_GRAPHBLAS) ...

else #end

and now it correctly returns the maximum number of threads, which was set by OMP_NUM_THREADS.

I appreciate your help.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AEYIIOIXFVBOVRXOMOG7ULTQCBO4BA5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3ENFRQ#issuecomment-516477638, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYIIOMI5JIVVRAUW6JD7XLQCBO4BANCNFSM4IHW4YJA .

nicelhc13 commented 5 years ago

Thank you very much Dr. Davis. Issue is solved.

tgmattso commented 5 years ago

OK, I am a bit confused just because I haven’t had time to read through the code as much as I should.

Omp_get_max_threads() as the name implies says that unless you do something different, you will not get more threads than this number for subsequent parallel regions. A system, however, may silently give you fewer threads.

What is needed, if you want to know for sure how many threads you are actually using, is inside a parallel region, call omp_get_num_threads(). Depending on the situation and the number of threads you are asking for, it may be quite a bit less than the value returned by omp_get_max_threads().

--Tim

From: Gabor Szarnyas notifications@github.com Reply-To: GraphBLAS/LAGraph reply@reply.github.com Date: Tuesday, July 30, 2019 at 7:17 AM To: GraphBLAS/LAGraph LAGraph@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [GraphBLAS/LAGraph] Parallel execution (#14)

5cd40c8https://github.com/GraphBLAS/LAGraph/commit/5cd40c8a47706c8388100402b212dd571ea8ff58 introduced a compile error for me:

/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c: In function ‘LAGraph_get_nthreads’:

/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c:53:5: error: ‘info’ undeclared (first use in this function); did you mean ‘ynf’?

 info = GxB_get (GxB_NTHREADS, &nthreads) ;

 ^~~~

 ynf

/

The lines in question are: https://github.com/GraphBLAS/LAGraph/blob/5cd40c8a47706c8388100402b212dd571ea8ff58/Source/Utility/LAGraph_get_nthreads.c#L52-L54

Adding the GrB_Info fixed the error.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AATVME3Y4T4ECXCJSY4NZC3QCBEN7A5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3EDXMQ#issuecomment-516438962, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AATVMEYUPSUXEJLKXP5P4FTQCBEN7ANCNFSM4IHW4YJA.

DrTimothyAldenDavis commented 5 years ago

This is something a bit different.

All of my #pragmas use a clause “num_threads(nthreads)”

The values of nthreads by default is what I get from omp-get-max-threads. But I can modify that.

I have a global setting of my own nthreads-max that starts as the value from omp-get-threads. Then the user can change this global int with GxB-set(GxB_NTHREADS, whatiwant ).

So all subsequent parallel regions use nthreads set to whatiwant , at most. I may reduce it for any particular region if there’ not enough work to do.

The LAGraph get/set threads is an interface to the GxB get/set functions. But if the library isn’t SuiteSparse:GraphBLAS then LAGraph has to do something else because the GxB are my own extensions.

This whole feature makes no use of omp _get_num_threads. I rarely need to know that, if ever.

I just want to control the max threads in each region, for several reasons. If the work is tiny I use fewer threads. Also if the user already had enough parallelism they may wish to tell me to use fewer. Say on an 8 core system the user makes 4 parallel calls to GraphBLAS. Then if they tell me to use 2 threads, things will go better. Otherwise I would use 8 threads in each operation which is too many.

The nthreads setting can be done globally or in a descriptor, per operation.

On Tue, Jul 30, 2019 at 9:49 PM Tim Mattson notifications@github.com wrote:

OK, I am a bit confused just because I haven’t had time to read through the code as much as I should.

Omp_get_max_threads() as the name implies says that unless you do something different, you will not get more threads than this number for subsequent parallel regions. A system, however, may silently give you fewer threads.

What is needed, if you want to know for sure how many threads you are actually using, is inside a parallel region, call omp_get_num_threads(). Depending on the situation and the number of threads you are asking for, it may be quite a bit less than the value returned by omp_get_max_threads().

--Tim

From: Gabor Szarnyas notifications@github.com Reply-To: GraphBLAS/LAGraph reply@reply.github.com Date: Tuesday, July 30, 2019 at 7:17 AM To: GraphBLAS/LAGraph LAGraph@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [GraphBLAS/LAGraph] Parallel execution (#14)

5cd40c8< https://github.com/GraphBLAS/LAGraph/commit/5cd40c8a47706c8388100402b212dd571ea8ff58> introduced a compile error for me:

/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c: In function ‘LAGraph_get_nthreads’:

/home/ubuntu/git/LAGraph/Source/Utility/LAGraph_get_nthreads.c:53:5: error: ‘info’ undeclared (first use in this function); did you mean ‘ynf’?

info = GxB_get (GxB_NTHREADS, &nthreads) ;

^~~~

ynf

/

The lines in question are:

https://github.com/GraphBLAS/LAGraph/blob/5cd40c8a47706c8388100402b212dd571ea8ff58/Source/Utility/LAGraph_get_nthreads.c#L52-L54

Adding the GrB_Info fixed the error.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub< https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AATVME3Y4T4ECXCJSY4NZC3QCBEN7A5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3EDXMQ#issuecomment-516438962>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AATVMEYUPSUXEJLKXP5P4FTQCBEN7ANCNFSM4IHW4YJA>.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/GraphBLAS/LAGraph/issues/14?email_source=notifications&email_token=AEYIIOJDFXI2BD5LBMKBBFLQCD4SVA5CNFSM4IHW4YJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3F4U2Y#issuecomment-516672107, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYIIOO2GJZQN2SPWM6MKC3QCD4SVANCNFSM4IHW4YJA .

-- Sent from Gmail Mobile