Interesting work. I would suggest one improvement that does not involve much work. Would it be possible to have a more generic interface for the GPU side since you make direct calls to CUDA while cp2k will most probably runs on AMD hardware very soon.

What I am thinking of is to just isolate the cuda depend part to its dedicated directory like it is done with collocate and integrate.

Interesting work.

Thank you :-)

runs on AMD hardware very soon.

Yes, it would be great if you could help with the HIP backend. Hopefully, Nvidia will step up for the CUDA backend. The current kernel is really just a placeholder.

Would it be possible to have a more generic interface for the GPU

Yes, the code is already laid out to support multiple backends. One only has to hook into these four routines.

What I am thinking of is to just isolate the cuda depend part to its dedicated directory

Yes, we should definitely create a sub-directory once a backend outgrows a single file.

Interesting work.

Thank you :-)

I think anything that make the code cleaner is welcome. Added bonus if it speed things up ;)

runs on AMD hardware very soon.

Yes, it would be great if you could help with the HIP backend. Hopefully, Nvidia will step up for the CUDA backend. The current kernel is really just a placeholder.

I think I can get that running fast at least to put things in place for hip. kernels optimization will come later but for now I can just use the kernels you wrote since hip or cuda syntax are almost the same.

Would it be possible to have a more generic interface for the GPU

Yes, the code is already laid out to support multiple backends. One only has to hook into these four routines.

What I am thinking of is to just isolate the cuda depend part to its dedicated directory

Yes, we should definitely create a sub-directory once a backend outgrows a single file.
Then better split as soon as possible. I can take care of the split and make a PR on your branch.

I think anything that make the code cleaner is welcome.

Yes, I was able to reduce the code size significantly (> 10x) which hopefully allows us to keep up with future hardware changes.

Added bonus if it speed things up ;)

According to my measurements there is already a clear speedup with OpenMP, otherwise it's currently a draw. Since I didn't spend much time on optimization there should still be plenty of headroom.

I think I can get that running fast at least to put things in place for hip.

Awesome!

Btw, the code is also pretty much ready for CUDA-Aware MPI, but I'm not using it just yet.

Timings for the GW_PBE_4benzene.inp benchmark are looking good, 3x speedup with CUDA despite the naïve kernel:

That's impressive. It means a lot of time is spend not doing calculations.

Also, just using shared memory for the kernel would reduce the runtime even more. I think the basic kernel from the cuda documentation can be adapted without too much effort.

• I already have the hip support done I just need to make pr to your local repo so you can look at it. But it is basically the same code (not duplicated). I will try to find time to write the dgemm kernel asap. Just locked with hip_fft that misbehaves.
• Question : Why do you use atomicAdd in your kernel ?

I did some tests of my own, on 16 Piz Daint GPU nodes. The system is 64 water molecules (HF, cc-TZ, ADMM-cpFIT3) with the newly implemented RI-HFX method which relies heavily on tensor contractions. I tested both the MO flavor (which tends to have good GPU performance) and the RHO flavor (in the best case GPU and no GPU runs are equally fast). Here are some timings:

MO flavor, 1 MPI rank and 12 OMP threads per node:

MO flavor, 6 MPI ranks and 2 OMP threads per node:

RHO flavor, 1 MPI rank and 12 OMP threads per node:

RHO flavor, 6 MPI ranks and 2 OMP threads per node:

Generally, the new library has similar performances as CPU only runs. In the cases where the current DBCSR tensor backend performs well (MO flavor, rather dense and large blocks), the new library is not quite there yet. However, it seems to be more robust, in the sense that there are not occurrence where the timings explode (contrary to GPU SCF in last table). This looks quite good and I am very excited to see what a more optimized kernel will yield! I'd be happy to repeat those tests at each new update.

The kernel needs some attention. I understand perfectly that @oschuett did not want to spend time on this until the full code is working. I will give it a try as well.

Thanks a lot @abussy for running those additional benchmarks!

So, unless there are any objects I'll merge this PR later today.

Btw, DBCSR's performance fluctuates because of optimizations like auto-tuned kernels and calling cuBLAS. These optimizations are "binary" in nature, ie. it's hit or miss. For linear scaling DFT this was usually not a problem. However for tensors we apparently need a more "contiguous" behavior.

Once this PR is merged, I'll open some issues to start the discussions on possible followup improvements.

No objections on my side. I can make a PR specifically for the hip backend afterwards.

The GCC 5 test is failing because it lacks support for ALLOCATE(var, source=...).

While we could adopt DBCSR's workaround, I'm wondering if we should keep the clean code and drop GCC 5 support instead?

We dropped GCC 6 over a year ago. Our next release is still many months away. By then GCC 5 will be over 7 years old.

I vote to drop GCC 5...

BTW, CCE is reporting the following error:

ftn-1904 crayftn: ERROR DBM_CREATE_FROM_TEMPLATE, File = ../../../../../home/users/alazzaro/cp2k/CP2K_AUTO_REGTEST/CRAY-XC40-cce/psmp/cp2k/src/dbm/dbm_api.F, Line = 271, Column = 39 
  Dummy argument "ROW_BLOCK_SIZES" has the CONTIGUOUS and POINTER attributes.  It requires an actual argument with the CONTIGUOUS and POINTER attributes.


ftn-1904 crayftn: ERROR DBM_CREATE_FROM_TEMPLATE, File = ../../../../../home/users/alazzaro/cp2k/CP2K_AUTO_REGTEST/CRAY-XC40-cce/psmp/cp2k/src/dbm/dbm_api.F, Line = 272, Column = 39 
  Dummy argument "COL_BLOCK_SIZES" has the CONTIGUOUS and POINTER attributes.  It requires an actual argument with the CONTIGUOUS and POINTER attributes.

The CCE error appears to be a false alert. As a workaround I'll try storing the pointers in intermediate variables to make it easier for the compiler to track the CONTIGUOUS attribute.