Merge pull request #2700 from rbberger/python_calls_refactor

Python package refactor

src/PYTHON/fix_python_invoke.cpp

@@ -21,6 +21,7 @@
 #include "error.h"
 #include "lmppython.h"
 #include "python_compat.h"
+#include "python_utils.h"
 #include "update.h"
 
 #include <cstring>
@@ -51,12 +52,12 @@ FixPythonInvoke::FixPythonInvoke(LAMMPS *lmp, int narg, char **arg) :
   }
 
   // get Python function
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
 
   PyObject *pyMain = PyImport_AddModule("__main__");
 
   if (!pyMain) {
-    PyGILState_Release(gstate);
+    PyUtils::Print_Errors();
     error->all(FLERR,"Could not initialize embedded Python");
   }
 
@@ -64,11 +65,19 @@ FixPythonInvoke::FixPythonInvoke(LAMMPS *lmp, int narg, char **arg) :
   pFunc = PyObject_GetAttrString(pyMain, fname);
 
   if (!pFunc) {
-    PyGILState_Release(gstate);
+    PyUtils::Print_Errors();
     error->all(FLERR,"Could not find Python function");
   }
 
-  PyGILState_Release(gstate);
+  lmpPtr = PY_VOID_POINTER(lmp);
+}
+
+/* ---------------------------------------------------------------------- */
+
+FixPythonInvoke::~FixPythonInvoke()
+{
+  PyUtils::GIL lock;
+  Py_CLEAR(lmpPtr);
 }
 
 /* ---------------------------------------------------------------------- */
@@ -82,18 +91,16 @@ int FixPythonInvoke::setmask()
 
 void FixPythonInvoke::end_of_step()
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
 
-  PyObject *ptr = PY_VOID_POINTER(lmp);
+  PyObject * result = PyObject_CallFunction((PyObject*)pFunc, "O", (PyObject*)lmpPtr);
-  PyObject *arglist = Py_BuildValue("(O)", ptr);
 
-  PyObject *result = PyEval_CallObject((PyObject*)pFunc, arglist);
+  if (!result) {
-  Py_DECREF(arglist);
+    PyUtils::Print_Errors();
-  if (!result && (comm->me == 0)) PyErr_Print();
-
-  PyGILState_Release(gstate);
-  if (!result)
     error->all(FLERR,"Fix python/invoke end_of_step() method failed");
+  }
+
+  Py_CLEAR(result);
 }
 
 /* ---------------------------------------------------------------------- */
@@ -102,16 +109,14 @@ void FixPythonInvoke::post_force(int vflag)
 {
   if (update->ntimestep % nevery != 0) return;
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
 
-  PyObject *ptr = PY_VOID_POINTER(lmp);
+  PyObject * result = PyObject_CallFunction((PyObject*)pFunc, "Oi", (PyObject*)lmpPtr, vflag);
-  PyObject *arglist = Py_BuildValue("(Oi)", ptr, vflag);
 
-  PyObject *result = PyEval_CallObject((PyObject*)pFunc, arglist);
+  if (!result) {
-  Py_DECREF(arglist);
+    PyUtils::Print_Errors();
-  if (!result && (comm->me == 0)) PyErr_Print();
-
-  PyGILState_Release(gstate);
-  if (!result)
     error->all(FLERR,"Fix python/invoke post_force() method failed");
+  }
+
+  Py_CLEAR(result);
 }

src/PYTHON/fix_python_invoke.h

@@ -21,6 +21,7 @@ FixStyle(python,FixPythonInvoke)
 #ifndef LMP_FIX_PYTHON_INVOKE_H
 #define LMP_FIX_PYTHON_INVOKE_H
 
