#1618: Compile with adaptive MPI (based on Charm++) -------------------------------+-------------------------------------------- Reporter: jtao@… | Owner: Type: enhancement | Status: new Priority: major | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: -------------------------------+--------------------------------------------
Comment (by eschnett):
No, we do not like this patch:
AMPI_TLS needs to be named CCTK_TLS. Possibly, "TLS" should be "THREAD_LOCAL" instead, so that it is more similar to the modern C and C++ standards.
The construct {{{ #ifdef AMPI CmiDisableTLS(); #endif }}} should be wrapped into a single function, probably called "CCTK_DisableThreadLocal()" or so. Same for CmiEnableTLS. In particular, the #ifdefs should go into this function, so that using it requires only 1 instead of 3 lines of code.
The various comments containing the work "hack" should instead contain an explanation of what they do. There is no need to call it "hack".
"AMPI" should become "CCTK_HAVE_AMPI", similar to other capabilities.
The header file mpi.h should be accessed via <mpi.h>, not "mpi.h".
There are some commented out lines of code that are introduced. These should not be introduced.
The code in Boundary.c seems fishy. Are these static variables really correct, even if executed by multiple threads? Does the code need to be redesigned instead?
shelob.ini should not change the default for Shelob.
The newly introduced CCTK_* macros and functions need to be documented.
But before we do any of this, it would be good to describe in an email to the developers' list which new features you propose to introduce to the flesh, and why, and what other packages (except AMPI) may want similar features, how this compares to C11 and C++11, etc.