Merge remote-tracking branch 'upstream/master'

openmmapi/src/ContextImpl.cpp

@@ -56,12 +56,13 @@ const static char CHECKPOINT_MAGIC_BYTES[] = "OpenMM Binary Checkpoint\n";
 ContextImpl::ContextImpl(Context& owner, const System& system, Integrator& integrator, Platform* platform, const map<string, string>& properties) :
         owner(owner), system(system), integrator(integrator), hasInitializedForces(false), hasSetPositions(false), integratorIsDeleted(false),
         lastForceGroups(-1), platform(platform), platformData(NULL) {
-    if (system.getNumParticles() == 0)
+    int numParticles = system.getNumParticles();
+    if (numParticles == 0)
         throw OpenMMException("Cannot create a Context for a System with no particles");
     
     // Check for errors in virtual sites and massless particles.
     
-    for (int i = 0; i < system.getNumParticles(); i++) {
+    for (int i = 0; i < numParticles; i++) {
         if (system.isVirtualSite(i)) {
             if (system.getParticleMass(i) != 0.0)
                 throw OpenMMException("Virtual site has nonzero mass");
@@ -71,14 +72,23 @@ ContextImpl::ContextImpl(Context& owner, const System& system, Integrator& integ
                     throw OpenMMException("A virtual site cannot depend on another virtual site");
         }
     }
+    set<pair<int, int> > constraintAtoms;
     for (int i = 0; i < system.getNumConstraints(); i++) {
         int particle1, particle2;
         double distance;
         system.getConstraintParameters(i, particle1, particle2, distance);
+        if (particle1 == particle2)
+            throw OpenMMException("A constraint cannot connect a particle to itself");
+        if (particle1 < 0 || particle2 < 0 || particle1 >= numParticles || particle2 >= numParticles)
+            throw OpenMMException("Illegal particle index in constraint");
         double mass1 = system.getParticleMass(particle1);
         double mass2 = system.getParticleMass(particle2);
         if ((mass1 == 0.0 && mass2 != 0.0) || (mass2 == 0.0 && mass1 != 0.0))
             throw OpenMMException("A constraint cannot involve a massless particle");
+        pair<int, int> atoms = make_pair(min(particle1, particle2), max(particle1, particle2));
+        if (constraintAtoms.find(atoms) != constraintAtoms.end())
+            throw OpenMMException("The System has two constraints between the same atoms.  This will produce a singular constraint matrix.");
+        constraintAtoms.insert(atoms);
     }
     
     // Validate the list of properties.
@@ -170,7 +180,7 @@ ContextImpl::ContextImpl(Context& owner, const System& system, Integrator& integ
     for (size_t i = 0; i < forceImpls.size(); ++i)
         forceImpls[i]->initialize(*this);
     integrator.initialize(*this);
-    updateStateDataKernel.getAs<UpdateStateDataKernel>().setVelocities(*this, vector<Vec3>(system.getNumParticles()));
+    updateStateDataKernel.getAs<UpdateStateDataKernel>().setVelocities(*this, vector<Vec3>(numParticles));
 }
 
 ContextImpl::~ContextImpl() {
@@ -259,10 +269,14 @@ void ContextImpl::setPeriodicBoxVectors(const Vec3& a, const Vec3& b, const Vec3
 }
 
 void ContextImpl::applyConstraints(double tol) {
+    if (!hasSetPositions)
+        throw OpenMMException("Particle positions have not been set");
     applyConstraintsKernel.getAs<ApplyConstraintsKernel>().apply(*this, tol);
 }
 
 void ContextImpl::applyVelocityConstraints(double tol) {
+    if (!hasSetPositions)
+        throw OpenMMException("Particle positions have not been set");
     applyConstraintsKernel.getAs<ApplyConstraintsKernel>().applyToVelocities(*this, tol);
 }
 
@@ -460,3 +474,7 @@ void ContextImpl::loadCheckpoint(istream& stream) {
     updateStateDataKernel.getAs<UpdateStateDataKernel>().loadCheckpoint(*this, stream);
     hasSetPositions = true;
 }
+
+void ContextImpl::systemChanged() {
+    integrator.stateChanged(State::Energy);
+}

openmmapi/src/CustomAngleForceImpl.cpp

@@ -108,4 +108,5 @@ map<string, double> CustomAngleForceImpl::getDefaultParameters() {
 
 void CustomAngleForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomAngleForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/CustomBondForceImpl.cpp

@@ -113,4 +113,5 @@ vector<pair<int, int> > CustomBondForceImpl::getBondedParticles() const {
 
 void CustomBondForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomBondForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/CustomCentroidBondForceImpl.cpp

@@ -246,6 +246,7 @@ void CustomCentroidBondForceImpl::addBondsBetweenGroups(int group1, int group2,
 
 void CustomCentroidBondForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomCentroidBondForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }
 
 void CustomCentroidBondForceImpl::computeNormalizedWeights(const CustomCentroidBondForce& force, const System& system, vector<vector<double> >& weights) {

openmmapi/src/CustomCompoundBondForceImpl.cpp

@@ -208,4 +208,5 @@ ExpressionTreeNode CustomCompoundBondForceImpl::replaceFunctions(const Expressio
 
 void CustomCompoundBondForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomCompoundBondForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/CustomExternalForceImpl.cpp

@@ -97,4 +97,5 @@ map<string, double> CustomExternalForceImpl::getDefaultParameters() {
 
 void CustomExternalForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomExternalForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/CustomGBForceImpl.cpp

@@ -126,4 +126,5 @@ map<string, double> CustomGBForceImpl::getDefaultParameters() {
 
 void CustomGBForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomGBForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/CustomHbondForceImpl.cpp

@@ -278,4 +278,5 @@ ExpressionTreeNode CustomHbondForceImpl::replaceFunctions(const ExpressionTreeNo
 
 void CustomHbondForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomHbondForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/CustomIntegrator.cpp

@@ -6,7 +6,7 @@
  * Biological Structures at Stanford, funded under the NIH Roadmap for        *
  * Medical Research, grant U54 GM072970. See https://simtk.org.               *
  *                                                                            *
- * Portions copyright (c) 2011-2014 Stanford University and the Authors.      *
+ * Portions copyright (c) 2011-2016 Stanford University and the Authors.      *
  * Authors: Peter Eastman                                                     *
  * Contributors:                                                              *
  *                                                                            *
@@ -63,6 +63,15 @@ void CustomIntegrator::initialize(ContextImpl& contextRef) {
         if (contextRef.getParameters().find(name) != contextRef.getParameters().end())
             throw OpenMMException("The Integrator defines a variable with the same name as a Context parameter: "+name);
     }
+    set<std::string> globalTargets;
+    globalTargets.insert(globalNames.begin(), globalNames.end());
+    globalTargets.insert("dt");
+    for (map<string, double>::const_iterator iter = contextRef.getParameters().begin(); iter != contextRef.getParameters().end(); ++iter)
+        globalTargets.insert(iter->first);
+    for (int i = 0; i < computations.size(); i++) {
+        if (computations[i].type == ComputeGlobal && globalTargets.find(computations[i].variable) == globalTargets.end())
+            throw OpenMMException("Unknown global variable: "+computations[i].variable);
+    }
     context = &contextRef;
     owner = &contextRef.getOwner();
     kernel = context->getPlatform().createKernel(IntegrateCustomStepKernel::Name(), contextRef);

openmmapi/src/CustomManyParticleForceImpl.cpp

@@ -240,6 +240,7 @@ ExpressionTreeNode CustomManyParticleForceImpl::replaceFunctions(const Expressio
 
 void CustomManyParticleForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomManyParticleForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }
 
 void CustomManyParticleForceImpl::buildFilterArrays(const CustomManyParticleForce& force, int& numTypes, vector<int>& particleTypes, vector<int>& orderIndex, vector<vector<int> >& particleOrder) {

openmmapi/src/CustomNonbondedForceImpl.cpp

@@ -155,6 +155,7 @@ map<string, double> CustomNonbondedForceImpl::getDefaultParameters() {
 
 void CustomNonbondedForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomNonbondedForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }
 
 void CustomNonbondedForceImpl::calcLongRangeCorrection(const CustomNonbondedForce& force, const Context& context, double& coefficient, vector<double>& derivatives) {

openmmapi/src/CustomTorsionForceImpl.cpp

@@ -115,4 +115,5 @@ map<string, double> CustomTorsionForceImpl::getDefaultParameters() {
 
 void CustomTorsionForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcCustomTorsionForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/GBSAOBCForceImpl.cpp

@@ -69,4 +69,5 @@ std::vector<std::string> GBSAOBCForceImpl::getKernelNames() {
 
 void GBSAOBCForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcGBSAOBCForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/GayBerneForceImpl.cpp

@@ -126,4 +126,5 @@ std::vector<std::string> GayBerneForceImpl::getKernelNames() {
 
 void GayBerneForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcGayBerneForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/HarmonicAngleForceImpl.cpp

@@ -63,4 +63,5 @@ std::vector<std::string> HarmonicAngleForceImpl::getKernelNames() {
 
 void HarmonicAngleForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcHarmonicAngleForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/HarmonicBondForceImpl.cpp

@@ -73,4 +73,5 @@ vector<pair<int, int> > HarmonicBondForceImpl::getBondedParticles() const {
 
 void HarmonicBondForceImpl::updateParametersInContext(ContextImpl& context) {
     kernel.getAs<CalcHarmonicBondForceKernel>().copyParametersToContext(context, owner);
+    context.systemChanged();
 }

openmmapi/src/LocalEnergyMinimizer.cpp

@@ -105,83 +105,89 @@ static lbfgsfloatval_t evaluate(void *instance, const lbfgsfloatval_t *x, lbfgsf
 void LocalEnergyMinimizer::minimize(Context& context, double tolerance, int maxIterations) {
     const System& system = context.getSystem();
     int numParticles = system.getNumParticles();
-    lbfgsfloatval_t *x = lbfgs_malloc(numParticles*3);
-    if (x == NULL)
-        throw OpenMMException("LocalEnergyMinimizer: Failed to allocate memory");
     double constraintTol = context.getIntegrator().getConstraintTolerance();
     double workingConstraintTol = std::max(1e-4, constraintTol);
     double k = tolerance/workingConstraintTol;
+    lbfgsfloatval_t *x = lbfgs_malloc(numParticles*3);
+    if (x == NULL)
+        throw OpenMMException("LocalEnergyMinimizer: Failed to allocate memory");
+    try {
 
-    // Initialize the minimizer.
+        // Initialize the minimizer.
 
-    lbfgs_parameter_t param;
-    lbfgs_parameter_init(&param);
-    if (!context.getPlatform().supportsDoublePrecision())
-        param.xtol = 1e-7;
-    param.max_iterations = maxIterations;
-    param.linesearch = LBFGS_LINESEARCH_BACKTRACKING_STRONG_WOLFE;
+        lbfgs_parameter_t param;
+        lbfgs_parameter_init(&param);
+        if (!context.getPlatform().supportsDoublePrecision())
+            param.xtol = 1e-7;
+        param.max_iterations = maxIterations;
+        param.linesearch = LBFGS_LINESEARCH_BACKTRACKING_STRONG_WOLFE;
 
-    // Make sure the initial configuration satisfies all constraints.
+        // Make sure the initial configuration satisfies all constraints.
 
-    context.applyConstraints(workingConstraintTol);
+        context.applyConstraints(workingConstraintTol);
 
-    // Record the initial positions and determine a normalization constant for scaling the tolerance.
+        // Record the initial positions and determine a normalization constant for scaling the tolerance.
 
-    vector<Vec3> initialPos = context.getState(State::Positions).getPositions();
-    double norm = 0.0;
-    for (int i = 0; i < numParticles; i++) {
-        x[3*i] = initialPos[i][0];
-        x[3*i+1] = initialPos[i][1];
-        x[3*i+2] = initialPos[i][2];
-        norm += initialPos[i].dot(initialPos[i]);
-    }
-    norm /= numParticles;
-    norm = (norm < 1 ? 1 : sqrt(norm));
-    param.epsilon = tolerance/norm;
-
-    // Repeatedly minimize, steadily increasing the strength of the springs until all constraints are satisfied.
-
-    double prevMaxError = 1e10;
-    while (true) {
-        // Perform the minimization.
-
-        lbfgsfloatval_t fx;
-        MinimizerData data(context, k);
-        lbfgs(numParticles*3, x, &fx, evaluate, NULL, &data, &param);
-
-        // Check whether all constraints are satisfied.
-
-        vector<Vec3> positions = context.getState(State::Positions).getPositions();
-        int numConstraints = system.getNumConstraints();
-        double maxError = 0.0;
-        for (int i = 0; i < numConstraints; i++) {
-            int particle1, particle2;
-            double distance;
-            system.getConstraintParameters(i, particle1, particle2, distance);
-            Vec3 delta = positions[particle2]-positions[particle1];
-            double r = sqrt(delta.dot(delta));
-            double error = fabs(r-distance);
-            if (error > maxError)
-                maxError = error;
+        vector<Vec3> initialPos = context.getState(State::Positions).getPositions();
+        double norm = 0.0;
+        for (int i = 0; i < numParticles; i++) {
+            x[3*i] = initialPos[i][0];
+            x[3*i+1] = initialPos[i][1];
+            x[3*i+2] = initialPos[i][2];
+            norm += initialPos[i].dot(initialPos[i]);
         }
-        if (maxError <= workingConstraintTol)
-            break; // All constraints are satisfied.
-        context.setPositions(initialPos);
-        if (maxError >= prevMaxError)
-            break; // Further tightening the springs doesn't seem to be helping, so just give up.
-        prevMaxError = maxError;
-        k *= 10;
-        if (maxError > 100*workingConstraintTol) {
-            // We've gotten far enough from a valid state that we might have trouble getting
-            // back, so reset to the original positions.
-            
-            for (int i = 0; i < numParticles; i++) {
-                x[3*i] = initialPos[i][0];
-                x[3*i+1] = initialPos[i][1];
-                x[3*i+2] = initialPos[i][2];
+        norm /= numParticles;
+        norm = (norm < 1 ? 1 : sqrt(norm));
+        param.epsilon = tolerance/norm;
+
+        // Repeatedly minimize, steadily increasing the strength of the springs until all constraints are satisfied.
+
+        double prevMaxError = 1e10;
+        while (true) {
+            // Perform the minimization.
+
+            lbfgsfloatval_t fx;
+            MinimizerData data(context, k);
+            lbfgs(numParticles*3, x, &fx, evaluate, NULL, &data, &param);
+
+            // Check whether all constraints are satisfied.
+
+            vector<Vec3> positions = context.getState(State::Positions).getPositions();
+            int numConstraints = system.getNumConstraints();
+            double maxError = 0.0;
+            for (int i = 0; i < numConstraints; i++) {
+                int particle1, particle2;
+                double distance;
+                system.getConstraintParameters(i, particle1, particle2, distance);
+                Vec3 delta = positions[particle2]-positions[particle1];
+                double r = sqrt(delta.dot(delta));
+                double error = fabs(r-distance);
+                if (error > maxError)
+                    maxError = error;
+            }
+            if (maxError <= workingConstraintTol)
+                break; // All constraints are satisfied.
+            context.setPositions(initialPos);
+            if (maxError >= prevMaxError)
+                break; // Further tightening the springs doesn't seem to be helping, so just give up.
+            prevMaxError = maxError;
+            k *= 10;
+            if (maxError > 100*workingConstraintTol) {
+                // We've gotten far enough from a valid state that we might have trouble getting
+                // back, so reset to the original positions.
+
+                for (int i = 0; i < numParticles; i++) {
+                    x[3*i] = initialPos[i][0];
+                    x[3*i+1] = initialPos[i][1];
+                    x[3*i+2] = initialPos[i][2];
+                }
             }
         }
     }
+    catch (...) {
+        lbfgs_free(x);
+        throw;
+    }
     lbfgs_free(x);
     
     // If necessary, do a final constraint projection to make sure they are satisfied

openmmapi/src/MonteCarloAnisotropicBarostatImpl.cpp

@@ -34,6 +34,7 @@
 #include "openmm/internal/OSRngSeed.h"
 #include "openmm/Context.h"
 #include "openmm/kernels.h"
+#include "openmm/OpenMMException.h"
 #include <cmath>
 #include <vector>
 #include <algorithm>
@@ -51,6 +52,8 @@ MonteCarloAnisotropicBarostatImpl::MonteCarloAnisotropicBarostatImpl(const Monte
 }
 
 void MonteCarloAnisotropicBarostatImpl::initialize(ContextImpl& context) {
+    if (!context.getSystem().usesPeriodicBoundaryConditions())
+        throw OpenMMException("A barostat cannot be used with a non-periodic system");
     kernel = context.getPlatform().createKernel(ApplyMonteCarloBarostatKernel::Name(), context);
     kernel.getAs<ApplyMonteCarloBarostatKernel>().initialize(context.getSystem(), owner);
     Vec3 box[3];

openmmapi/src/MonteCarloBarostatImpl.cpp

@@ -6,7 +6,7 @@
  * Biological Structures at Stanford, funded under the NIH Roadmap for        *
  * Medical Research, grant U54 GM072970. See https://simtk.org.               *
  *                                                                            *
- * Portions copyright (c) 2010 Stanford University and the Authors.           *
+ * Portions copyright (c) 2010-2016 Stanford University and the Authors.      *
  * Authors: Peter Eastman                                                     *
  * Contributors:                                                              *
  *                                                                            *
@@ -34,6 +34,7 @@
 #include "openmm/internal/OSRngSeed.h"
 #include "openmm/Context.h"
 #include "openmm/kernels.h"
+#include "openmm/OpenMMException.h"
 #include <cmath>
 #include <vector>
 #include <algorithm>
@@ -51,6 +52,8 @@ MonteCarloBarostatImpl::MonteCarloBarostatImpl(const MonteCarloBarostat& owner)
 }
 
 void MonteCarloBarostatImpl::initialize(ContextImpl& context) {
+    if (!context.getSystem().usesPeriodicBoundaryConditions())
+        throw OpenMMException("A barostat cannot be used with a non-periodic system");
     kernel = context.getPlatform().createKernel(ApplyMonteCarloBarostatKernel::Name(), context);
     kernel.getAs<ApplyMonteCarloBarostatKernel>().initialize(context.getSystem(), owner);
     Vec3 box[3];

openmmapi/src/MonteCarloMembraneBarostatImpl.cpp

@@ -34,6 +34,7 @@
 #include "openmm/internal/OSRngSeed.h"
 #include "openmm/Context.h"
 #include "openmm/kernels.h"
+#include "openmm/OpenMMException.h"
 #include <cmath>
 #include <vector>
 #include <algorithm>
@@ -51,6 +52,8 @@ MonteCarloMembraneBarostatImpl::MonteCarloMembraneBarostatImpl(const MonteCarloM
 }
 
 void MonteCarloMembraneBarostatImpl::initialize(ContextImpl& context) {
+    if (!context.getSystem().usesPeriodicBoundaryConditions())
+        throw OpenMMException("A barostat cannot be used with a non-periodic system");
     kernel = context.getPlatform().createKernel(ApplyMonteCarloBarostatKernel::Name(), context);
     kernel.getAs<ApplyMonteCarloBarostatKernel>().initialize(context.getSystem(), owner);
     Vec3 box[3];