+
 #include "fix.h"
 
 namespace LAMMPS_NS {
@@ -28,12 +29,13 @@ namespace LAMMPS_NS {
 class FixPythonInvoke : public Fix {
  public:
   FixPythonInvoke(class LAMMPS *, int, char **);
-  virtual ~FixPythonInvoke() {}
+  virtual ~FixPythonInvoke();
   int setmask();
   virtual void end_of_step();
   virtual void post_force(int);
 
  private:
+  void * lmpPtr;
   void * pFunc;
   int selected_callback;
 };

src/PYTHON/fix_python_move.cpp

@@ -21,8 +21,9 @@
 #include "error.h"
 #include "lmppython.h"
 #include "python_compat.h"
+#include "python_utils.h"
 
-#include <cstring>
+#include <string>
 #include <Python.h>   // IWYU pragma: export
 
 using namespace LAMMPS_NS;
@@ -40,7 +41,7 @@ FixPythonMove::FixPythonMove(LAMMPS *lmp, int narg, char **arg) :
 
   py_move = nullptr;
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
 
   // add current directory to PYTHONPATH
   PyObject *py_path = PySys_GetObject((char *)"path");
@@ -48,70 +49,50 @@ FixPythonMove::FixPythonMove(LAMMPS *lmp, int narg, char **arg) :
 
 
   // create integrator instance
-  char *full_cls_name = arg[3];
+  std::string full_cls_name = arg[3];
-  char *lastpos = strrchr(full_cls_name, '.');
+  size_t lastpos = full_cls_name.rfind(".");
 
-  if (lastpos == nullptr) {
+  if (lastpos == std::string::npos) {
     error->all(FLERR,"Fix python/integrate requires fully qualified class name");
   }
 
-  size_t module_name_length = strlen(full_cls_name) - strlen(lastpos);
+  std::string module_name = full_cls_name.substr(0, lastpos);
-  size_t cls_name_length = strlen(lastpos)-1;
+  std::string cls_name = full_cls_name.substr(lastpos+1);
 
-  char *module_name = new char[module_name_length+1];
+  PyObject *pModule = PyImport_ImportModule(module_name.c_str());
-  char *cls_name = new char[cls_name_length+1];
-  strncpy(module_name, full_cls_name, module_name_length);
-  module_name[module_name_length] = 0;
-
-  strcpy(cls_name, lastpos+1);
-
-  PyObject *pModule = PyImport_ImportModule(module_name);
   if (!pModule) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Loading python integrator module failure");
   }
 
   // create LAMMPS atom type to potential file type mapping in python class
   // by calling 'lammps_pair_style.map_coeff(name,type)'
 
-  PyObject *py_move_type = PyObject_GetAttrString(pModule, cls_name);
+  PyObject *py_move_type = PyObject_GetAttrString(pModule, cls_name.c_str());
   if (!py_move_type) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not find integrator class in module'");
   }
 
-  delete [] module_name;
-  delete [] cls_name;
-
   PyObject *ptr = PY_VOID_POINTER(lmp);
-  PyObject *arglist = Py_BuildValue("(O)", ptr);
+  PyObject *py_move_obj = PyObject_CallFunction(py_move_type, "O", ptr);
-  PyObject *py_move_obj = PyObject_CallObject(py_move_type, arglist);
+  Py_CLEAR(ptr);
-  Py_DECREF(arglist);
 
   if (!py_move_obj) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not instantiate instance of integrator class'");
   }
 
   // check object interface
   py_move = (void *) py_move_obj;
-
-  PyGILState_Release(gstate);
 }
 
 /* ---------------------------------------------------------------------- */
 
 FixPythonMove::~FixPythonMove()
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  if (py_move) Py_DECREF((PyObject*) py_move);
+  Py_CLEAR(py_move);
-  PyGILState_Release(gstate);
 }
 
 /* ---------------------------------------------------------------------- */
@@ -130,122 +111,82 @@ int FixPythonMove::setmask()
 
 void FixPythonMove::init()
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  PyObject *py_move_obj = (PyObject *) py_move;
+  PyObject * result = PyObject_CallMethod((PyObject *)py_move, "init", nullptr);
-  PyObject *py_init = PyObject_GetAttrString(py_move_obj,(char *)"init");
+
-  if (!py_init) {
+  if (!result) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
+    error->all(FLERR,"Fix python/move init() method failed");
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'init()' method'");
   }
-  PyObject *result = PyEval_CallObject(py_init, nullptr);
+  Py_CLEAR(result);
-  if (!result && (comm->me == 0)) PyErr_Print();
-  PyGILState_Release(gstate);
-  if (!result) error->all(FLERR,"Fix python/move init() method failed");
 }
 
 /* ---------------------------------------------------------------------- */
 
 void FixPythonMove::initial_integrate(int vflag)
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  PyObject *py_move_obj = (PyObject *) py_move;
+  PyObject * result = PyObject_CallMethod((PyObject*)py_move, "initial_integrate", "i", vflag);
-  PyObject *py_initial_integrate = PyObject_GetAttrString(py_move_obj,"initial_integrate");
+
-  if (!py_initial_integrate) {
+  if (!result) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
+    error->all(FLERR,"Fix python/move initial_integrate() method failed");
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'initial_integrate' method'");
   }
