Skip to content

Clean up build configuration

Tuomas Rossi requested to merge trossi/gpaw:gpu-simplify-gpaw-python into master

This MR will simplify building gpaw-python executable and clean up related outdated configs in siteconfig.

Siteconfig changes

  • Setting parallel_python_interpreter config doesn't affect anymore the resulting _gpaw.so. Old behaviour: parallel_python_interpreter=True disabled MPI in _gpaw.so.
  • Deprecate mpilinker config. Reason: it was used only for gpaw-python build although MPI is in _gpaw.so too. Separate linker command is no more supported but can be easily added later if needed.
  • Deprecate mpi_libaries and other mpi_* configs. Reason: same as for mpilinker. Use libraries etc configs for MPI too.
  • Deprecate mpicompiler and use compiler and new mpi=True/False config instead. Reason: consistency with other boolean configs settings and simplicity. Old behaviour required setting mpicompiler=None to disable MPI. Now clearer mpi=False does the same.
    • Like before, MPI is still automatically enabled if mpicc is found by which.
  • New advanced configs: compiler_args, linker_so_args, and linker_exe_args. They are like extra_compile_args and extra_link_args, but they replace all the default compilation/linking args coming from setuptools (including -fPIC and shared -> hence "advanced"). In practice they enable the same as the old long-gone --remove-default-flags.

Code changes

  • Rewrite gpaw-python build with setuptools' compiler object (self.compiler in setup.py). This simplifies the code in config.py a lot.
  • Include deprecation logic in setup.py for the siteconfig changes.
  • Use setuptools' runtime_library_paths instead of explicit -Wl,-rpath=...
  • Define macros without dummy value, e.g. -DGPAW_WITH_FFTW=1 -> -DGPAW_WITH_FFTW
  • CI: Remove separate gpaw-python tests and do them together with other builds and tests
  • Do not force users to unset CC anymore. Reason: if user does export CC= to circumvent the CC assert, then some setuptools' default compiler arguments get lost too.
  • Split _gpaw.c to _gpaw.h, _gpaw_so.c, and main.c, where _gpaw_so.c is used for _gpaw.so and main.c for gpaw-python. No more GPAW_INTERPRETER macro. Reason: simpler distinction between _gpaw.so and gpaw-python builds.
    • globally_broadcast_bytes() moved to mpi.c and it is built only when MPI support enabled.

Note: The diff in gitlab is messy as _gpaw.c was split to _gpaw.h, _gpaw_so.c, and main.c. Use diff -u <(git show master:c/_gpaw.c) <(cat c/_gpaw.h c/_gpaw_so.c c/main.c) for better diff (shown below):

better diff
--- a/c/_gpaw.c
+++ b/c/{_gpaw.h,_gpaw_so.c,main.c}
@@ -68,7 +68,9 @@
 PyObject* spherical_harmonics(PyObject *self, PyObject *args);
 PyObject* spline_to_grid(PyObject *self, PyObject *args);
 PyObject* NewLFCObject(PyObject *self, PyObject *args);
+#ifdef PARALLEL
 PyObject* globally_broadcast_bytes(PyObject *self, PyObject *args);
+#endif
 #if defined(GPAW_WITH_SL) && defined(PARALLEL)
 PyObject* new_blacs_context(PyObject *self, PyObject *args);
 PyObject* get_blacs_gridinfo(PyObject* self, PyObject *args);
@@ -198,7 +200,9 @@
     {"pc_potential", pc_potential, METH_VARARGS, 0},
     {"spline_to_grid", spline_to_grid, METH_VARARGS, 0},
     {"LFC", NewLFCObject, METH_VARARGS, 0},
+#ifdef PARALLEL
     {"globally_broadcast_bytes", globally_broadcast_bytes, METH_VARARGS, 0},
+#endif
     {"get_num_threads", get_num_threads, METH_VARARGS, 0},
 #if defined(GPAW_WITH_SL) && defined(PARALLEL)
     {"new_blacs_context", new_blacs_context, METH_VARARGS, NULL},
@@ -296,39 +300,6 @@
 extern PyTypeObject lxcXCFunctionalType;
 #endif
 
-PyObject* globally_broadcast_bytes(PyObject *self, PyObject *args)
-{
-    PyObject *pybytes;
-    if(!PyArg_ParseTuple(args, "O", &pybytes)){
-        return NULL;
-    }
-
-#ifdef PARALLEL
-    MPI_Comm comm = MPI_COMM_WORLD;
-    int rank;
-    MPI_Comm_rank(comm, &rank);
-
-    long size;
-    if(rank == 0) {
-        size = PyBytes_Size(pybytes);  // Py_ssize_t --> long
-    }
-    MPI_Bcast(&size, 1, MPI_LONG, 0, comm);
-
-    char *dst = (char *)malloc(size);
-    if(rank == 0) {
-        char *src = PyBytes_AsString(pybytes);  // Read-only
-        memcpy(dst, src, size);
-    }
-    MPI_Bcast(dst, size, MPI_BYTE, 0, comm);
-
-    PyObject *value = PyBytes_FromStringAndSize(dst, size);
-    free(dst);
-    return value;
-#else
-    return pybytes;
-#endif
-}
-
 
 static struct PyModuleDef moduledef = {
     PyModuleDef_HEAD_INIT,
@@ -406,23 +377,18 @@
 #ifndef GPAW_WITHOUT_LIBXC
     Py_INCREF(&lxcXCFunctionalType);
 #endif
-#ifndef GPAW_INTERPRETER
-    // gpaw-python needs to import arrays at the right time, so this is
-    // done in gpaw_main().  In serial, we just do it here:
-    import_array1(0);
-#endif
     return m;
 }
-
-#ifndef GPAW_INTERPRETER
-
+#include "_gpaw.h"
 
 PyMODINIT_FUNC PyInit__gpaw(void)
 {
+    // gpaw-python needs to import arrays at the right time, so this is
+    // done in gpaw_main(). For _gpaw.so, we do it here:
+    import_array1(0);
     return moduleinit();
 }
-
-#else // ifndef GPAW_INTERPRETER
+#include "_gpaw.h"
 
 int
 gpaw_main()
@@ -522,4 +488,3 @@
 
     return status;
 }
-#endif // GPAW_INTERPRETER

Merge request reports