From 91b0ae798a5cfdb1bfe95fee321a26fd4ac3ef73 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 4 Sep 2021 12:41:52 -0400 Subject: [PATCH 1/5] make VALUELENGTH constant consistent. --- src/PYTHON/python_impl.cpp | 4 +--- src/variable.cpp | 1 - src/variable.h | 1 + 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/PYTHON/python_impl.cpp b/src/PYTHON/python_impl.cpp index 2da5f68895..29a2ea9dba 100644 --- a/src/PYTHON/python_impl.cpp +++ b/src/PYTHON/python_impl.cpp @@ -40,8 +40,6 @@ using namespace LAMMPS_NS; enum{NONE,INT,DOUBLE,STRING,PTR}; -#define VALUELENGTH 64 // also in variable.cpp - /* ---------------------------------------------------------------------- */ @@ -369,7 +367,7 @@ void PythonImpl::invoke_function(int ifunc, char *result) const char *pystr = PY_STRING_AS_STRING(pValue); if (pfuncs[ifunc].longstr) strncpy(pfuncs[ifunc].longstr,pystr,pfuncs[ifunc].length_longstr); - else strncpy(result,pystr,VALUELENGTH-1); + else strncpy(result,pystr,Variable::VALUELENGTH-1); } } Py_CLEAR(pValue); diff --git a/src/variable.cpp b/src/variable.cpp index ba34ea2a78..a59a24381f 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -49,7 +49,6 @@ using namespace MathConst; #define MAXLEVEL 4 #define MAXLINE 256 #define CHUNK 1024 -#define VALUELENGTH 64 // also in python.cpp #define MAXFUNCARG 6 #define MYROUND(a) (( a-floor(a) ) >= .5) ? ceil(a) : floor(a) diff --git a/src/variable.h b/src/variable.h index 845145d336..dbad793b57 100644 --- a/src/variable.h +++ b/src/variable.h @@ -72,6 +72,7 @@ class Variable : protected Pointers { PYTHON, INTERNAL }; + static constexpr int VALUELENGTH = 64; private: int me; From 0286c3e2be868fba614d980b52944d2d0bd6f9e0 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 4 Sep 2021 12:45:59 -0400 Subject: [PATCH 2/5] treat Py_Finalize() more like MPI_Finalize() this is done by - not automatically calling Py_Finalize() when destructing a python interpreter - adding wrapper functions so that the call to Py_Finalize() is hidden and skipped if Python support is no included. - call the Python::finalize() wrapper in main.cpp (similar to the equivalent Kokkos function) - add a wrapper of that call to the C library interface --- doc/src/Library_create.rst | 11 ++++++++++ src/PYTHON/python_impl.cpp | 17 ++++++-------- src/PYTHON/python_impl.h | 4 ++-- src/library.cpp | 45 ++++++++++++++++++++++++++++++++++++-- src/library.h | 1 + src/lmppython.cpp | 9 ++++++++ src/lmppython.h | 1 + src/main.cpp | 7 ++++++ tools/swig/lammps.i | 2 ++ 9 files changed, 83 insertions(+), 14 deletions(-) diff --git a/doc/src/Library_create.rst b/doc/src/Library_create.rst index b3fea4b89e..3566cb3cc9 100644 --- a/doc/src/Library_create.rst +++ b/doc/src/Library_create.rst @@ -10,6 +10,7 @@ This section documents the following functions: - :cpp:func:`lammps_mpi_init` - :cpp:func:`lammps_mpi_finalize` - :cpp:func:`lammps_kokkos_finalize` +- :cpp:func:`lammps_python_finalize` -------------------- @@ -104,3 +105,13 @@ calling program. .. doxygenfunction:: lammps_mpi_finalize :project: progguide + +----------------------- + +.. doxygenfunction:: lammps_kokkos_finalize + :project: progguide + +----------------------- + +.. doxygenfunction:: lammps_python_finalize + :project: progguide diff --git a/src/PYTHON/python_impl.cpp b/src/PYTHON/python_impl.cpp index 29a2ea9dba..08fba05367 100644 --- a/src/PYTHON/python_impl.cpp +++ b/src/PYTHON/python_impl.cpp @@ -63,10 +63,6 @@ PythonImpl::PythonImpl(LAMMPS *lmp) : Pointers(lmp) #endif #endif - // one-time initialization of Python interpreter - // pyMain stores pointer to main module - external_interpreter = Py_IsInitialized(); - #ifdef MLIAP_PYTHON // Inform python intialization scheme of the mliappy module. // This -must- happen before python is initialized. @@ -108,12 +104,6 @@ PythonImpl::~PythonImpl() } } - // shutdown Python interpreter - if (!external_interpreter) { - PyGILState_Ensure(); - Py_Finalize(); - } - memory->sfree(pfuncs); } @@ -545,3 +535,10 @@ bool PythonImpl::has_minimum_version(int major, int minor) { return (PY_MAJOR_VERSION == major && PY_MINOR_VERSION >= minor) || (PY_MAJOR_VERSION > major); } + +/* ------------------------------------------------------------------ */ + +void PythonImpl::finalize() +{ + if (Py_IsInitialized()) Py_Finalize(); +} diff --git a/src/PYTHON/python_impl.h b/src/PYTHON/python_impl.h index dd215fdedf..fe14862186 100644 --- a/src/PYTHON/python_impl.h +++ b/src/PYTHON/python_impl.h @@ -20,9 +20,8 @@ namespace LAMMPS_NS { class PythonImpl : protected Pointers, public PythonInterface { - public: - bool external_interpreter; + public: PythonImpl(class LAMMPS *); ~PythonImpl(); void command(int, char **); @@ -33,6 +32,7 @@ class PythonImpl : protected Pointers, public PythonInterface { int execute_string(char *); int execute_file(char *); bool has_minimum_version(int major, int minor); + static void finalize(); private: void *pyMain; diff --git a/src/library.cpp b/src/library.cpp index b60f8659ee..1acb61c66d 100644 --- a/src/library.cpp +++ b/src/library.cpp @@ -33,17 +33,18 @@ #include "group.h" #include "info.h" #include "input.h" +#include "lmppython.h" #include "memory.h" #include "modify.h" #include "molecule.h" #include "neigh_list.h" #include "neighbor.h" -#include "region.h" -#include "respa.h" #include "output.h" #if defined(LMP_PLUGIN) #include "plugin.h" #endif +#include "region.h" +#include "respa.h" #include "thermo.h" #include "timer.h" #include "universe.h" @@ -339,6 +340,8 @@ function no more MPI calls may be made. .. versionadded:: 18Sep2020 +*See also* + :cpp:func:`lammps_kokkos_finalize`, :cpp:func:`lammps_python_finalize` \endverbatim */ void lammps_mpi_finalize() @@ -369,6 +372,8 @@ After calling this function no Kokkos functionality may be used. .. versionadded:: 2Jul2021 +*See also* + :cpp:func:`lammps_mpi_finalize`, :cpp:func:`lammps_python_finalize` \endverbatim */ void lammps_kokkos_finalize() @@ -376,6 +381,42 @@ void lammps_kokkos_finalize() KokkosLMP::finalize(); } +/* ---------------------------------------------------------------------- */ + +/** Clear the embedded Python environment + * +\verbatim embed:rst + +This function resets and clears an embedded Python environment +by calling the `Py_Finalize() function +`_ +of the embedded Python library, if enabled. +This call would free up all allocated resources and release +loaded shared objects. + +However, this is **not** done when a LAMMPS instance is deleted because +a) LAMMPS may have been used through the Python module and thus +the Python interpreter is external and not embedded into LAMMPS +and therefore may not be reset by LAMMPS b) some Python modules +and extensions, most notably NumPy, are not compatible with being +initialized multiple times, which would happen if additional +LAMMPS instances using Python would be created *after* +after calling Py_Finalize(). + +This function can be called to explicitly clear the Python +environment in case it is safe to do so. + +.. versionadded:: TBD + +*See also* + :cpp:func:`lammps_mpi_finalize`, :cpp:func:`lammps_kokkos_finalize` +\endverbatim */ + +void lammps_python_finalize() +{ + Python::finalize(); +} + // ---------------------------------------------------------------------- // Library functions to process commands // ---------------------------------------------------------------------- diff --git a/src/library.h b/src/library.h index e55f906a11..1605267818 100644 --- a/src/library.h +++ b/src/library.h @@ -95,6 +95,7 @@ void lammps_close(void *handle); void lammps_mpi_init(); void lammps_mpi_finalize(); void lammps_kokkos_finalize(); +void lammps_python_finalize(); /* ---------------------------------------------------------------------- * Library functions to process commands diff --git a/src/lmppython.cpp b/src/lmppython.cpp index 44b469eed3..c200b4ba50 100644 --- a/src/lmppython.cpp +++ b/src/lmppython.cpp @@ -126,3 +126,12 @@ bool Python::has_minimum_version(int major, int minor) init(); return impl->has_minimum_version(major, minor); } + +/* ------------------------------------------------------------------ */ + +void Python::finalize() +{ +#if defined(LMP_PYTHON) + PythonImpl::finalize(); +#endif +} diff --git a/src/lmppython.h b/src/lmppython.h index 3c7ae396bc..7ecee915e5 100644 --- a/src/lmppython.h +++ b/src/lmppython.h @@ -47,6 +47,7 @@ class Python : protected Pointers { bool is_enabled() const; void init(); + static void finalize(); private: PythonInterface *impl; diff --git a/src/main.cpp b/src/main.cpp index 608ef4ae5b..32420324ef 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -15,6 +15,8 @@ #include "accelerator_kokkos.h" #include "input.h" +#include "lmppython.h" + #if defined(LAMMPS_EXCEPTIONS) #include "exceptions.h" #endif @@ -79,15 +81,18 @@ int main(int argc, char **argv) delete lammps; } catch (LAMMPSAbortException &ae) { KokkosLMP::finalize(); + Python::finalize(); MPI_Abort(ae.universe, 1); } catch (LAMMPSException &e) { KokkosLMP::finalize(); + Python::finalize(); MPI_Barrier(lammps_comm); MPI_Finalize(); exit(1); } catch (fmt::format_error &fe) { fprintf(stderr, "fmt::format_error: %s\n", fe.what()); KokkosLMP::finalize(); + Python::finalize(); MPI_Abort(MPI_COMM_WORLD, 1); exit(1); } @@ -99,11 +104,13 @@ int main(int argc, char **argv) } catch (fmt::format_error &fe) { fprintf(stderr, "fmt::format_error: %s\n", fe.what()); KokkosLMP::finalize(); + Python::finalize(); MPI_Abort(MPI_COMM_WORLD, 1); exit(1); } #endif KokkosLMP::finalize(); + Python::finalize(); MPI_Barrier(lammps_comm); MPI_Finalize(); } diff --git a/tools/swig/lammps.i b/tools/swig/lammps.i index 2767e4c068..4d0d52f779 100644 --- a/tools/swig/lammps.i +++ b/tools/swig/lammps.i @@ -64,6 +64,7 @@ extern void lammps_close(void *handle); extern void lammps_mpi_init(); extern void lammps_mpi_finalize(); extern void lammps_kokkos_finalize(); +extern void lammps_python_finalize(); extern void lammps_file(void *handle, const char *file); extern char *lammps_command(void *handle, const char *cmd); extern void lammps_commands_list(void *handle, int ncmd, const char **cmds); @@ -199,6 +200,7 @@ extern void lammps_close(void *handle); extern void lammps_mpi_init(); extern void lammps_mpi_finalize(); extern void lammps_kokkos_finalize(); +extern void lammps_python_finalize(); extern void lammps_file(void *handle, const char *file); extern char *lammps_command(void *handle, const char *cmd); extern void lammps_commands_list(void *handle, int ncmd, const char **cmds); From e2d8fd58fa0b787bccd9a451c6d0e2a55f121522 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 4 Sep 2021 14:01:24 -0400 Subject: [PATCH 3/5] apply clang-format --- src/PYTHON/python_impl.cpp | 255 +++++++++++++++++-------------------- 1 file changed, 118 insertions(+), 137 deletions(-) diff --git a/src/PYTHON/python_impl.cpp b/src/PYTHON/python_impl.cpp index 08fba05367..983aafb16b 100644 --- a/src/PYTHON/python_impl.cpp +++ b/src/PYTHON/python_impl.cpp @@ -1,4 +1,3 @@ -// clang-format off /* ---------------------------------------------------------------------- LAMMPS - Large-scale Atomic/Molecular Massively Parallel Simulator https://www.lammps.org/, Sandia National Laboratories @@ -25,8 +24,8 @@ #include "python_utils.h" #include "variable.h" +#include // IWYU pragma: export #include -#include // IWYU pragma: export #ifdef MLIAP_PYTHON #include "mliap_model_python.h" @@ -38,8 +37,7 @@ using namespace LAMMPS_NS; -enum{NONE,INT,DOUBLE,STRING,PTR}; - +enum { NONE, INT, DOUBLE, STRING, PTR }; /* ---------------------------------------------------------------------- */ @@ -53,7 +51,7 @@ PythonImpl::PythonImpl(LAMMPS *lmp) : Pointers(lmp) #if PY_MAJOR_VERSION >= 3 #ifndef Py_LIMITED_API // check for PYTHONUNBUFFERED environment variable - const char * PYTHONUNBUFFERED = getenv("PYTHONUNBUFFERED"); + const char *PYTHONUNBUFFERED = getenv("PYTHONUNBUFFERED"); if (PYTHONUNBUFFERED != nullptr && strcmp(PYTHONUNBUFFERED, "1") == 0) { // Python Global configuration variable @@ -67,7 +65,7 @@ PythonImpl::PythonImpl(LAMMPS *lmp) : Pointers(lmp) // Inform python intialization scheme of the mliappy module. // This -must- happen before python is initialized. int err = PyImport_AppendInittab("mliap_model_python_couple", PyInit_mliap_model_python_couple); - if (err) error->all(FLERR,"Could not register MLIAPPY embedded python module."); + if (err) error->all(FLERR, "Could not register MLIAPPY embedded python module."); #endif Py_Initialize(); @@ -76,15 +74,13 @@ PythonImpl::PythonImpl(LAMMPS *lmp) : Pointers(lmp) // With Python 3.7 this function is now called by Py_Initialize() // Deprecated since version 3.9, will be removed in version 3.11 #if PY_MAJOR_VERSION < 3 || PY_MINOR_VERSION < 7 - if (!PyEval_ThreadsInitialized()) { - PyEval_InitThreads(); - } + if (!PyEval_ThreadsInitialized()) { PyEval_InitThreads(); } #endif PyUtils::GIL lock; PyObject *pModule = PyImport_AddModule("__main__"); - if (!pModule) error->all(FLERR,"Could not initialize embedded Python"); + if (!pModule) error->all(FLERR, "Could not initialize embedded Python"); pyMain = (void *) pModule; } @@ -98,7 +94,7 @@ PythonImpl::~PythonImpl() PyUtils::GIL lock; for (int i = 0; i < nfunc; i++) { - delete [] pfuncs[i].name; + delete[] pfuncs[i].name; deallocate(i); Py_CLEAR(pfuncs[i].pFunc); } @@ -111,39 +107,37 @@ PythonImpl::~PythonImpl() void PythonImpl::command(int narg, char **arg) { - if (narg < 2) error->all(FLERR,"Invalid python command"); + if (narg < 2) error->all(FLERR, "Invalid python command"); // if invoke is only keyword, invoke the previously defined function - if (narg == 2 && strcmp(arg[1],"invoke") == 0) { + if (narg == 2 && strcmp(arg[1], "invoke") == 0) { int ifunc = find(arg[0]); - if (ifunc < 0) error->all(FLERR,"Python invoke of undefined function"); + if (ifunc < 0) error->all(FLERR, "Python invoke of undefined function"); char *str = nullptr; if (pfuncs[ifunc].noutput) { - str = input->variable->pythonstyle(pfuncs[ifunc].ovarname, - pfuncs[ifunc].name); - if (!str) - error->all(FLERR,"Python variable does not match Python function"); + str = input->variable->pythonstyle(pfuncs[ifunc].ovarname, pfuncs[ifunc].name); + if (!str) error->all(FLERR, "Python variable does not match Python function"); } - invoke_function(ifunc,str); + invoke_function(ifunc, str); return; } // if source is only keyword, execute the python code - if (narg == 3 && strcmp(arg[1],"source") == 0) { + if (narg == 3 && strcmp(arg[1], "source") == 0) { int err; - FILE *fp = fopen(arg[2],"r"); + FILE *fp = fopen(arg[2], "r"); if (fp == nullptr) err = execute_string(arg[2]); else err = execute_file(arg[2]); if (fp) fclose(fp); - if (err) error->all(FLERR,"Could not process Python source command"); + if (err) error->all(FLERR, "Could not process Python source command"); return; } @@ -162,53 +156,53 @@ void PythonImpl::command(int narg, char **arg) int iarg = 1; while (iarg < narg) { - if (strcmp(arg[iarg],"input") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Invalid python command"); - ninput = utils::inumeric(FLERR,arg[iarg+1],false,lmp); - if (ninput < 0) error->all(FLERR,"Invalid python command"); + if (strcmp(arg[iarg], "input") == 0) { + if (iarg + 2 > narg) error->all(FLERR, "Invalid python command"); + ninput = utils::inumeric(FLERR, arg[iarg + 1], false, lmp); + if (ninput < 0) error->all(FLERR, "Invalid python command"); iarg += 2; delete[] istr; - istr = new char*[ninput]; - if (iarg+ninput > narg) error->all(FLERR,"Invalid python command"); - for (int i = 0; i < ninput; i++) istr[i] = arg[iarg+i]; + istr = new char *[ninput]; + if (iarg + ninput > narg) error->all(FLERR, "Invalid python command"); + for (int i = 0; i < ninput; i++) istr[i] = arg[iarg + i]; iarg += ninput; - } else if (strcmp(arg[iarg],"return") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Invalid python command"); + } else if (strcmp(arg[iarg], "return") == 0) { + if (iarg + 2 > narg) error->all(FLERR, "Invalid python command"); noutput = 1; - delete[] ostr; - ostr = arg[iarg+1]; + ostr = arg[iarg + 1]; iarg += 2; - } else if (strcmp(arg[iarg],"format") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Invalid python command"); - format = utils::strdup(arg[iarg+1]); + } else if (strcmp(arg[iarg], "format") == 0) { + if (iarg + 2 > narg) error->all(FLERR, "Invalid python command"); + format = utils::strdup(arg[iarg + 1]); iarg += 2; - } else if (strcmp(arg[iarg],"length") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Invalid python command"); - length_longstr = utils::inumeric(FLERR,arg[iarg+1],false,lmp); - if (length_longstr <= 0) error->all(FLERR,"Invalid python command"); + } else if (strcmp(arg[iarg], "length") == 0) { + if (iarg + 2 > narg) error->all(FLERR, "Invalid python command"); + length_longstr = utils::inumeric(FLERR, arg[iarg + 1], false, lmp); + if (length_longstr <= 0) error->all(FLERR, "Invalid python command"); iarg += 2; - } else if (strcmp(arg[iarg],"file") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Invalid python command"); + } else if (strcmp(arg[iarg], "file") == 0) { + if (iarg + 2 > narg) error->all(FLERR, "Invalid python command"); delete[] pyfile; - pyfile = utils::strdup(arg[iarg+1]); + pyfile = utils::strdup(arg[iarg + 1]); iarg += 2; - } else if (strcmp(arg[iarg],"here") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Invalid python command"); - herestr = arg[iarg+1]; + } else if (strcmp(arg[iarg], "here") == 0) { + if (iarg + 2 > narg) error->all(FLERR, "Invalid python command"); + herestr = arg[iarg + 1]; iarg += 2; - } else if (strcmp(arg[iarg],"exists") == 0) { + } else if (strcmp(arg[iarg], "exists") == 0) { existflag = 1; iarg++; - } else error->all(FLERR,"Invalid python command"); + } else + error->all(FLERR, "Invalid python command"); } - if (pyfile && herestr) error->all(FLERR,"Invalid python command"); - if (pyfile && existflag) error->all(FLERR,"Invalid python command"); - if (herestr && existflag) error->all(FLERR,"Invalid python command"); + if (pyfile && herestr) error->all(FLERR, "Invalid python command"); + if (pyfile && existflag) error->all(FLERR, "Invalid python command"); + if (herestr && existflag) error->all(FLERR, "Invalid python command"); // create or overwrite entry in pfuncs vector with name = arg[0] - int ifunc = create_entry(arg[0],ninput,noutput,length_longstr, istr, ostr, format); + int ifunc = create_entry(arg[0], ninput, noutput, length_longstr, istr, ostr, format); PyUtils::GIL lock; @@ -218,18 +212,18 @@ void PythonImpl::command(int narg, char **arg) // exist: do nothing, assume code has already been run if (pyfile) { - FILE *fp = fopen(pyfile,"r"); + FILE *fp = fopen(pyfile, "r"); if (fp == nullptr) { PyUtils::Print_Errors(); - error->all(FLERR,"Could not open Python file"); + error->all(FLERR, "Could not open Python file"); } - int err = PyRun_SimpleFile(fp,pyfile); + int err = PyRun_SimpleFile(fp, pyfile); if (err) { PyUtils::Print_Errors(); - error->all(FLERR,"Could not process Python file"); + error->all(FLERR, "Could not process Python file"); } fclose(fp); @@ -238,34 +232,32 @@ void PythonImpl::command(int narg, char **arg) if (err) { PyUtils::Print_Errors(); - error->all(FLERR,"Could not process Python string"); + error->all(FLERR, "Could not process Python string"); } } // pFunc = function object for requested function PyObject *pModule = (PyObject *) pyMain; - PyObject *pFunc = PyObject_GetAttrString(pModule,pfuncs[ifunc].name); + PyObject *pFunc = PyObject_GetAttrString(pModule, pfuncs[ifunc].name); if (!pFunc) { PyUtils::Print_Errors(); - error->all(FLERR,"Could not find Python function {}", - pfuncs[ifunc].name); + error->all(FLERR, "Could not find Python function {}", pfuncs[ifunc].name); } if (!PyCallable_Check(pFunc)) { PyUtils::Print_Errors(); - error->all(FLERR,"Python function {} is not callable", - pfuncs[ifunc].name); + error->all(FLERR, "Python function {} is not callable", pfuncs[ifunc].name); } pfuncs[ifunc].pFunc = (void *) pFunc; // clean-up input storage - delete [] istr; - delete [] format; - delete [] pyfile; + delete[] istr; + delete[] format; + delete[] pyfile; } /* ------------------------------------------------------------------ */ @@ -283,20 +275,14 @@ void PythonImpl::invoke_function(int ifunc, char *result) int ninput = pfuncs[ifunc].ninput; PyObject *pArgs = PyTuple_New(ninput); - if (!pArgs) { - error->all(FLERR,"Could not create Python function arguments"); - } + if (!pArgs) { error->all(FLERR, "Could not create Python function arguments"); } for (int i = 0; i < ninput; i++) { int itype = pfuncs[ifunc].itype[i]; if (itype == INT) { if (pfuncs[ifunc].ivarflag[i]) { str = input->variable->retrieve(pfuncs[ifunc].svalue[i]); - - if (!str) { - error->all(FLERR,"Could not evaluate Python function input variable"); - } - + if (!str) { error->all(FLERR, "Could not evaluate Python function input variable"); } pValue = PY_INT_FROM_LONG(atoi(str)); } else { pValue = PY_INT_FROM_LONG(pfuncs[ifunc].ivalue[i]); @@ -304,11 +290,7 @@ void PythonImpl::invoke_function(int ifunc, char *result) } else if (itype == DOUBLE) { if (pfuncs[ifunc].ivarflag[i]) { str = input->variable->retrieve(pfuncs[ifunc].svalue[i]); - - if (!str) { - error->all(FLERR,"Could not evaluate Python function input variable"); - } - + if (!str) { error->all(FLERR, "Could not evaluate Python function input variable"); } pValue = PyFloat_FromDouble(atof(str)); } else { pValue = PyFloat_FromDouble(pfuncs[ifunc].dvalue[i]); @@ -316,10 +298,7 @@ void PythonImpl::invoke_function(int ifunc, char *result) } else if (itype == STRING) { if (pfuncs[ifunc].ivarflag[i]) { str = input->variable->retrieve(pfuncs[ifunc].svalue[i]); - if (!str) { - error->all(FLERR,"Could not evaluate Python function input variable"); - } - + if (!str) { error->all(FLERR, "Could not evaluate Python function input variable"); } pValue = PY_STRING_FROM_STRING(str); } else { pValue = PY_STRING_FROM_STRING(pfuncs[ifunc].svalue[i]); @@ -327,20 +306,20 @@ void PythonImpl::invoke_function(int ifunc, char *result) } else if (itype == PTR) { pValue = PY_VOID_POINTER(lmp); } else { - error->all(FLERR,"Unsupported variable type"); + error->all(FLERR, "Unsupported variable type"); } - PyTuple_SetItem(pArgs,i,pValue); + PyTuple_SetItem(pArgs, i, pValue); } // call the Python function // error check with one() since only some procs may fail - pValue = PyObject_CallObject(pFunc,pArgs); + pValue = PyObject_CallObject(pFunc, pArgs); Py_CLEAR(pArgs); if (!pValue) { PyUtils::Print_Errors(); - error->one(FLERR,"Python function evaluation failed"); + error->one(FLERR, "Python function evaluation failed"); } // function returned a value @@ -350,14 +329,15 @@ void PythonImpl::invoke_function(int ifunc, char *result) if (pfuncs[ifunc].noutput) { int otype = pfuncs[ifunc].otype; if (otype == INT) { - sprintf(result,"%ld",PY_INT_AS_LONG(pValue)); + sprintf(result, "%ld", PY_INT_AS_LONG(pValue)); } else if (otype == DOUBLE) { - sprintf(result,"%.15g",PyFloat_AsDouble(pValue)); + sprintf(result, "%.15g", PyFloat_AsDouble(pValue)); } else if (otype == STRING) { const char *pystr = PY_STRING_AS_STRING(pValue); if (pfuncs[ifunc].longstr) - strncpy(pfuncs[ifunc].longstr,pystr,pfuncs[ifunc].length_longstr); - else strncpy(result,pystr,Variable::VALUELENGTH-1); + strncpy(pfuncs[ifunc].longstr, pystr, pfuncs[ifunc].length_longstr); + else + strncpy(result, pystr, Variable::VALUELENGTH - 1); } } Py_CLEAR(pValue); @@ -368,19 +348,18 @@ void PythonImpl::invoke_function(int ifunc, char *result) int PythonImpl::find(const char *name) { for (int i = 0; i < nfunc; i++) - if (strcmp(name,pfuncs[i].name) == 0) return i; + if (strcmp(name, pfuncs[i].name) == 0) return i; return -1; } /* ------------------------------------------------------------------ */ -int PythonImpl::variable_match(const char *name, const char *varname, - int numeric) +int PythonImpl::variable_match(const char *name, const char *varname, int numeric) { int ifunc = find(name); if (ifunc < 0) return -1; if (pfuncs[ifunc].noutput == 0) return -1; - if (strcmp(pfuncs[ifunc].ovarname,varname) != 0) return -1; + if (strcmp(pfuncs[ifunc].ovarname, varname) != 0) return -1; if (numeric && pfuncs[ifunc].otype == STRING) return -1; return ifunc; } @@ -394,8 +373,8 @@ char *PythonImpl::long_string(int ifunc) /* ------------------------------------------------------------------ */ -int PythonImpl::create_entry(char *name, int ninput, int noutput, int length_longstr, - char **istr, char *ostr, char *format) +int PythonImpl::create_entry(char *name, int ninput, int noutput, int length_longstr, char **istr, + char *ostr, char *format) { // ifunc = index to entry by name in pfuncs vector, can be old or new // free old vectors if overwriting old pfunc @@ -405,18 +384,18 @@ int PythonImpl::create_entry(char *name, int ninput, int noutput, int length_lon if (ifunc < 0) { ifunc = nfunc; nfunc++; - pfuncs = (PyFunc *) - memory->srealloc(pfuncs,nfunc*sizeof(struct PyFunc),"python:pfuncs"); + pfuncs = (PyFunc *) memory->srealloc(pfuncs, nfunc * sizeof(struct PyFunc), "python:pfuncs"); pfuncs[ifunc].name = utils::strdup(name); - } else deallocate(ifunc); + } else + deallocate(ifunc); pfuncs[ifunc].ninput = ninput; pfuncs[ifunc].noutput = noutput; - if (!format && ninput+noutput) - error->all(FLERR,"Invalid python command"); - else if (format && ((int) strlen(format) != ninput+noutput)) - error->all(FLERR,"Invalid python command"); + if (!format && ninput + noutput) + error->all(FLERR, "Invalid python command"); + else if (format && ((int) strlen(format) != ninput + noutput)) + error->all(FLERR, "Invalid python command"); // process inputs as values or variables @@ -424,34 +403,34 @@ int PythonImpl::create_entry(char *name, int ninput, int noutput, int length_lon pfuncs[ifunc].ivarflag = new int[ninput]; pfuncs[ifunc].ivalue = new int[ninput]; pfuncs[ifunc].dvalue = new double[ninput]; - pfuncs[ifunc].svalue = new char*[ninput]; + pfuncs[ifunc].svalue = new char *[ninput]; for (int i = 0; i < ninput; i++) { pfuncs[ifunc].svalue[i] = nullptr; char type = format[i]; if (type == 'i') { pfuncs[ifunc].itype[i] = INT; - if (utils::strmatch(istr[i],"^v_")) { + if (utils::strmatch(istr[i], "^v_")) { pfuncs[ifunc].ivarflag[i] = 1; - pfuncs[ifunc].svalue[i] = utils::strdup(istr[i]+2); + pfuncs[ifunc].svalue[i] = utils::strdup(istr[i] + 2); } else { pfuncs[ifunc].ivarflag[i] = 0; - pfuncs[ifunc].ivalue[i] = utils::inumeric(FLERR,istr[i],false,lmp); + pfuncs[ifunc].ivalue[i] = utils::inumeric(FLERR, istr[i], false, lmp); } } else if (type == 'f') { pfuncs[ifunc].itype[i] = DOUBLE; - if (utils::strmatch(istr[i],"^v_")) { + if (utils::strmatch(istr[i], "^v_")) { pfuncs[ifunc].ivarflag[i] = 1; - pfuncs[ifunc].svalue[i] = utils::strdup(istr[i]+2); + pfuncs[ifunc].svalue[i] = utils::strdup(istr[i] + 2); } else { pfuncs[ifunc].ivarflag[i] = 0; - pfuncs[ifunc].dvalue[i] = utils::numeric(FLERR,istr[i],false,lmp); + pfuncs[ifunc].dvalue[i] = utils::numeric(FLERR, istr[i], false, lmp); } } else if (type == 's') { pfuncs[ifunc].itype[i] = STRING; - if (utils::strmatch(istr[i],"^v_")) { + if (utils::strmatch(istr[i], "^v_")) { pfuncs[ifunc].ivarflag[i] = 1; - pfuncs[ifunc].svalue[i] = utils::strdup(istr[i]+2); + pfuncs[ifunc].svalue[i] = utils::strdup(istr[i] + 2); } else { pfuncs[ifunc].ivarflag[i] = 0; pfuncs[ifunc].svalue[i] = utils::strdup(istr[i]); @@ -459,10 +438,10 @@ int PythonImpl::create_entry(char *name, int ninput, int noutput, int length_lon } else if (type == 'p') { pfuncs[ifunc].ivarflag[i] = 0; pfuncs[ifunc].itype[i] = PTR; - if (strcmp(istr[i],"SELF") != 0) - error->all(FLERR,"Invalid python command"); + if (strcmp(istr[i], "SELF") != 0) error->all(FLERR, "Invalid python command"); - } else error->all(FLERR,"Invalid python command"); + } else + error->all(FLERR, "Invalid python command"); } // process output as value or variable @@ -472,22 +451,25 @@ int PythonImpl::create_entry(char *name, int ninput, int noutput, int length_lon if (!noutput) return ifunc; char type = format[ninput]; - if (type == 'i') pfuncs[ifunc].otype = INT; - else if (type == 'f') pfuncs[ifunc].otype = DOUBLE; - else if (type == 's') pfuncs[ifunc].otype = STRING; - else error->all(FLERR,"Invalid python command"); + if (type == 'i') + pfuncs[ifunc].otype = INT; + else if (type == 'f') + pfuncs[ifunc].otype = DOUBLE; + else if (type == 's') + pfuncs[ifunc].otype = STRING; + else + error->all(FLERR, "Invalid python command"); if (length_longstr) { if (pfuncs[ifunc].otype != STRING) - error->all(FLERR,"Python command length keyword " - "cannot be used unless output is a string"); + error->all(FLERR, "Python command length keyword cannot be used unless output is a string"); pfuncs[ifunc].length_longstr = length_longstr; - pfuncs[ifunc].longstr = new char[length_longstr+1]; + pfuncs[ifunc].longstr = new char[length_longstr + 1]; pfuncs[ifunc].longstr[length_longstr] = '\0'; } - if (strstr(ostr,"v_") != ostr) error->all(FLERR,"Invalid python command"); - pfuncs[ifunc].ovarname = utils::strdup(ostr+2); + if (strstr(ostr, "v_") != ostr) error->all(FLERR, "Invalid python command"); + pfuncs[ifunc].ovarname = utils::strdup(ostr + 2); return ifunc; } @@ -504,11 +486,11 @@ int PythonImpl::execute_string(char *cmd) int PythonImpl::execute_file(char *fname) { - FILE *fp = fopen(fname,"r"); + FILE *fp = fopen(fname, "r"); if (fp == nullptr) return -1; PyUtils::GIL lock; - int err = PyRun_SimpleFile(fp,fname); + int err = PyRun_SimpleFile(fp, fname); if (fp) fclose(fp); return err; @@ -518,27 +500,26 @@ int PythonImpl::execute_file(char *fname) void PythonImpl::deallocate(int i) { - delete [] pfuncs[i].itype; - delete [] pfuncs[i].ivarflag; - delete [] pfuncs[i].ivalue; - delete [] pfuncs[i].dvalue; - for (int j = 0; j < pfuncs[i].ninput; j++) - delete [] pfuncs[i].svalue[j]; - delete [] pfuncs[i].svalue; - delete [] pfuncs[i].ovarname; - delete [] pfuncs[i].longstr; + delete[] pfuncs[i].itype; + delete[] pfuncs[i].ivarflag; + delete[] pfuncs[i].ivalue; + delete[] pfuncs[i].dvalue; + for (int j = 0; j < pfuncs[i].ninput; j++) delete[] pfuncs[i].svalue[j]; + delete[] pfuncs[i].svalue; + delete[] pfuncs[i].ovarname; + delete[] pfuncs[i].longstr; } /* ------------------------------------------------------------------ */ bool PythonImpl::has_minimum_version(int major, int minor) { - return (PY_MAJOR_VERSION == major && PY_MINOR_VERSION >= minor) || (PY_MAJOR_VERSION > major); + return (PY_MAJOR_VERSION == major && PY_MINOR_VERSION >= minor) || (PY_MAJOR_VERSION > major); } /* ------------------------------------------------------------------ */ void PythonImpl::finalize() { - if (Py_IsInitialized()) Py_Finalize(); + if (Py_IsInitialized()) Py_Finalize(); } From 805b15f5c46ba5517481d6e5d201a61c80871143 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 4 Sep 2021 14:19:51 -0400 Subject: [PATCH 4/5] apply clang-format --- src/lmppython.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lmppython.cpp b/src/lmppython.cpp index c200b4ba50..b7e65d1f02 100644 --- a/src/lmppython.cpp +++ b/src/lmppython.cpp @@ -1,4 +1,3 @@ -// clang-format off /* ---------------------------------------------------------------------- LAMMPS - Large-scale Atomic/Molecular Massively Parallel Simulator https://www.lammps.org/, Sandia National Laboratories @@ -39,9 +38,7 @@ Python::~Python() /* ---------------------------------------------------------------------- */ -PythonInterface::~PythonInterface() -{ -} +PythonInterface::~PythonInterface() {} /* ---------------------------------------------------------------------- */ @@ -50,12 +47,13 @@ void Python::init() #if defined(LMP_PYTHON) if (!impl) impl = new PythonImpl(lmp); #else - error->all(FLERR,"Python support missing! Compile with PYTHON package installed!"); + error->all(FLERR, "Python support missing! Compile with PYTHON package installed!"); #endif } /* ---------------------------------------------------------------------- */ -bool Python::is_enabled() const { +bool Python::is_enabled() const +{ #if defined(LMP_PYTHON) return true; #else @@ -97,7 +95,7 @@ int Python::variable_match(const char *name, const char *varname, int numeric) /* ------------------------------------------------------------------ */ -char * Python::long_string(int ifunc) +char *Python::long_string(int ifunc) { init(); return impl->long_string(ifunc); From 29505404bc8010c82a86027421b4bed51f719977 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 6 Sep 2021 17:42:18 -0400 Subject: [PATCH 5/5] add unit test for checking the impact of lammps_python_finalize() --- unittest/python/CMakeLists.txt | 18 ++++++++++++------ unittest/python/test_python_package.cpp | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/unittest/python/CMakeLists.txt b/unittest/python/CMakeLists.txt index d76f73aceb..1886d84537 100644 --- a/unittest/python/CMakeLists.txt +++ b/unittest/python/CMakeLists.txt @@ -4,11 +4,6 @@ # availability of the PYTHON package is tested for inside the tester. set(TEST_INPUT_FOLDER ${CMAKE_CURRENT_SOURCE_DIR}) -add_executable(test_python_package test_python_package.cpp) -target_link_libraries(test_python_package PRIVATE lammps GTest::GMock GTest::GTest) -target_compile_definitions(test_python_package PRIVATE -DTEST_INPUT_FOLDER=${TEST_INPUT_FOLDER}) -add_test(NAME PythonPackage COMMAND test_python_package WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) -set_tests_properties(PythonPackage PROPERTIES ENVIRONMENT "LAMMPS_POTENTIALS=${LAMMPS_POTENTIALS_DIR};PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}:${LAMMPS_PYTHON_DIR}:$ENV{PYTHONPATH};PYTHONUNBUFFERED=1") # we must have shared libraries enabled for testing the python module if(NOT BUILD_SHARED_LIBS) @@ -22,9 +17,20 @@ if(CMAKE_VERSION VERSION_LESS 3.12) set(Python_EXECUTABLE ${PYTHON_EXECUTABLE}) endif() else() - find_package(Python3 COMPONENTS Interpreter) + find_package(Python3 COMPONENTS Interpreter Development) endif() +add_executable(test_python_package test_python_package.cpp) +target_link_libraries(test_python_package PRIVATE lammps GTest::GMock GTest::GTest) +target_compile_definitions(test_python_package PRIVATE -DTEST_INPUT_FOLDER=${TEST_INPUT_FOLDER}) +# this requires CMake 3.12. don't care to add backward compatibility for this. +if(Python3_Development_FOUND) + target_compile_definitions(test_python_package PRIVATE -DTEST_HAVE_PYTHON_DEVELOPMENT=1) + target_link_libraries(test_python_package PRIVATE Python::Python) +endif() +add_test(NAME PythonPackage COMMAND test_python_package WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) +set_tests_properties(PythonPackage PROPERTIES ENVIRONMENT "LAMMPS_POTENTIALS=${LAMMPS_POTENTIALS_DIR};PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}:${LAMMPS_PYTHON_DIR}:$ENV{PYTHONPATH};PYTHONUNBUFFERED=1") + if(Python_EXECUTABLE) # prepare to augment the environment so that the LAMMPS python module and the shared library is found. set(PYTHON_TEST_ENVIRONMENT PYTHONPATH=${LAMMPS_PYTHON_DIR}:$ENV{PYTHONPATH}) diff --git a/unittest/python/test_python_package.cpp b/unittest/python/test_python_package.cpp index ac40a2eae4..e04be89df1 100644 --- a/unittest/python/test_python_package.cpp +++ b/unittest/python/test_python_package.cpp @@ -27,6 +27,10 @@ #include "../testing/core.h" #include "../testing/systems/melt.h" +#if defined(TEST_HAVE_PYTHON_DEVELOPMENT) +#include +#endif + // location of '*.py' files required by tests #define STRINGIFY(val) XSTR(val) #define XSTR(val) #val @@ -88,6 +92,23 @@ TEST_F(PythonPackageTest, InvokeFunctionFromFile) ASSERT_THAT(output, HasSubstr("2.25\n")); } +#if defined(TEST_HAVE_PYTHON_DEVELOPMENT) +TEST_F(PythonPackageTest, InvokeInitialized) +{ + // execute python function from file + HIDE_OUTPUT([&] { + command("python printnum file ${input_dir}/func.py"); + }); + ASSERT_TRUE(Py_IsInitialized()); + HIDE_OUTPUT([&] { + command("clear"); + }); + ASSERT_TRUE(Py_IsInitialized()); + lammps_python_finalize(); + ASSERT_FALSE(Py_IsInitialized()); +} +#endif + TEST_F(PythonPackageTest, InvokeFunctionPassInt) { // execute python function, passing integer as argument