-  PyObject *arglist = Py_BuildValue("(i)", vflag);
+  Py_CLEAR(result);
-  PyObject *result = PyEval_CallObject(py_initial_integrate, arglist);
-  Py_DECREF(arglist);
-  if (!result && (comm->me == 0)) PyErr_Print();
-  PyGILState_Release(gstate);
-  if (!result) error->all(FLERR,"Fix python/move initial_integrate() "
-                          "method failed");
 }
 
 /* ---------------------------------------------------------------------- */
 
 void FixPythonMove::final_integrate()
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  PyObject *py_move_obj = (PyObject *) py_move;
+  PyObject * result = PyObject_CallMethod((PyObject*)py_move, "final_integrate", nullptr);
-  PyObject *py_final_integrate = PyObject_GetAttrString(py_move_obj,"final_integrate");
+
-  if (!py_final_integrate) {
+  if (!result) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
+    error->all(FLERR,"Fix python/move final_integrate() method failed");
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'final_integrate' method'");
   }
-  PyObject *result = PyEval_CallObject(py_final_integrate, nullptr);
+  Py_CLEAR(result);
-  if (!result && (comm->me == 0)) PyErr_Print();
-  PyGILState_Release(gstate);
-  if (!result) error->all(FLERR,"Fix python/move final_integrate() method "
-                          "failed");
 }
 
 /* ---------------------------------------------------------------------- */
 
 void FixPythonMove::initial_integrate_respa(int vflag, int ilevel, int iloop)
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  PyObject *py_move_obj = (PyObject *) py_move;
+  PyObject * result = PyObject_CallMethod((PyObject*)py_move, "initial_integrate_respa", "iii", vflag, ilevel, iloop);
-  PyObject *py_initial_integrate_respa = PyObject_GetAttrString(py_move_obj,"initial_integrate_respa");
+
-  if (!py_initial_integrate_respa) {
+  if (!result) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
+    error->all(FLERR,"Fix python/move initial_integrate_respa() method failed");
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'initial_integrate_respa' method'");
   }
-  PyObject *arglist = Py_BuildValue("(iii)", vflag, ilevel, iloop);
+  Py_CLEAR(result);
-  PyObject *result = PyEval_CallObject(py_initial_integrate_respa, arglist);
-  Py_DECREF(arglist);
-  if (!result && (comm->me == 0)) PyErr_Print();
-  PyGILState_Release(gstate);
-  if (!result) error->all(FLERR,"Fix python/move initial_integrate_respa() "
-                          "method failed");
 }
 
 /* ---------------------------------------------------------------------- */
 
 void FixPythonMove::final_integrate_respa(int ilevel, int iloop)
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  PyObject *py_move_obj = (PyObject *) py_move;
+  PyObject * result = PyObject_CallMethod((PyObject*)py_move, "final_integrate_respa", "ii", ilevel, iloop);
-  PyObject *py_final_integrate_respa = PyObject_GetAttrString(py_move_obj,"final_integrate_respa");
+
-  if (!py_final_integrate_respa) {
+  if (!result) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
+    error->all(FLERR,"Fix python/move final_integrate_respa() method failed");
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'final_integrate_respa' method'");
   }
-  PyObject *arglist = Py_BuildValue("(ii)", ilevel, iloop);
+  Py_CLEAR(result);
-  PyObject *result = PyEval_CallObject(py_final_integrate_respa, arglist);
-  Py_DECREF(arglist);
-  if (!result && (comm->me == 0)) PyErr_Print();
-  PyGILState_Release(gstate);
-  if (!result) error->all(FLERR,"Fix python/move final_integrate_respa() "
-                          "method failed");
 }
 
 /* ---------------------------------------------------------------------- */
 
 void FixPythonMove::reset_dt()
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  PyObject *py_move_obj = (PyObject *) py_move;
+  PyObject * result = PyObject_CallMethod((PyObject*)py_move, "reset_dt", nullptr);
-  PyObject *py_reset_dt = PyObject_GetAttrString(py_move_obj,"reset_dt");
+
-  if (!py_reset_dt) {
+  if (!result) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
+    error->all(FLERR,"Fix python/move reset_dt() method failed");
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'reset_dt' method'");
   }
-  PyObject *result = PyEval_CallObject(py_reset_dt, nullptr);
+  Py_CLEAR(result);
-  if (!result && (comm->me == 0)) PyErr_Print();
-  PyGILState_Release(gstate);
-  if (!result) error->all(FLERR,"Fix python/move reset_dt() method failed");
 }

src/PYTHON/pair_python.cpp

@@ -24,9 +24,10 @@
 #include "memory.h"
 #include "neigh_list.h"
 #include "python_compat.h"
