May 2008 - IMP-dev - salilab.org

Test code for cover bonds
by Daniel Russel 22 May '08

22 May '08

Here is some test code for the cover bonds score state. I forgot to include it before. Index: kernel/test/states/test_cover_bonds.py =================================================================== --- kernel/test/states/test_cover_bonds.py (revision 0) +++ kernel/test/states/test_cover_bonds.py (revision 0) @@ -0,0 +1,71 @@ +import unittest +import IMP +import IMP.test + +rk = IMP.FloatKey("radius") + +class TestBondCover(IMP.test.TestCase): + """Tests for BondCover""" + + def _set_up_stuff(self, n): + # return [model, particles, bonds] + m= IMP.Model() + ps= IMP.Particles() + for i in range(0,n): + p= IMP.Particle() + m.add_particle(p) + d= IMP.XYZDecorator.create(p) + d.randomize_in_box(IMP.Vector3D(0,0,0), + IMP.Vector3D(10,10,10)) + ps.append(p) + bds= [] + for i in range(1,n): + ba= IMP.BondedDecorator.create(ps[i-1]) + bb= IMP.BondedDecorator.create(ps[i]) + bds.append(IMP.custom_bond(ba, bb, 10, 1)) + bl= IMP.BondDecoratorListScoreState(ps) + ss= IMP.CoverBondsScoreState(bl, rk) + m.add_score_state(bl) + m.add_score_state(ss) + return [m, ps, bds] + + + + + def test_set_position(self): + """Make sure that bond cover coordinates are correct""" + IMP.set_log_level(IMP.VERBOSE) + [m,ps, bds]= self._set_up_stuff(10) + m.evaluate(False) + for b in bds: + b.get_particle().show() + ba= b.get_bonded(0) + bb= b.get_bonded(1) + da= IMP.XYZDecorator.cast(ba.get_particle()) + db= IMP.XYZDecorator.cast(bb.get_particle()) + dc= IMP.XYZDecorator.cast(b.get_particle()) + center= [(da.get_x()+ db.get_x()) /2.0, + (da.get_y()+ db.get_y()) /2.0, + (da.get_z()+ db.get_z()) /2.0] + r= ((center[0]- da.get_x())**2 + +(center[1]- da.get_y())**2 + +(center[2]- da.get_z())**2)**.5 + print r + print center + da.show() + db.show() + dc.show() + self.assertInTolerance(r, b.get_particle().get_value(rk), r*.1, + "Radius is not close enough") + self.assertInTolerance(center[0], dc.get_x(), .1, + "X is not close enough") + self.assertInTolerance(center[1], dc.get_y(), .1, + "Y is not close enough") + self.assertInTolerance(center[2], dc.get_z(), .1, + "Z is not close enough") + + + + +if __name__ == '__main__': + unittest.main()

1 0

patches
by Daniel Russel 21 May '08

21 May '08

