WIP: Stokesian Dynamics

[biermanncarl]

Fixes #3241

[review-notebook-app]

Check out this pull request on 

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

[jngrad]

@biermanncarl @hmenke I'm installing blas + lapack on all docker images (espressomd/docker#155). However, many containers (Fedora, CentOS, OpenSUSE, Debian, ...) fail to build this branch due to a missing thrust header file. Is an ifdef guard for CUDA missing?

In file included from /home/espresso/espresso/libs/stokesian_dynamics/sd.hpp:4,
                 from /home/espresso/espresso/libs/stokesian_dynamics/sd_cpu.cpp:4:
/home/espresso/espresso/libs/stokesian_dynamics/device_matrix.hpp:7:10: fatal error: thrust/device_vector.h: No such file or directory
    7 | #include <thrust/device_vector.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

CMake also fails in the CUDA 10.1 docker image:

CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
CUDA_cublas_device_LIBRARY (ADVANCED)
    linked by target "sd_gpu" in directory /builds/espressomd/espresso/libs/stokesian_dynamics

This error is independent of the CMake version (tested on CMake 3.10.0, 3.11.0, 3.13.0, 3.13.3). It looks like the feature cublas_device was removed from CUDA 10.0: https://stackoverflow.com/a/55618413

The container for Ubuntu without dependencies was not updated with blas and lapack.

[hmenke]

Thrust is independent of CUDA. https://packages.ubuntu.com/bionic/libthrust-dev

[jngrad]

Then we need to reflect that requirement in the CMake logic, and make it fail if thrust is missing (either because it's now a hard requirement of espresso, or because the user enabled the SD feature).

[espresso-ci]

Your pull request does not meet our code style rules. Pylint summary:

************* Module stokesian_dynamics
testsuite/python/stokesian_dynamics.py:86:18: E0602: Undefined variable 'constraints' (undefined-variable)

You can generate these warnings with maintainer/CI/fix_style.sh. This is the same command that I have executed to generate the log above.

[jngrad]

• The GPU implementation is slow. One source of overhead is the re-allocation of matrices. Carl looked into introducing a global variable to re-use the same allocated matrix (85eb621), although he has some reservations on this strategy.
• The GPU implementation fails in CI on ROCm 3.3 with libs/stokesian_dynamics/src/device_matrix.hpp:429: static void internal::cusolver<policy::device, double>::potrf(double *, double *, int): Assertion 'info[0] == 0' failed (logfile, a printf reveals info[0] contains a random integer).
• Locally on seraue with a GeForce RTX 2080 CUDA 9.1, the GPU code is extremely slow and produces incorrect trajectories (the 3 particles have a linear trajectory and don't move very far, less than 1 unit along the y-axis), while on gerenuk with a GeForce GT 610 CUDA 9.1 and in CI the GPU trajectories are correct.

I'm unsure about the naming set_sd for both the integrator and thermostat. It could be a source of confusion with the Steepest Descent integrator. Renaming the SD thermostat and integrator to set_stokesian_dynamics resp. set_stokesian would remove that ambiguity and follow the pattern for the other methods, e.g. Brownian Dynamics handles are called set_brownian_dynamics resp. set_brownian. I don't have a strong opinion on that change though, since integrator and thermostat frameworks in espresso might get refactored in the (far) future.

[RudolfWeeber]

I'm unsure about the naming set_sd for both the integrator and thermostat. It could be a source of confusion with the Steepest Descent integrator. Renaming >the SD thermostat and integrator to set_stokesian_dynamics resp. set_stokesian would remove that ambiguity and follow the pattern for the other >methods, e.g. Brownian Dynamics handles are called set_brownian_dynamics resp. set_brownian. I don't have a strong opinion on that change though, >since integrator and thermostat frameworks in espresso might get refactored in the (far) future.

I’m in favour of renaming. Abbreviations only make sense for stuff that is used often in a script.

[hmenke]

I have some ideas how to improve the cublas initialization and salvage the Thrust wrapper, but you have locked me out of my own code.

[jngrad]

@hmenke you can pull this branch into #3241 and optimize the cublas initialization. From my side, I would like to move forward with this PR and merge it in its current state, rather soon. It took me longer than expected to get CI to pass, and I would like to avoid dealing with merge conflicts with our planned espresso refactoring projects, if possible.

[hmenke]

@jngrad After some gymnastics with git filter-branch I have extracted the libs/stokesian_dynamics folder, hopefully preserving the history correctly. The repo is located here for now, because I can't create repos in the espressomd organization: https://github.com/hmenke/espresso-stokesian-dynamics
Please review and let me know whether that is good.

[mkuron]

@hmenke, are you sure about the repo name? The library is not Espreso-specific and could be used elsewhere. Also there is still your own little integrator on the old private repo that can be used to run stand-alone.

[jngrad]

• The FetchContent URL was updated with the hmenke/espresso-stokesian-dynamics repository
• The GPU code fails in the intel:19 image (logfile) with CUDA 10.1 + thrust 1.9.6, same potrf error as on ROCm 3.3
• The GPU code works on seraue with a GeForce RTX 2080 when using CUDA 10.0 + thrust 1.9.5.

[jngrad]

The rebased SD was merged. Closing.