+#include "python_utils.h"
 #include "update.h"
 
-#include <cstring>
+#include <string>
 #include <Python.h>  // IWYU pragma: export
 
 using namespace LAMMPS_NS;
@@ -50,7 +51,7 @@ PairPython::PairPython(LAMMPS *lmp) : Pair(lmp) {
 
   // add current directory to PYTHONPATH
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
   PyObject *py_path = PySys_GetObject((char *)"path");
   PyList_Append(py_path, PY_STRING_FROM_STRING("."));
 
@@ -61,14 +62,14 @@ PairPython::PairPython(LAMMPS *lmp) : Pair(lmp) {
   if (potentials_path != nullptr) {
     PyList_Append(py_path, PY_STRING_FROM_STRING(potentials_path));
   }
-  PyGILState_Release(gstate);
 }
 
 /* ---------------------------------------------------------------------- */
 
 PairPython::~PairPython()
 {
-  if (py_potential) Py_DECREF((PyObject*) py_potential);
+  PyUtils::GIL lock;
+  Py_CLEAR(py_potential);
   delete[] skip_types;
 
   if (allocated) {
@@ -103,41 +104,31 @@ void PairPython::compute(int eflag, int vflag)
 
   // prepare access to compute_force and compute_energy functions
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
   PyObject *py_pair_instance = (PyObject *) py_potential;
   PyObject *py_compute_force = PyObject_GetAttrString(py_pair_instance,"compute_force");
   if (!py_compute_force) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not find 'compute_force' method'");
   }
   if (!PyCallable_Check(py_compute_force)) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Python 'compute_force' is not callable");
   }
 
   PyObject *py_compute_energy = PyObject_GetAttrString(py_pair_instance,"compute_energy");
   if (!py_compute_energy) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not find 'compute_energy' method'");
   }
   if (!PyCallable_Check(py_compute_energy)) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Python 'compute_energy' is not callable");
   }
 
   PyObject *py_compute_args = PyTuple_New(3);
   if (!py_compute_args) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not create tuple for 'compute' function arguments");
   }
 
@@ -179,13 +170,11 @@ void PairPython::compute(int eflag, int vflag)
         PyTuple_SetItem(py_compute_args,0,py_rsq);
         py_value = PyObject_CallObject(py_compute_force,py_compute_args);
         if (!py_value) {
-          PyErr_Print();
+          PyUtils::Print_Errors();
-          PyErr_Clear();
-          PyGILState_Release(gstate);
           error->all(FLERR,"Calling 'compute_force' function failed");
         }
         fpair = factor_lj*PyFloat_AsDouble(py_value);
-        Py_DECREF(py_value);
+        Py_CLEAR(py_value);
 
         f[i][0] += delx*fpair;
         f[i][1] += dely*fpair;
@@ -198,8 +187,12 @@ void PairPython::compute(int eflag, int vflag)
 
         if (eflag) {
           py_value = PyObject_CallObject(py_compute_energy,py_compute_args);
+          if (!py_value) {
+            PyUtils::Print_Errors();
+            error->all(FLERR,"Calling 'compute_energy' function failed");
+          }
           evdwl = factor_lj*PyFloat_AsDouble(py_value);
-          Py_DECREF(py_value);
+          Py_CLEAR(py_value);
         } else evdwl = 0.0;
 
         if (evflag) ev_tally(i,j,nlocal,newton_pair,
@@ -207,8 +200,7 @@ void PairPython::compute(int eflag, int vflag)
       }
     }
   }
-  Py_DECREF(py_compute_args);
+  Py_CLEAR(py_compute_args);
-  PyGILState_Release(gstate);
 
   if (vflag_fdotr) virial_fdotr_compute();
 }
@@ -261,112 +253,49 @@ void PairPython::coeff(int narg, char **arg)
     error->all(FLERR,"Incorrect args for pair coefficients");
 
   // check if python potential file exists and source it
-  char * full_cls_name = arg[2];
+  std::string full_cls_name = arg[2];
-  char * lastpos = strrchr(full_cls_name, '.');
+  size_t lastpos = full_cls_name.rfind(".");
 