The first just fixes some minor errors in the LIST macros. The second cleans up on the MoverBase, making it use the LIST macros. In addition, the order of the arguments on the movers has been changed to make particles last as in the restraints. Index: kernel/include/IMP/macros.h =================================================================== --- kernel/include/IMP/macros.h (revision 569) +++ kernel/include/IMP/macros.h (working copy) @@ -240,7 +240,7 @@ /** \short Add several objects. \param[in] obj a vector of pointers */ \ - void add_##lcname##s(const std::vector<Ucname*>& obj); \ + void add_##lcname##s(const std::vector<Data>& obj); \ /** \short Clear the contents of the container */ \ void clear_##lcname##s(); \ /** \short return the number of objects*/ \ @@ -277,7 +277,7 @@ IndexType index= lcname##_vector_.push_back(obj); \ Init_obj; \ Onchanged; \ - if (false) std::cout << index; \ + if (false) {index=index; obj=obj;}; \ return index; \ } \ void Class::add_##lcname##s(const std::vector<Data> &objs) { \ @@ -288,7 +288,7 @@ Data obj= lcname##_vector_[osz+i]; \ IndexType index(osz+i); \ Init_obj; \ - if (false) std::cout << *obj << index; \ + if (false) {obj=obj; index=index;} \ } \ Onchanged; \ } \ Index: kernel/include/IMP/optimizers/MoverBase.h =================================================================== --- kernel/include/IMP/optimizers/MoverBase.h (revision 569) +++ kernel/include/IMP/optimizers/MoverBase.h (working copy) @@ -10,6 +10,7 @@ #include "Mover.h" #include "../Particle.h" +#include "../macros.h" #include <vector> @@ -28,9 +29,6 @@ { std::vector<Floats> floats_; std::vector<Ints> ints_; - FloatKeys fkeys_; - IntKeys ikeys_; - Particles ps_; public: virtual void accept_move(){} virtual void reject_move(); @@ -39,59 +37,24 @@ */ virtual void propose_move(float f); + IMP_LIST(public, Particle, particle, Particle*); + IMP_LIST(public, FloatKey, float_key, FloatKey); + IMP_LIST(public, IntKey, int_key, IntKey); + protected: //! implement this method to propose a move /** See NormalMover for a simple example. */ virtual void generate_move(float f)=0; - //! Add to the set of particles being controlled - void add_particles(const Particles &ps) { - ps_.insert(ps_.end(), ps.begin(), ps.end()); - } - //! Clear the set of particles being controlled - void clear_particles() { - ps_.clear(); - } - - //! Get the number of particles - unsigned int number_of_particles() const { - return ps_.size(); - } - Particle *get_particle(unsigned int i) const { - IMP_check(i< ps_.size(), "Bad index in MoverBase::get_particle", - IndexException("Particle index out of range in mover base")); - return ps_[i]; - } - - //! Add an attribute - unsigned int add_key(FloatKey k) { - fkeys_.push_back(k); - return fkeys_.size()-1; - } - //! Add an attribute - unsigned int add_key(IntKey k) { - ikeys_.push_back(k); - return ikeys_.size()-1; - } - - unsigned int number_of_float_keys() const {return fkeys_.size();} - unsigned int number_of_int_keys() const {return ikeys_.size();} - - - //! Get the value of a controlled attribute /** \param [in] i The index of the particle. \param [in] j The index of the attribute. */ Float get_float(unsigned int i, unsigned int j) const { - IMP_assert(i < ps_.size(), - "Out of range controlled attribute in guard"); - IMP_assert(j < fkeys_.size(), - "Out of range controlled attribute in guard"); - IMP_assert(ps_.size() == floats_.size(), + IMP_assert(number_of_particles() == floats_.size(), "Only call get_float from within generate_proposal"); - return get_particle(i)->get_value(fkeys_[j]); + return get_particle(i)->get_value(get_float_key(j)); } //! Get an int attribute value @@ -99,23 +62,19 @@ \param [in] j The index of the attribute. */ Int get_int(unsigned int i, unsigned int j) const { - IMP_assert(i < ps_.size(), - "Out of range controlled attribute in guard"); - IMP_assert(j < ikeys_.size(), - "Out of range controlled attribute in guard"); - IMP_assert(ps_.size() == ints_.size(), - "Only call get_float from within generate_proposal"); - - return get_particle(i)->get_value(ikeys_[j]); + IMP_assert(number_of_particles() == ints_.size(), + "Only call get_int from within generate_proposal"); + return get_particle(i)->get_value(get_int_key(j)); } + //! Propose a value /** \param[in] i The index of the particle. \param[in] j The index of the key \param[in] t The value to propose */ void propose_value(unsigned int i, unsigned int j, Float t) { - if (get_particle(i)->get_is_optimized(fkeys_[j])) { - get_particle(i)->set_value(fkeys_[j], t); + if (get_particle(i)->get_is_optimized(get_float_key(j))) { + get_particle(i)->set_value(get_float_key(j), t); } } //! Propose a value @@ -124,7 +83,7 @@ \param[in] t The value to propose */ void propose_value(unsigned int i, unsigned int j, Int t) { - get_particle(i)->set_value(ikeys_[j], t); + get_particle(i)->set_value(get_int_key(j), t); } MoverBase(){} Index: kernel/include/IMP/optimizers/movers/NormalMover.h =================================================================== --- kernel/include/IMP/optimizers/movers/NormalMover.h (revision 569) +++ kernel/include/IMP/optimizers/movers/NormalMover.h (working copy) @@ -21,16 +21,13 @@ class IMPDLLEXPORT NormalMover :public MoverBase { public: - /** \param[in] ps The particles to perturb. - \param[in] vars The variables to use (normally the keys for x,y,z) + /** \param[in] vars The variables to use (normally the keys for x,y,z) \param[in] sigma The standard deviation to use. + \param[in] ps The particles to perturb. */ - NormalMover(const Particles &ps, const FloatKeys &vars, - Float sigma); - void set_particles(const Particles &ps) { - MoverBase::clear_particles(); - MoverBase::add_particles(ps); - } + NormalMover(const FloatKeys &vars, + Float sigma, const Particles &ps=Particles()); + void set_sigma(Float sigma) { IMP_check(sigma > 0, "Sigma must be positive", ValueException("Negative sigma")); Index: kernel/include/IMP/optimizers/movers/BallMover.h =================================================================== --- kernel/include/IMP/optimizers/movers/BallMover.h (revision 569) +++ kernel/include/IMP/optimizers/movers/BallMover.h (working copy) @@ -26,12 +26,12 @@ public: /** The attributes are perturbed within a pall whose dimensionality is given by the number of attributes and radius by the given value. - \param[in] ps The particles to perturb. \param[in] vars The variables to use (normally the keys for x,y,z) \param[in] radius The radius deviation to use. + \param[in] ps The particles to perturb. */ - BallMover(const Particles &ps, const FloatKeys &vars, - Float radius); + BallMover(const FloatKeys &vars, + Float radius, const Particles &ps); //! void set_radius(Float radius) { IMP_check(radius > 0, "The radius must be positive", Index: kernel/src/optimizers/MoverBase.cpp =================================================================== --- kernel/src/optimizers/MoverBase.cpp (revision 569) +++ kernel/src/optimizers/MoverBase.cpp (working copy) @@ -10,21 +10,25 @@ namespace IMP { +IMP_LIST_IMPL(MoverBase, Particle, particle, Particle*,,); +IMP_LIST_IMPL(MoverBase, FloatKey, float_key, FloatKey,,); +IMP_LIST_IMPL(MoverBase, IntKey, int_key, IntKey,,); + void MoverBase::propose_move(float f) { - if (!fkeys_.empty()) { - floats_.resize(number_of_particles(), Floats(fkeys_.size(), 0)); + if (number_of_float_keys() != 0) { + floats_.resize(number_of_particles(), Floats(number_of_float_keys(), 0)); } - if (!ikeys_.empty()) { - ints_.resize(number_of_particles(), Ints(ikeys_.size(), 0)); + if (number_of_int_keys() != 0) { + ints_.resize(number_of_particles(), Ints(number_of_int_keys(), 0)); } for (unsigned int i=0; i< number_of_particles(); ++i) { Particle *p = get_particle(i); - for (unsigned int j=0; j< fkeys_.size(); ++j) { - floats_[i][j]= p->get_value(fkeys_[j]); + for (unsigned int j=0; j< number_of_float_keys(); ++j) { + floats_[i][j]= p->get_value(get_float_key(j)); } - for (unsigned int j=0; j< ikeys_.size(); ++j) { - ints_[i][j]= p->get_value(ikeys_[j]); + for (unsigned int j=0; j< number_of_int_keys(); ++j) { + ints_[i][j]= p->get_value(get_int_key(j)); } } generate_move(f); @@ -34,11 +38,11 @@ { for (unsigned int i=0; i< number_of_particles(); ++i) { Particle *p = get_particle(i); - for (unsigned int j=0; j< fkeys_.size(); ++j) { - p->set_value(fkeys_[j], floats_[i][j]); + for (unsigned int j=0; j< number_of_float_keys(); ++j) { + p->set_value(get_float_key(j), floats_[i][j]); } - for (unsigned int j=0; j< ikeys_.size(); ++j) { - p->set_value(fkeys_[j], floats_[i][j]); + for (unsigned int j=0; j< number_of_int_keys(); ++j) { + p->set_value(get_int_key(j), ints_[i][j]); } } } Index: kernel/src/optimizers/movers/NormalMover.cpp =================================================================== --- kernel/src/optimizers/movers/NormalMover.cpp (revision 569) +++ kernel/src/optimizers/movers/NormalMover.cpp (working copy) @@ -13,14 +13,12 @@ namespace IMP { -NormalMover::NormalMover(const Particles &pis, - const FloatKeys &vars, - Float max) +NormalMover::NormalMover(const FloatKeys &vars, + Float max, + const Particles &pis) { add_particles(pis); - for (unsigned int i = 0; i < vars.size(); ++i) { - add_key(vars[i]); - } + add_float_keys(vars); set_sigma(max); } Index: kernel/src/optimizers/movers/BallMover.cpp =================================================================== --- kernel/src/optimizers/movers/BallMover.cpp (revision 569) +++ kernel/src/optimizers/movers/BallMover.cpp (working copy) @@ -46,14 +46,12 @@ -BallMover::BallMover(const Particles &pis, - const FloatKeys &vars, - Float max) +BallMover::BallMover(const FloatKeys &vars, + Float max, + const Particles &pis) { add_particles(pis); - for (unsigned int i = 0; i < vars.size(); ++i) { - add_key(vars[i]); - } + add_float_keys(vars); set_radius(max); }

