Code cleanup (#3014)

devtools/run-ctest.py

@@ -12,7 +12,6 @@ import os.path
 import shutil
 import time
 from glob import glob
-from os.path import join, exists
 from subprocess import call
 from argparse import ArgumentParser
 from datetime import datetime, timedelta

examples/benchmark.py

@@ -2,7 +2,6 @@ from __future__ import print_function
 import openmm.app as app
 import openmm as mm
 import openmm.unit as unit
-import sys
 from datetime import datetime
 import os
 from argparse import ArgumentParser
@@ -68,8 +67,6 @@ def runOneTest(testName, options):
         else:
             ff = app.ForceField('amoeba2009.xml', 'amoeba2009_gk.xml')
             pdb = app.PDBFile('5dfr_minimized.pdb')
-            cutoff = 2.0*unit.nanometers
-            vdwCutoff = 1.2*unit.nanometers
             system = ff.createSystem(pdb.topology, nonbondedMethod=app.NoCutoff, constraints=constraints, mutualInducedTargetEpsilon=epsilon, polarization=options.polarization)
         for f in system.getForces():
             if isinstance(f, mm.AmoebaMultipoleForce) or isinstance(f, mm.AmoebaVdwForce) or isinstance(f, mm.AmoebaGeneralizedKirkwoodForce) or isinstance(f, mm.AmoebaWcaDispersionForce):
@@ -86,7 +83,6 @@ def runOneTest(testName, options):
         fileName = names[testName]
         prmtop = app.AmberPrmtopFile(os.path.join(dirname, f'PME/Topologies/{fileName}.prmtop'))
         inpcrd = app.AmberInpcrdFile(os.path.join(dirname, f'PME/Coordinates/{fileName}.inpcrd'))
-        topology = prmtop.topology
         positions = inpcrd.positions
         dt = 0.004*unit.picoseconds
         method = app.PME

examples/simulateCharmm.py

 from openmm.app import *
 from openmm import *
 from openmm.unit import *
-from sys import stdout, exit, stderr
+from sys import stdout
 
 # Read the PSF
 psf = CharmmPsfFile('ala_ala_ala.psf')

platforms/common/src/ComputeParameterSet.cpp

@@ -40,25 +40,25 @@ ComputeParameterSet::ComputeParameterSet(ComputeContext& context, int numParamet
     string elementType = (useDoublePrecision ? "double" : "float");
     if (!arrayPerParameter) {
         while (params > 2) {
-            std::stringstream name;
-            name << "param" << (++bufferCount);
+            std::stringstream arrayName;
+            arrayName << "param" << (++bufferCount);
             arrays.push_back(context.createArray());
-            arrays.back()->initialize(context, numObjects, elementSize*4, name.str());
+            arrays.back()->initialize(context, numObjects, elementSize*4, arrayName.str());
             params -= 4;
         }
         if (params > 1) {
-            std::stringstream name;
-            name << "param" << (++bufferCount);
+            std::stringstream arrayName;
+            arrayName << "param" << (++bufferCount);
             arrays.push_back(context.createArray());
-            arrays.back()->initialize(context, numObjects, elementSize*2, name.str());
+            arrays.back()->initialize(context, numObjects, elementSize*2, arrayName.str());
             params -= 2;
         }
     }
     while (params > 0) {
-        std::stringstream name;
-        name << "param" << (++bufferCount);
+        std::stringstream arrayName;
+        arrayName << "param" << (++bufferCount);
             arrays.push_back(context.createArray());
-        arrays.back()->initialize(context, numObjects, elementSize, name.str());
+        arrays.back()->initialize(context, numObjects, elementSize, arrayName.str());
         params--;
     }
     for (ArrayInterface* array : arrays)

platforms/cpu/src/CpuBondForce.cpp

@@ -159,8 +159,8 @@ void CpuBondForce::assignBond(int bond, int thread, vector<int>& atomThread, vec
         if (atom != -1)
             throw OpenMMException("CpuBondForce: Internal error: atoms assigned to threads incorrectly");
         atom = thread;
-        for (int bond : atomBonds[atom])
-            candidateBonds.push_back(bond);
+        for (int bondIndex : atomBonds[atom])
+            candidateBonds.push_back(bondIndex);
     }
 }
 

platforms/reference/include/ReferenceBondIxn.h

@@ -51,7 +51,7 @@ class OPENMM_EXPORT ReferenceBondIxn {
       
          --------------------------------------------------------------------------------------- */
 
-       ~ReferenceBondIxn();
+       virtual ~ReferenceBondIxn();
 
       /**---------------------------------------------------------------------------------------
       

plugins/amoeba/platforms/reference/src/SimTKReference/AmoebaReferenceMultipoleForce.cpp

@@ -4894,7 +4894,7 @@ AmoebaReferencePmeMultipoleForce::~AmoebaReferencePmeMultipoleForce()
         fftpack_destroy(_fftplan);
     }
     if (_pmeGrid) {
-        delete _pmeGrid;
+        delete[] _pmeGrid;
     }
 };
 
@@ -4980,7 +4980,7 @@ void AmoebaReferencePmeMultipoleForce::resizePmeArrays()
     _totalGridSize = _pmeGridDimensions[0]*_pmeGridDimensions[1]*_pmeGridDimensions[2];
     if (_pmeGridSize < _totalGridSize) {
         if (_pmeGrid) {
-            delete _pmeGrid;
+            delete[] _pmeGrid;
         }
         _pmeGrid      = new t_complex[_totalGridSize];
         _pmeGridSize  = _totalGridSize;

plugins/amoeba/wrappers/generateAmoebaWrappers.py

 from __future__ import print_function
 import sys, os
-import time
-import getopt
 import re
 import xml.etree.ElementTree as etree
 
@@ -97,7 +95,7 @@ class WrapperGenerator:
             self.findBaseNodes(node, orderedClassNodes)
         return orderedClassNodes
 
-    def findBaseNodes(self, node, excludedClassNodes=[]):
+    def findBaseNodes(self, node, excludedClassNodes):
         if node in excludedClassNodes:
             return
         if node.attrib['prot'] == 'private':
@@ -114,7 +112,6 @@ class WrapperGenerator:
 
     def getClassMethods(self, classNode):
         className = getText("compoundname", classNode)
-        shortClassName = stripOpenMMPrefix(className)
         methodList = []
         for section in findNodes(classNode, "sectiondef", kind="public-static-func")+findNodes(classNode, "sectiondef", kind="public-func"):
             for memberNode in findNodes(section, "memberdef", kind="function", prot="public"):
@@ -202,7 +199,6 @@ class CHeaderGenerator(WrapperGenerator):
             for node in findNodes(section, "memberdef", kind="enum", prot="public"):
                 enumNodes.append(node)
         className = getText("compoundname", classNode)
-        shortClassName = stripOpenMMPrefix(className)
         typeName = convertOpenMMPrefix(className)
         for enumNode in enumNodes:
             enumName = getText("name", enumNode)
@@ -1023,7 +1019,6 @@ class FortranSourceGenerator(WrapperGenerator):
     
     def writeOneConstructor(self, classNode, methodNode, functionName, wrapperFunctionName):
         className = getText("compoundname", classNode)
-        shortClassName = stripOpenMMPrefix(className)
         typeName = convertOpenMMPrefix(className)
         self.out.write("OPENMM_EXPORT_AMOEBA void %s(%s*& result" % (wrapperFunctionName, typeName))
         self.writeArguments(methodNode, True)
@@ -1049,8 +1044,6 @@ class FortranSourceGenerator(WrapperGenerator):
         returnType = self.getType(methodType)
         hasReturnValue = (returnType in ('int', 'bool', 'double'))
         hasReturnArg = not (hasReturnValue or returnType == 'void')
-        if methodType in self.classesByShortName:
-            methodType = self.classesByShortName[methodType]
         self.out.write("OPENMM_EXPORT_AMOEBA ")
         if hasReturnValue:
             self.out.write(returnType)
@@ -1069,7 +1062,7 @@ class FortranSourceGenerator(WrapperGenerator):
                 returnArg = 'char* result'
             else:
                 returnArg = "%s& result" % returnType
-        numArgs = self.writeArguments(methodNode, isInstanceMethod, returnArg)
+        self.writeArguments(methodNode, isInstanceMethod, returnArg)
         if hasReturnArg and returnType == 'const char*':
             self.out.write(", int result_length")
         self.out.write(") {\n")

serialization/src/CustomAngleForceProxy.cpp

@@ -64,12 +64,12 @@ void CustomAngleForceProxy::serialize(const void* object, SerializationNode& nod
         int p1, p2, p3;
         vector<double> params;
         force.getAngleParameters(i, p1, p2, p3, params);
-        SerializationNode& node = angles.createChildNode("Angle").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3);
+        SerializationNode& angle = angles.createChildNode("Angle").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3);
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            angle.setDoubleProperty(key.str(), params[j]);
         }
     }
 }

serialization/src/CustomBondForceProxy.cpp

@@ -64,12 +64,12 @@ void CustomBondForceProxy::serialize(const void* object, SerializationNode& node
         int p1, p2;
         vector<double> params;
         force.getBondParameters(i, p1, p2, params);
-        SerializationNode& node = bonds.createChildNode("Bond").setIntProperty("p1", p1).setIntProperty("p2", p2);
+        SerializationNode& bond = bonds.createChildNode("Bond").setIntProperty("p1", p1).setIntProperty("p2", p2);
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            bond.setDoubleProperty(key.str(), params[j]);
         }
     }
 }

serialization/src/CustomCVForceProxy.cpp

@@ -56,8 +56,8 @@ void CustomCVForceProxy::serialize(const void* object, SerializationNode& node)
     }
     SerializationNode& cvs = node.createChildNode("CollectiveVariables");
     for (int i = 0; i < force.getNumCollectiveVariables(); i++) {
-        SerializationNode& node = cvs.createChildNode("CollectiveVariable").setStringProperty("name", force.getCollectiveVariableName(i));
-        node.createChildNode("Force", &force.getCollectiveVariable(i));
+        SerializationNode& cv = cvs.createChildNode("CollectiveVariable").setStringProperty("name", force.getCollectiveVariableName(i));
+        cv.createChildNode("Force", &force.getCollectiveVariable(i));
     }
     SerializationNode& functions = node.createChildNode("Functions");
     for (int i = 0; i < force.getNumTabulatedFunctions(); i++)

serialization/src/CustomCentroidBondForceProxy.cpp

@@ -78,18 +78,18 @@ void CustomCentroidBondForceProxy::serialize(const void* object, SerializationNo
         vector<int> groups;
         vector<double> params;
         force.getBondParameters(i, groups, params);
-        SerializationNode& node = bonds.createChildNode("Bond");
+        SerializationNode& bond = bonds.createChildNode("Bond");
         for (int j = 0; j < (int) groups.size(); j++) {
             stringstream key;
             key << "g";
             key << j+1;
-            node.setIntProperty(key.str(), groups[j]);
+            bond.setIntProperty(key.str(), groups[j]);
         }
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            bond.setDoubleProperty(key.str(), params[j]);
         }
     }
     SerializationNode& functions = node.createChildNode("Functions");

serialization/src/CustomCompoundBondForceProxy.cpp

@@ -65,18 +65,18 @@ void CustomCompoundBondForceProxy::serialize(const void* object, SerializationNo
         vector<int> particles;
         vector<double> params;
         force.getBondParameters(i, particles, params);
-        SerializationNode& node = bonds.createChildNode("Bond");
+        SerializationNode& bond = bonds.createChildNode("Bond");
         for (int j = 0; j < (int) particles.size(); j++) {
             stringstream key;
             key << "p";
             key << j+1;
-            node.setIntProperty(key.str(), particles[j]);
+            bond.setIntProperty(key.str(), particles[j]);
         }
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            bond.setDoubleProperty(key.str(), params[j]);
         }
     }
     SerializationNode& functions = node.createChildNode("Functions");

serialization/src/CustomExternalForceProxy.cpp

@@ -56,15 +56,15 @@ void CustomExternalForceProxy::serialize(const void* object, SerializationNode&
     }
     SerializationNode& particles = node.createChildNode("Particles");
     for (int i = 0; i < force.getNumParticles(); i++) {
-        int particle;
+        int particleIndex;
         vector<double> params;
-        force.getParticleParameters(i, particle, params);
-        SerializationNode& node = particles.createChildNode("Particle").setIntProperty("index", particle);
+        force.getParticleParameters(i, particleIndex, params);
+        SerializationNode& particle = particles.createChildNode("Particle").setIntProperty("index", particleIndex);
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            particle.setDoubleProperty(key.str(), params[j]);
         }
     }
 }

serialization/src/CustomGBForceProxy.cpp

@@ -77,12 +77,12 @@ void CustomGBForceProxy::serialize(const void* object, SerializationNode& node)
     for (int i = 0; i < force.getNumParticles(); i++) {
         vector<double> params;
         force.getParticleParameters(i, params);
-        SerializationNode& node = particles.createChildNode("Particle");
+        SerializationNode& particle = particles.createChildNode("Particle");
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            particle.setDoubleProperty(key.str(), params[j]);
         }
     }
     SerializationNode& exclusions = node.createChildNode("Exclusions");

serialization/src/CustomHbondForceProxy.cpp

@@ -65,12 +65,12 @@ void CustomHbondForceProxy::serialize(const void* object, SerializationNode& nod
         int p1, p2, p3;
         vector<double> params;
         force.getDonorParameters(i, p1, p2, p3, params);
-        SerializationNode& node = donors.createChildNode("Donor").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3);
+        SerializationNode& donor = donors.createChildNode("Donor").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3);
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            donor.setDoubleProperty(key.str(), params[j]);
         }
     }
     SerializationNode& acceptors = node.createChildNode("Acceptors");
@@ -78,12 +78,12 @@ void CustomHbondForceProxy::serialize(const void* object, SerializationNode& nod
         int p1, p2, p3;
         vector<double> params;
         force.getAcceptorParameters(i, p1, p2, p3, params);
-        SerializationNode& node = acceptors.createChildNode("Acceptor").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3);
+        SerializationNode& acceptor = acceptors.createChildNode("Acceptor").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3);
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            acceptor.setDoubleProperty(key.str(), params[j]);
         }
     }
     SerializationNode& exclusions = node.createChildNode("Exclusions");

serialization/src/CustomManyParticleForceProxy.cpp

@@ -63,13 +63,13 @@ void CustomManyParticleForceProxy::serialize(const void* object, SerializationNo
         vector<double> params;
         int type;
         force.getParticleParameters(i, params, type);
-        SerializationNode& node = particles.createChildNode("Particle");
-        node.setIntProperty("type", type);
+        SerializationNode& particle = particles.createChildNode("Particle");
+        particle.setIntProperty("type", type);
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            particle.setDoubleProperty(key.str(), params[j]);
         }
     }
     SerializationNode& exclusions = node.createChildNode("Exclusions");

serialization/src/CustomNonbondedForceProxy.cpp

@@ -67,12 +67,12 @@ void CustomNonbondedForceProxy::serialize(const void* object, SerializationNode&
     for (int i = 0; i < force.getNumParticles(); i++) {
         vector<double> params;
         force.getParticleParameters(i, params);
-        SerializationNode& node = particles.createChildNode("Particle");
+        SerializationNode& particle = particles.createChildNode("Particle");
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            particle.setDoubleProperty(key.str(), params[j]);
         }
     }
     SerializationNode& exclusions = node.createChildNode("Exclusions");

serialization/src/CustomTorsionForceProxy.cpp

@@ -64,12 +64,12 @@ void CustomTorsionForceProxy::serialize(const void* object, SerializationNode& n
         int p1, p2, p3, p4;
         vector<double> params;
         force.getTorsionParameters(i, p1, p2, p3, p4, params);
-        SerializationNode& node = torsions.createChildNode("Torsion").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3).setIntProperty("p4", p4);
+        SerializationNode& torsion = torsions.createChildNode("Torsion").setIntProperty("p1", p1).setIntProperty("p2", p2).setIntProperty("p3", p3).setIntProperty("p4", p4);
         for (int j = 0; j < (int) params.size(); j++) {
             stringstream key;
             key << "param";
             key << j+1;
-            node.setDoubleProperty(key.str(), params[j]);
+            torsion.setDoubleProperty(key.str(), params[j]);
         }
     }
 }

wrappers/generateWrappers.py

 from __future__ import print_function
 import sys, os
-import time
-import getopt
 import re
 import xml.etree.ElementTree as etree
 
@@ -110,7 +108,7 @@ class WrapperGenerator:
             self.findBaseNodes(node, orderedClassNodes)
         return orderedClassNodes
 
-    def findBaseNodes(self, node, excludedClassNodes=[]):
+    def findBaseNodes(self, node, excludedClassNodes):
         if node in excludedClassNodes:
             return
         if node.attrib['prot'] == 'private':
@@ -127,7 +125,6 @@ class WrapperGenerator:
 
     def getClassMethods(self, classNode):
         className = getText("compoundname", classNode)
-        shortClassName = stripOpenMMPrefix(className)
         methodList = []
         for section in findNodes(classNode, "sectiondef", kind="public-static-func")+findNodes(classNode, "sectiondef", kind="public-func"):
             for memberNode in findNodes(section, "memberdef", kind="function", prot="public"):
@@ -211,7 +208,6 @@ class CHeaderGenerator(WrapperGenerator):
             for node in findNodes(section, "memberdef", kind="enum", prot="public"):
                 enumNodes.append(node)
         className = getText("compoundname", classNode)
-        shortClassName = stripOpenMMPrefix(className)
         typeName = convertOpenMMPrefix(className)
         for enumNode in enumNodes:
             enumName = getText("name", enumNode)
@@ -1590,7 +1586,6 @@ class FortranSourceGenerator(WrapperGenerator):
     
     def writeOneConstructor(self, classNode, methodNode, functionName, wrapperFunctionName):
         className = getText("compoundname", classNode)
-        shortClassName = stripOpenMMPrefix(className)
         typeName = convertOpenMMPrefix(className)
         self.out.write("OPENMM_EXPORT void %s(%s*& result" % (wrapperFunctionName, typeName))
         self.writeArguments(methodNode, True)
@@ -1616,8 +1611,6 @@ class FortranSourceGenerator(WrapperGenerator):
         returnType = self.getType(methodType)
         hasReturnValue = (returnType in ('int', 'bool', 'double'))
         hasReturnArg = not (hasReturnValue or returnType == 'void')
-        if methodType in self.classesByShortName:
-            methodType = self.classesByShortName[methodType]
         self.out.write("OPENMM_EXPORT ")
         if hasReturnValue:
             self.out.write(returnType)
@@ -1636,7 +1629,7 @@ class FortranSourceGenerator(WrapperGenerator):
                 returnArg = 'char* result'
             else:
                 returnArg = "%s& result" % returnType
-        numArgs = self.writeArguments(methodNode, isInstanceMethod, returnArg)
+        self.writeArguments(methodNode, isInstanceMethod, returnArg)
         if hasReturnArg and returnType == 'const char*':
             self.out.write(", int result_length")
         self.out.write(") {\n")