-  if (lastpos == nullptr) {
+  if (lastpos == std::string::npos) {
     error->all(FLERR,"Python pair style requires fully qualified class name");
   }
 
-  size_t module_name_length = strlen(full_cls_name) - strlen(lastpos);
+  std::string module_name = full_cls_name.substr(0, lastpos);
-  size_t cls_name_length = strlen(lastpos)-1;
+  std::string cls_name = full_cls_name.substr(lastpos+1);
 
-  char * module_name = new char[module_name_length+1];
+  PyUtils::GIL lock;
-  char * cls_name = new char[cls_name_length+1];
-  strncpy(module_name, full_cls_name, module_name_length);
-  module_name[module_name_length] = 0;
 
-  strcpy(cls_name, lastpos+1);
+  PyObject * pModule = PyImport_ImportModule(module_name.c_str());
-
-  PyGILState_STATE gstate = PyGILState_Ensure();
-
-  PyObject * pModule = PyImport_ImportModule(module_name);
   if (!pModule) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Loading python pair style module failure");
   }
 
   // create LAMMPS atom type to potential file type mapping in python class
   // by calling 'lammps_pair_style.map_coeff(name,type)'
 
-  PyObject *py_pair_type = PyObject_GetAttrString(pModule, cls_name);
+  PyObject *py_pair_type = PyObject_GetAttrString(pModule, cls_name.c_str());
   if (!py_pair_type) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not find pair style class in module'");
   }
 
-  delete [] module_name;
-  delete [] cls_name;
-
   PyObject * py_pair_instance = PyObject_CallObject(py_pair_type, nullptr);
   if (!py_pair_instance) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not instantiate instance of pair style class'");
   }
 
   py_potential = (void *) py_pair_instance;
 
-  PyObject *py_check_units = PyObject_GetAttrString(py_pair_instance,"check_units");
+  PyObject *py_value = PyObject_CallMethod(py_pair_instance, "check_units", "s", update->unit_style);
-  if (!py_check_units) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'check_units' method'");
-  }
-  if (!PyCallable_Check(py_check_units)) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Python 'check_units' is not callable");
-  }
-  PyObject *py_units_args = PyTuple_New(1);
-  if (!py_units_args) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not create tuple for 'check_units' function arguments");
-  }
-
-  PyObject *py_name = PY_STRING_FROM_STRING(update->unit_style);
-  PyTuple_SetItem(py_units_args,0,py_name);
-  PyObject *py_value = PyObject_CallObject(py_check_units,py_units_args);
   if (!py_value) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Calling 'check_units' function failed");
   }
-  Py_DECREF(py_units_args);
+  Py_CLEAR(py_value);
 
 
-  PyObject *py_map_coeff = PyObject_GetAttrString(py_pair_instance,"map_coeff");
-  if (!py_map_coeff) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'map_coeff' method'");
-  }
-  if (!PyCallable_Check(py_map_coeff)) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Python 'map_coeff' is not callable");
-  }
-
-  PyObject *py_map_args = PyTuple_New(2);
-  if (!py_map_args) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not create tuple for 'map_coeff' function arguments");
-  }
-
   delete[] skip_types;
   skip_types = new int[ntypes+1];
   skip_types[0] = 1;
@@ -375,25 +304,20 @@ void PairPython::coeff(int narg, char **arg)
       skip_types[i] = 1;
       continue;
     } else skip_types[i] = 0;
-    PyObject *py_type = PY_INT_FROM_LONG(i);
+    const int type = i;
-    py_name = PY_STRING_FROM_STRING(arg[2+i]);
+    const char * name = arg[2+i];
-    PyTuple_SetItem(py_map_args,0,py_name);
+    py_value = PyObject_CallMethod(py_pair_instance, "map_coeff", "si", name, type);
-    PyTuple_SetItem(py_map_args,1,py_type);
-    py_value = PyObject_CallObject(py_map_coeff,py_map_args);
     if (!py_value) {
-      PyErr_Print();
+      PyUtils::Print_Errors();
-      PyErr_Clear();
-      PyGILState_Release(gstate);
       error->all(FLERR,"Calling 'map_coeff' function failed");
     }
+    Py_CLEAR(py_value);
 
     for (int j = i; j <= ntypes ; j++) {
       setflag[i][j] = 1;
       cutsq[i][j] = cut_global*cut_global;
     }
   }
-  Py_DECREF(py_map_args);
-  PyGILState_Release(gstate);
 }
 
 /* ---------------------------------------------------------------------- */