1 0

Re: [IMP-dev] [IMP-commits] r566 - in trunk/kernel: include/IMP include/IMP/internal pyext src src/restraints src/score_states
by Daniel Russel 19 May '08

19 May '08

> > Modified: trunk/kernel/include/IMP/internal/ParticleGrid.h > =================================================================== > --- trunk/kernel/include/IMP/internal/ParticleGrid.h 2008-05-17 > 00:35:56 UTC (rev 565) > +++ trunk/kernel/include/IMP/internal/ParticleGrid.h 2008-05-18 > 07:07:11 UTC (rev 566) > @@ -158,8 +158,8 @@ > } > bool operator==(const ParticleVoxelIterator &o) const { > return cvoxel_== o.cvoxel_ && (cvoxel_ == evoxel_ > - && o.cvoxel_ == o.evoxel_ > - || curp_ == o.curp_); > + && (o.cvoxel_ == o.evoxel_ > + || curp_ == o.curp_)); > } > bool operator!=(const ParticleVoxelIterator &o) const { > return !operator==(o); > This changes the logic and I don't think it is correct. BTW, what didn't gcc 4.3 like with the Keys? Thanks.

2 2

More small patches
by Daniel Russel 16 May '08

16 May '08

- Modify MaxChangeScoreState to use the list macros. - Clean up various attempts at automatically picking parameters in the nonbonded lists. For now just have them be user setable. Also, I don't think that functions which are not part of the API should be given doxygen comments. Index: kernel/include/IMP/score_states/MaxChangeScoreState.h =================================================================== --- kernel/include/IMP/score_states/MaxChangeScoreState.h (revision 560) +++ kernel/include/IMP/score_states/MaxChangeScoreState.h (working copy) @@ -27,9 +27,7 @@ { FloatKeys keys_; FloatKeys origkeys_; - Particles ps_; float max_change_; - virtual void do_before_evaluate(); public: //! Track the changes with the specified keys. MaxChangeScoreState(const FloatKeys &keys, @@ -37,6 +35,8 @@ virtual ~MaxChangeScoreState(){} + IMP_SCORE_STATE(internal::kernel_version_info); + //! Measure differences from the current value. void reset(); Index: kernel/src/score_states/MaxChangeScoreState.cpp =================================================================== --- kernel/src/score_states/MaxChangeScoreState.cpp (revision 560) +++ kernel/src/score_states/MaxChangeScoreState.cpp (working copy) @@ -74,4 +74,8 @@ max_change_=0; } +void MaxChangeScoreState::show(std::ostream &out) const { + out << "MaxChangeScoreState" << std::endl; +} + } // namespace IMP Index: kernel/include/IMP/score_states/NonbondedListScoreState.h =================================================================== --- kernel/include/IMP/score_states/NonbondedListScoreState.h (revision 560) +++ kernel/include/IMP/score_states/NonbondedListScoreState.h (working copy) @@ -42,15 +42,19 @@ /** How much to add to the size of particles to allow particles to move without rebuilding the list */ Float slack_; - //! An estimate of what the slack should be next time the list is recomputed - Float next_slack_; - int num_steps_; bool nbl_is_valid_; int number_of_rebuilds_; int number_of_updates_; + int number_of_overflows_; + //! The maximum allowable size for the NBL + /** An exception will be thrown if the list exceeds this size. + */ + unsigned int max_nbl_size_; typedef std::vector<std::pair<Particle*, Particle*> > NBL; NBL nbl_; + struct NBLTooLargeException{}; + protected: Float get_slack() const {return slack_;} @@ -93,7 +97,11 @@ if (!are_bonded(a,b)) { IMP_LOG(VERBOSE, "Found pair " << a->get_index() << " " << b->get_index() << std::endl); - nbl_.push_back(std::make_pair(a, b)); + if (nbl_.size() < max_nbl_size_) { + nbl_.push_back(std::make_pair(a, b)); + } else { + throw NBLTooLargeException(); + } } else { IMP_LOG(VERBOSE, "Pair " << a->get_index() << " and " << b->get_index() << " rejected on bond" @@ -184,6 +192,29 @@ FloatKey get_radius_key() const {return rk_;} void set_radius_key(FloatKey rk) {rk_=rk;} + //! Set the maximum allowable size for the NBL + /** The NBL will keep reducing the slack and trying to + rebuild until it can make the list smaller than this. + */ + void set_max_size(unsigned int mx) { + max_nbl_size_= mx; + } + + + //! Set the slack used when generating the NBL + /** The slack allows the the NBL to non be rebuilt every step + making the process more efficient. However, too large + a value can result in the NBL being excessively large. + + A good guideline is that it should be the maximum amount + a particle coordinate would change in 20 steps or so. + */ + void set_slack(float slack) { + IMP_check(slack>= 0, "Slack must be nonnegative", + ValueException("Negative slack")); + slack_=slack; + } + IMP_CONTAINER(BondedListScoreState, bonded_list, BondedListIndex); Index: kernel/src/score_states/NonbondedListScoreState.cpp =================================================================== --- kernel/src/score_states/NonbondedListScoreState.cpp (revision 560) +++ kernel/src/score_states/NonbondedListScoreState.cpp (working copy) @@ -37,11 +37,11 @@ cutoff_(cut), nbl_is_valid_(false) { - slack_=20; - next_slack_=slack_; - num_steps_=1; + slack_=cutoff_; number_of_updates_=1; number_of_rebuilds_=0; + number_of_overflows_=0; + max_nbl_size_= std::numeric_limits<unsigned int>::max(); } @@ -55,7 +55,9 @@ { out << "Nonbonded list averaged " << static_cast<Float>(number_of_updates_) - / number_of_rebuilds_ << " steps between rebuilds" << std::endl; + / number_of_rebuilds_ << " steps between rebuilds" + << " and overflowed " << number_of_overflows_ + << " times" << std::endl; } @@ -96,47 +98,25 @@ (*bli)->before_evaluate(ScoreState::get_before_evaluate_iteration()); } - if (nbl_is_valid_) { - /*std::cout << "Rate is " << rate << " target is " << target_steps - << " so slack is " << target_slack << " mc " << mc - << " nbl " << nbl_.size() << " cost " - << rebuild_cost << std::endl;*/ - if (mc > slack_) { - /* Float rate= std::pow(static_cast<Float>(nbl_.size()), - .333f)/ num_steps_; - Float target_steps= .6*std::pow(rebuild_cost, .25f) - *std::pow(rate, -.75f); - Float target_slack= (target_steps+1)*mc/num_steps_; - next_slack_= target_slack*.5 + .5*next_slack_; - */ - - /*std::cout << "Killing nbl because " << mc << " - << slack_ << " " << next_slack_ - << " " << num_steps_ << std::endl;*/ - if (num_steps_ < 50) { - //slack_= next_slack_; - } - num_steps_=1; - //next_slack_= std::max(2.0*mc, 2.0*slack_); - set_nbl_is_valid(false); - ++number_of_rebuilds_; - slack_=next_slack_; - /*} else if (num_steps_ > 100) { - //next_slack_=slack_/2.0; - slack_=next_slack_; - num_steps_=1; - set_nbl_is_valid(false); - ++number_of_rebuilds_;*/ - } else { - ++num_steps_; - //next_slack_= next_slack_*.98; - } - } - bool rebuilt=false; if (!get_nbl_is_valid()) { - rebuild_nbl(); - rebuilt=true; + unsigned int rebuild_attempts=0; + do { + try { + ++rebuild_attempts; + ++number_of_rebuilds_; + rebuild_nbl(); + rebuilt=true; + } catch (NBLTooLargeException &) { + slack_= slack_/2.0; + ++number_of_overflows_; + if (number_of_rebuilds_==100) { + IMP_WARN("Can't rebuild NBL with given max NBL size of " + << max_nbl_size_ << std::endl); + throw ValueException("Bad NBL max size"); + } + } + } while (!rebuilt); } else { nbl_.erase(std::remove_if(nbl_.begin(), nbl_.end(), internal::HasInactive()),

2 3

Minor patches
by Daniel Russel 16 May '08

16 May '08

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>

1 0

IMP_BEGIN_CHECK
by Daniel Russel 15 May '08

15 May '08

As any of you who looked at the syntax may have noticed, the current syntax is a bit evil since you write IMP_BEGIN_CHECK(EXPENSIVE); some stuff which gets controlled by the surrounding statement and is in a nested scope IMP_END_CHECK; We want something that naturally conveys that the surrounded blocks are in a nested scope. One possibility is something like IMP_IF_CHECK(level) which would expand to if (get_check_level() > level) So the user has to add braces to put more than one statement in there. I think I like that better and emacs does the right thing with indenting it. Any thoughts?

2 1

ObjectContainer disabling checks
by Daniel Russel 15 May '08

15 May '08

I had missed wrapping one of the checks in ObjectContainer with NDEBUG which made adding large numbers of particles painfully slow. I can't submit a patch since it would conflict with the not yet committed changes. It is simple, the single object insert needs to have its check wrapped with IMP_BEGIN_CHECK(EXPENSIVE); IMP_END_CHECK And the multipartcle check which is currently CHEAP should be made EXPENSIVE.

1 0

GravityCenter::set_position...
by Daniel Russel 13 May '08

13 May '08

really shouldn't be called that as we have set_X take a value everywhere else. The method updates the position from its parts and so should be called something more like that.

1 0

Re: [IMP-dev] NBL cleanup and other small patches
by Ben Webb 13 May '08

13 May '08

Daniel Russel wrote: >> 'scons doc'. You do, however, need to have a complete DocBook toolchain >> installed. > Well, I guess flute doesn't have such then (although it does seem to > have some of docbook installed) > scons doc > blah, blah, blah > scons *** Do not know how to make target 'doc'. Stop. Oh, sorry. It should be 'kernel/doc'. Forgot that we moved all the kernel stuff to a subdirectory some time ago. > I haven't used docbook. For what we have so far, doxygen is much simpler > (and prettier :-), but perhaps you have some more involved directions to > take things. Docbook certain went overboard on the markup :-) Indeed - it's XML. The DocBook stuff right now is very basic, because most of today's documentation is API documentation. But once we have less technical users - as is the long term intention - they will need more traditional book-style documentation (of course pointing them to the docbook docs for any technical stuff). Ben -- ben(a)salilab.org http://salilab.org/~ben/ "It is a capital mistake to theorize before one has data." - Sir Arthur Conan Doyle

2 2

minor patch
by Friedrich Foerster 12 May '08

12 May '08

would anybody object the below patch? we (keren and me) need it for magic sphimp (i don't remember why, but keren will know). thanks frido Index: kernel/include/IMP/Restraint.h =================================================================== --- kernel/include/IMP/Restraint.h (revision 539) +++ kernel/include/IMP/Restraint.h (working copy) @@ -94,7 +94,8 @@ return model_.get(); } - IMP_LIST(protected, Particle, particle, Particle*) + //IMP_LIST(protected, Particle, particle, Particle*) + IMP_LIST(public, Particle, particle, Particle*) private: internal::ObjectPointer<Model, false> model_;

3 3