[IMP-dev] Minor patches

[Daniel Russel <>]

First, Harmonic.h needs to include utility.h since it uses square.

Second, I changed the check macro to be IMP_IF_CHECK as discussed. The 
patch also brings in a few other cleanups to the runtime checks.

Index: kernel/include/IMP/unary_functions/Harmonic.h
===================================================================
--- kernel/include/IMP/unary_functions/Harmonic.h	(revision 557)
+++ kernel/include/IMP/unary_functions/Harmonic.h	(working copy)
@@ -8,6 +8,7 @@
 #define __IMP_HARMONIC_H
 
 #include "../UnaryFunction.h"
+#include "../utility.h"
 
 namespace IMP
 {

Index: kernel/include/IMP/internal/ObjectContainer.h
===================================================================
--- kernel/include/IMP/internal/ObjectContainer.h	(revision 557)
+++ kernel/include/IMP/internal/ObjectContainer.h	(working copy)
@@ -8,6 +8,8 @@
 #ifndef __IMP_OBJECT_CONTAINER_H
 #define __IMP_OBJECT_CONTAINER_H
 
+#include "Object.h"
+
 #include <boost/iterator/filter_iterator.hpp>
 
 #include <vector>
@@ -104,15 +106,15 @@
     return Vector::operator[](get_index(i));
   }
   I push_back(O* d) {
+    IMP_CHECK_OBJECT(d);
     ref(d);
-    IMP_BEGIN_CHECK(EXPENSIVE);
-    IMP_CHECK_OBJECT(d);
-    for (typename Vector::const_iterator it= Vector::begin();
-         it != Vector::end(); ++it) {
-      IMP_assert(*it != d, "IMP Containers can only have one copy of "
-                 << " each object");
+    IMP_IF_CHECK(EXPENSIVE) {
+      for (typename Vector::const_iterator it= Vector::begin();
+           it != Vector::end(); ++it) {
+        IMP_assert(*it != d, "IMP Containers can only have one copy of "
+                   << " each object");
+      }
     }
-    IMP_END_CHECK;
     if (free_.empty()) {
       Vector::push_back(d);
       unsigned int idx= Vector::size()-1;
@@ -126,16 +128,16 @@
   }
   template <class It>
   void insert(iterator c, It b, It e) {
-    IMP_BEGIN_CHECK(EXPENSIVE);
-    for (It cc= b; cc != e; ++cc) {
-      IMP_CHECK_OBJECT(*cc);
-      for (typename Vector::const_iterator it= Vector::begin(); 
-           it != Vector::end(); ++it) {
-        IMP_assert(*it != *cc, "IMP Containers can only have one copy of "
-                   << " each object");
+    IMP_IF_CHECK(EXPENSIVE) {
+      for (It cc= b; cc != e; ++cc) {
+        IMP_CHECK_OBJECT(*cc);
+        for (typename Vector::const_iterator it= Vector::begin(); 
+             it != Vector::end(); ++it) {
+          IMP_assert(*it != *cc, "IMP Containers can only have one copy of "
+                     << " each object");
+        }
       }
     }
-    IMP_END_CHECK;
     for (It cc= b; cc != e; ++cc) {
       ref(*cc);
     }
Index: kernel/include/IMP/exception.h
===================================================================
--- kernel/include/IMP/exception.h	(revision 557)
+++ kernel/include/IMP/exception.h	(working copy)
@@ -125,12 +125,10 @@
 IMPDLLEXPORT CheckLevel get_check_level();
 
 
-#define IMP_BEGIN_CHECK(level)\
-  if (level <= ::IMP::get_check_level()) { do {} while(false)
 
-#define IMP_END_CHECK } do {} while(false)
+#define IMP_IF_CHECK(level)\
+  if (level <= ::IMP::get_check_level())
 
-
 namespace internal
 {
 
Index: kernel/src/Model.cpp
===================================================================
--- kernel/src/Model.cpp	(revision 557)
+++ kernel/src/Model.cpp	(working copy)
@@ -101,6 +101,20 @@
 
   IMP_LOG(TERSE, "End Model::evaluate. Final score: " << score << std::endl);
 
+  for (ParticleIterator pit= particles_begin();
+       pit != particles_end(); ++pit) {
+    for (Particle::FloatKeyIterator fkit = (*pit)->float_keys_begin();
+         fkit != (*pit)->float_keys_end(); ++fkit) {
+      Float v= (*pit)->get_value(*fkit);
+      IMP_check(v==v, "NaN found in particle " << (*pit)->get_index()
+                << " attribute " << *fkit,
+                ValueException("NaN found"));
+      Float d= (*pit)->get_derivative(*fkit);
+      IMP_check(d==d, "NaN found in particle derivative " << (*pit)->get_index()
+                << " attribute " << *fkit,
+                ValueException("NaN found"));
+    }
+  }
   ++iteration_;
   return score;
 }
Index: kernel/src/Particle.cpp
===================================================================
--- kernel/src/Particle.cpp	(revision 557)
+++ kernel/src/Particle.cpp	(working copy)
@@ -25,10 +25,14 @@
 
 void Particle::set_model(Model *md, ParticleIndex pi)
 {
+  IMP_check(model_==NULL || md==NULL,
+            "Set_model called for particle already in model",
+            ValueException("Cannot transfer particles directly"));
   model_ = md;
   pi_ = pi;
-  IMP_assert(model_==NULL || model_->get_particle(pi_)== this,
-             "Set_model called with inconsistent data");
+  IMP_check(model_==NULL || model_->get_particle(pi_)== this,
+            "Set_model called with inconsistent data",
+            ValueException("Cannot transfter particles directly"));
 }
 
 void Particle::set_is_active(const bool is_active)
Index: kernel/src/score_states/BondDecoratorListScoreState.cpp
===================================================================
--- kernel/src/score_states/BondDecoratorListScoreState.cpp	(revision 557)
+++ kernel/src/score_states/BondDecoratorListScoreState.cpp	(working copy)
@@ -24,7 +24,7 @@
   bonds_.clear();
   for (unsigned int i=0; i< ps_.size(); ++i) {
     if (!ps_[i]->get_is_active()) continue;
-    BondedDecorator di= BondedDecorator::cast(ps_[i]);
+    BondedDecorator di(ps_[i]);
     ParticleIndex pi= ps_[i]->get_index();
     for (unsigned int j=0; j< di.get_number_of_bonds(); ++j) {
       BondedDecorator dj= di.get_bonded(j);
Index: kernel/src/score_states/BipartiteNonbondedListScoreState.cpp
===================================================================
--- kernel/src/score_states/BipartiteNonbondedListScoreState.cpp	(revision 557)
+++ kernel/src/score_states/BipartiteNonbondedListScoreState.cpp	(working copy)
@@ -17,7 +17,7 @@
 
 //! Turn the default into an actual algorithm and work around missing algorithms
 /** This cannot be shared with AllNBL because that one has grid and this does
-    not.
+   not.
  */
 static BipartiteNonbondedListScoreState::Algorithm
 translate_algorithm(BipartiteNonbondedListScoreState::Algorithm a)
@@ -100,9 +100,9 @@
     mc1_->reset();
     mcr_->reset();
   }
-  IMP_BEGIN_CHECK(EXPENSIVE);
-  check_nbl();
-  IMP_END_CHECK;
+  IMP_IF_CHECK(EXPENSIVE) {
+    check_nbl();
+  }
 }
 
 void BipartiteNonbondedListScoreState::process_sets(const Particles &p0,
Index: kernel/src/score_states/AllNonbondedListScoreState.cpp
===================================================================
--- kernel/src/score_states/AllNonbondedListScoreState.cpp	(revision 557)
+++ kernel/src/score_states/AllNonbondedListScoreState.cpp	(working copy)
@@ -95,9 +95,9 @@
   if (P::update(mc, cost)) {
     mc_->reset();
   }
-  IMP_BEGIN_CHECK(EXPENSIVE);
-  check_nbl();
-  IMP_END_CHECK;
+  IMP_IF_CHECK(EXPENSIVE) {
+    check_nbl();
+  }
 }
 
 
Index: kernel/src/pair_scores/DistancePairScore.cpp
===================================================================
--- kernel/src/pair_scores/DistancePairScore.cpp	(revision 557)
+++ kernel/src/pair_scores/DistancePairScore.cpp	(working copy)
@@ -29,14 +29,13 @@
   Float d2 = 0, delta[3];
   Float score;
 
-  IMP_BEGIN_CHECK(CHEAP);
-  XYZDecorator::cast(a);
-  XYZDecorator::cast(b);
-  IMP_END_CHECK;
+  IMP_IF_CHECK(CHEAP) {
+    XYZDecorator::cast(a);
+    XYZDecorator::cast(b);
+  }
 
   XYZDecorator d0(a);
   XYZDecorator d1(b);
-
   for (int i = 0; i < 3; ++i) {
     delta[i] = d0.get_coordinate(i) - d1.get_coordinate(i);
     d2 += square(delta[i]);
Index: kernel/src/pair_scores/SphereDistancePairScore.cpp
===================================================================
--- kernel/src/pair_scores/SphereDistancePairScore.cpp	(revision 557)
+++ kernel/src/pair_scores/SphereDistancePairScore.cpp	(working copy)
@@ -21,6 +21,12 @@
 Float SphereDistancePairScore::evaluate(Particle *a, Particle *b,
                                         DerivativeAccumulator *da)
 {
+  IMP_check(a->has_attribute(radius_), "Particle " << a->get_index() 
+            << "missing radius in SphereDistancePairScore",
+            ValueException("Missing radius"));
+  IMP_check(b->has_attribute(radius_), "Particle " << b->get_index() 
+            << "missing radius in SphereDistancePairScore",
+            ValueException("Missing radius"));
   Float ra = a->get_value(radius_);
   Float rb = b->get_value(radius_);
   return internal::evaluate_distance_pair_score(a,b, da, f_.get(), 
Index: kernel/src/internal/bbox_nbl_helpers.cpp
===================================================================
--- kernel/src/internal/bbox_nbl_helpers.cpp	(revision 557)
+++ kernel/src/internal/bbox_nbl_helpers.cpp	(working copy)
@@ -9,6 +9,10 @@
 #include "IMP/decorators/XYZDecorator.h"
 #include "IMP/score_states/NonbondedListScoreState.h"
 
+/* compile the CGAL code with NDEBUG since it doesn't have the 
+   same level of control over errors as IMP
+*/
+#define NDEBUG
 #ifdef IMP_USE_CGAL
 #include <CGAL/box_intersection_d.h>
 #include <vector>