@@ -417,76 +341,52 @@ double PairPython::single(int /* i */, int /* j */, int itype, int jtype,
 
   // prepare access to compute_force and compute_energy functions
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  PyObject *py_pair_instance = (PyObject *) py_potential;
+  PyObject *py_compute_force = (PyObject *) get_member_function("compute_force");
-  PyObject *py_compute_force
+  PyObject *py_compute_energy = (PyObject *) get_member_function("compute_energy");
-    = PyObject_GetAttrString(py_pair_instance,"compute_force");
+  PyObject *py_compute_args = Py_BuildValue("(dii)", rsq, itype, jtype);
-  if (!py_compute_force) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'compute_force' method'");
-  }
-  if (!PyCallable_Check(py_compute_force)) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Python 'compute_force' is not callable");
-  }
 
-  PyObject *py_compute_energy
-    = PyObject_GetAttrString(py_pair_instance,"compute_energy");
-  if (!py_compute_energy) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Could not find 'compute_energy' method'");
-  }
-  if (!PyCallable_Check(py_compute_energy)) {
-    PyErr_Print();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
-    error->all(FLERR,"Python 'compute_energy' is not callable");
-  }
-
-  PyObject *py_rsq, *py_itype, *py_jtype, *py_value;
-  PyObject *py_compute_args = PyTuple_New(3);
   if (!py_compute_args) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not create tuple for 'compute' function arguments");
   }
 
-  py_itype = PY_INT_FROM_LONG(itype);
+  PyObject * py_value = PyObject_CallObject(py_compute_force, py_compute_args);
-  PyTuple_SetItem(py_compute_args,1,py_itype);
-  py_jtype = PY_INT_FROM_LONG(jtype);
-  PyTuple_SetItem(py_compute_args,2,py_jtype);
-  py_rsq = PyFloat_FromDouble(rsq);
-  PyTuple_SetItem(py_compute_args,0,py_rsq);
-
-  py_value = PyObject_CallObject(py_compute_force,py_compute_args);
   if (!py_value) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Calling 'compute_force' function failed");
   }
   fforce = factor_lj*PyFloat_AsDouble(py_value);
-  Py_DECREF(py_value);
+  Py_CLEAR(py_value);
 
-  py_value = PyObject_CallObject(py_compute_energy,py_compute_args);
+  py_value = PyObject_CallObject(py_compute_energy, py_compute_args);
   if (!py_value) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyErr_Clear();
-    PyGILState_Release(gstate);
     error->all(FLERR,"Calling 'compute_energy' function failed");
   }
   double evdwl = factor_lj*PyFloat_AsDouble(py_value);
-  Py_DECREF(py_value);
 
-  Py_DECREF(py_compute_args);
+  Py_CLEAR(py_value);
-  PyGILState_Release(gstate);
+  Py_CLEAR(py_compute_args);
 
   return evdwl;
 }
+
+
+/* ---------------------------------------------------------------------- */
+
+void * PairPython::get_member_function(const char * name)
+{
+  PyUtils::GIL lock;
+  PyObject *py_pair_instance = (PyObject *) py_potential;
+  PyObject * py_mfunc = PyObject_GetAttrString(py_pair_instance, name);
+  if (!py_mfunc) {
+    PyUtils::Print_Errors();
+    error->all(FLERR, fmt::format("Could not find '{}' method'", name));
+  }
+  if (!PyCallable_Check(py_mfunc)) {
+    PyUtils::Print_Errors();
+    error->all(FLERR, fmt::format("Python '{}' is not callable", name));
+  }
+  return py_mfunc;
+}

src/PYTHON/pair_python.h

@@ -50,6 +50,7 @@ class PairPython : public Pair {
   int  * skip_types;
 
   virtual void allocate();
+  void * get_member_function(const char *);
 };
 
 }

src/PYTHON/python_impl.cpp

@@ -21,6 +21,7 @@
 #include "input.h"
 #include "memory.h"
 #include "python_compat.h"
+#include "python_utils.h"
 #include "variable.h"
 
 #include <cstring>
@@ -91,13 +92,12 @@ PythonImpl::PythonImpl(LAMMPS *lmp) : Pointers(lmp)
   }
 #endif
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
 
   PyObject *pModule = PyImport_AddModule("__main__");
   if (!pModule) error->all(FLERR,"Could not initialize embedded Python");
 
   pyMain = (void *) pModule;
-  PyGILState_Release(gstate);
 }
 
 /* ---------------------------------------------------------------------- */
