NCAR / micm

A model-independent chemistry module for atmosphere models
https://ncar.github.io/micm/
Apache License 2.0
5 stars 5 forks source link

Share CUBLAS handle between different CUBLAS matrices #543

Closed sjsprecious closed 3 months ago

sjsprecious commented 4 months ago

This PR implements the generation of CUBLAS handle as singleton and shares between CUDA matrices. This will avoid the unnecessary creation of CUBLAS handle for each CUDA matrix and save some computational costs.

All the 44 tests pass on Derecho's A100 GPU using nvhpc/23.7.

fix #428

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.71%. Comparing base (d368ace) to head (81614c7). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #543 +/- ## ======================================= Coverage 92.71% 92.71% ======================================= Files 48 48 Lines 3475 3475 ======================================= Hits 3222 3222 Misses 253 253 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sjsprecious commented 3 months ago

When using the nvhpc/23.7 compiler on Derecho with gcc-toolchain/12, I got the the following warning messages (reported before at #401 ):

nvcc warning : The -std=c++20 flag is not supported with the configured host compiler. Flag will be ignored.
/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp(40): warning #3033-D: inline variables are a C++17 feature
        inline static cublasHandle_t handle_;
        ^

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp(21): warning #2919-D: C++17-style initializer is nonstandard in this mode
          if (auto search = cublas_handle_map.find(device_id); search == cublas_handle_map.end())
                                                             ^

/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp(40): warning #3033-D: inline variables are a C++17 feature
        inline static cublasHandle_t handle_;
        ^

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp(21): warning #2919-D: C++17-style initializer is nonstandard in this mode
          if (auto search = cublas_handle_map.find(device_id); search == cublas_handle_map.end())
                                                             ^

/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp:40:39: warning: inline variables are only available with -std=c++1z or -std=gnu++1z
       inline static cublasHandle_t handle_;
                                       ^~~~~  
/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp:41:48: warning: inline variables are only available with -std=c++1z or -std=gnu++1z
       inline static std::map<int, cublasHandle_t> cublas_handle_map;
                                                ^~~~~~~~~~~~~~~~~
/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp: In static member function ‘static micm::CublasHandleSingleton& micm::CublasHandleSingleton::GetInstance()’:
/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp:21:5: warning: init-statement in selection statements only available with -std=c++1z or -std=gnu++1z
         if (auto search = cublas_handle_map.find(device_id); search == cublas_handle_map.end())
     ^   
/glade/derecho/scratch/sunjian/micm/include/micm/util/cublas_handle_singleton.hpp:26:5: warning: init-statement in selection statements only available with -std=c++1z or -std=gnu++1z
         if (auto search = cublas_handle_map.find(device_id); search == cublas_handle_map.end())

@K20shores @mwaxmonsky @mattldawson just wonder whether there is a way to address these warning messages?

sjsprecious commented 3 months ago

@K20shores @mwaxmonsky After discussing with @mattldawson , it seems that we do not need the singleton class at all, but just a function to access the static map variable. Thus I further simplify the implementation in the latest commit.