@@ -106,23 +106,19 @@ PythonImpl::~PythonImpl()
 {
   if (pyMain) {
     // clean up
-    PyGILState_STATE gstate = PyGILState_Ensure();
+    PyUtils::GIL lock;
 
     for (int i = 0; i < nfunc; i++) {
       delete [] pfuncs[i].name;
       deallocate(i);
-      PyObject *pFunc = (PyObject *) pfuncs[i].pFunc;
+      Py_CLEAR(pfuncs[i].pFunc);
-      Py_XDECREF(pFunc);
     }
+  }
 
-    // shutdown Python interpreter
+  // shutdown Python interpreter
-
+  if (!external_interpreter) {
-    if (!external_interpreter) {
+    PyGILState_Ensure();
-      Py_Finalize();
+    Py_Finalize();
-    }
-    else {
-      PyGILState_Release(gstate);
-    }
   }
 
   memory->sfree(pfuncs);
@@ -228,7 +224,7 @@ void PythonImpl::command(int narg, char **arg)
 
   int ifunc = create_entry(arg[0]);
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
 
   // send Python code to Python interpreter
   // file: read the file via PyRun_SimpleFile()
@@ -239,14 +235,14 @@ void PythonImpl::command(int narg, char **arg)
     FILE *fp = fopen(pyfile,"r");
 
     if (fp == nullptr) {
-      PyGILState_Release(gstate);
+      PyUtils::Print_Errors();
       error->all(FLERR,"Could not open Python file");
     }
 
     int err = PyRun_SimpleFile(fp,pyfile);
 
     if (err) {
-      PyGILState_Release(gstate);
+      PyUtils::Print_Errors();
       error->all(FLERR,"Could not process Python file");
     }
 
@@ -255,7 +251,7 @@ void PythonImpl::command(int narg, char **arg)
     int err = PyRun_SimpleString(herestr);
 
     if (err) {
-      PyGILState_Release(gstate);
+      PyUtils::Print_Errors();
       error->all(FLERR,"Could not process Python string");
     }
   }
@@ -266,13 +262,13 @@ void PythonImpl::command(int narg, char **arg)
   PyObject *pFunc = PyObject_GetAttrString(pModule,pfuncs[ifunc].name);
 
   if (!pFunc) {
-    PyGILState_Release(gstate);
+    PyUtils::Print_Errors();
     error->all(FLERR,fmt::format("Could not find Python function {}",
                                  pfuncs[ifunc].name));
   }
 
   if (!PyCallable_Check(pFunc)) {
-    PyGILState_Release(gstate);
+    PyUtils::Print_Errors();
     error->all(FLERR,fmt::format("Python function {} is not callable",
                                  pfuncs[ifunc].name));
   }
@@ -284,14 +280,13 @@ void PythonImpl::command(int narg, char **arg)
   delete [] istr;
   delete [] format;
   delete [] pyfile;
-  PyGILState_Release(gstate);
 }
 
 /* ------------------------------------------------------------------ */
 
 void PythonImpl::invoke_function(int ifunc, char *result)
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
   PyObject *pValue;
   char *str;
 
@@ -303,7 +298,6 @@ void PythonImpl::invoke_function(int ifunc, char *result)
   PyObject *pArgs = PyTuple_New(ninput);
 
   if (!pArgs) {
-    PyGILState_Release(gstate);
     error->all(FLERR,"Could not create Python function arguments");
   }
 
@@ -314,7 +308,6 @@ void PythonImpl::invoke_function(int ifunc, char *result)
         str = input->variable->retrieve(pfuncs[ifunc].svalue[i]);
 
         if (!str) {
-          PyGILState_Release(gstate);
           error->all(FLERR,"Could not evaluate Python function input variable");
         }
 
@@ -327,7 +320,6 @@ void PythonImpl::invoke_function(int ifunc, char *result)
         str = input->variable->retrieve(pfuncs[ifunc].svalue[i]);
 
         if (!str) {
-          PyGILState_Release(gstate);
           error->all(FLERR,"Could not evaluate Python function input variable");
         }
 
@@ -339,7 +331,6 @@ void PythonImpl::invoke_function(int ifunc, char *result)
       if (pfuncs[ifunc].ivarflag[i]) {
         str = input->variable->retrieve(pfuncs[ifunc].svalue[i]);
         if (!str) {
-          PyGILState_Release(gstate);
           error->all(FLERR,"Could not evaluate Python function input variable");
         }
 
@@ -350,7 +341,6 @@ void PythonImpl::invoke_function(int ifunc, char *result)
     } else if (itype == PTR) {
       pValue = PY_VOID_POINTER(lmp);
     } else {
-      PyGILState_Release(gstate);
       error->all(FLERR,"Unsupported variable type");
     }
     PyTuple_SetItem(pArgs,i,pValue);
@@ -360,15 +350,13 @@ void PythonImpl::invoke_function(int ifunc, char *result)
   // error check with one() since only some procs may fail
 
   pValue = PyObject_CallObject(pFunc,pArgs);
+  Py_CLEAR(pArgs);
 
   if (!pValue) {
-    PyErr_Print();
+    PyUtils::Print_Errors();
-    PyGILState_Release(gstate);
     error->one(FLERR,"Python function evaluation failed");
   }
 
-  Py_DECREF(pArgs);
-
   // function returned a value
   // assign it to result string stored by python-style variable
   // or if user specified a length, assign it to longstr
@@ -385,10 +373,8 @@ void PythonImpl::invoke_function(int ifunc, char *result)
         strncpy(pfuncs[ifunc].longstr,pystr,pfuncs[ifunc].length_longstr);
       else strncpy(result,pystr,VALUELENGTH-1);
     }
-    Py_DECREF(pValue);
   }
-
+  Py_CLEAR(pValue);
-  PyGILState_Release(gstate);
 }
 
 /* ------------------------------------------------------------------ */
@@ -523,11 +509,8 @@ int PythonImpl::create_entry(char *name)
 
 int PythonImpl::execute_string(char *cmd)
 {
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
-  int err = PyRun_SimpleString(cmd);
+  return PyRun_SimpleString(cmd);
-  PyGILState_Release(gstate);
-
-  return err;
 }
 
 /* ---------------------------------------------------------------------- */
@@ -537,9 +520,8 @@ int PythonImpl::execute_file(char *fname)
   FILE *fp = fopen(fname,"r");
   if (fp == nullptr) return -1;
 
-  PyGILState_STATE gstate = PyGILState_Ensure();
+  PyUtils::GIL lock;
   int err = PyRun_SimpleFile(fp,fname);
-  PyGILState_Release(gstate);
 
   if (fp) fclose(fp);
   return err;

src/PYTHON/python_utils.h

@@ -0,0 +1,42 @@
+/* -*- c++ -*- ----------------------------------------------------------
+   LAMMPS - Large-scale Atomic/Molecular Massively Parallel Simulator
+   http://lammps.sandia.gov, Sandia National Laboratories
+   Steve Plimpton, sjplimp@sandia.gov
+
+   Copyright (2003) Sandia Corporation.  Under the terms of Contract
+   DE-AC04-94AL85000 with Sandia Corporation, the U.S. Government retains
+   certain rights in this software.  This software is distributed under
+   the GNU General Public License.
+
+   See the README file in the top-level LAMMPS directory.
+------------------------------------------------------------------------- */
+
+#ifndef LMP_PYTHON_UTILS_H
+#define LMP_PYTHON_UTILS_H
+
+#include <Python.h>
+
+namespace LAMMPS_NS {
+
+namespace PyUtils {
+
+class GIL {
+  PyGILState_STATE gstate;
+public:
+  GIL() : gstate(PyGILState_Ensure()) {
+  }
+  ~GIL() {
+    PyGILState_Release(gstate);
+  }
+};
+
+static void Print_Errors() {
+  PyErr_Print();
+  PyErr_Clear();
+}
+
+}
+
+}
+
+#endif

unittest/formats/test_atom_styles.cpp

@@ -40,11 +40,7 @@
 #elif defined(__GNUC__)
 #if (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 9))
 #pragma GCC optimize("no-var-tracking-assignments", "O0")
-#else
-#pragma GCC optimize("no-var-tracking-assignments")
 #endif
-#else
-#define _do_nothing
 #endif
 #endif
 

unittest/formats/test_potential_file_reader.cpp

@@ -43,18 +43,20 @@ using utils::split_words;
 // whether to print verbose output (i.e. not capturing LAMMPS screen output).
 bool verbose = false;
 
-const int LAMMPS_NS::PairSW::NPARAMS_PER_LINE;
+#if __cplusplus < 201703L
-const int LAMMPS_NS::PairComb::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairSW::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairComb3::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairComb::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairTersoff::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairComb3::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairTersoffMOD::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairTersoff::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairTersoffMODC::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairTersoffMOD::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairTersoffZBL::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairTersoffMODC::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairGW::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairTersoffZBL::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairGWZBL::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairGW::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairNb3bHarmonic::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairGWZBL::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairVashishta::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairNb3bHarmonic::NPARAMS_PER_LINE;
-const int LAMMPS_NS::PairTersoffTable::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairVashishta::NPARAMS_PER_LINE;
+constexpr int LAMMPS_NS::PairTersoffTable::NPARAMS_PER_LINE;
+#endif
 
 class PotentialFileReaderTest : public LAMMPSTest {
 };