Ben Webb wrote: > Daniel Russel wrote: >> - Clean up various attempts at automatically picking parameters in >> the nonbonded lists. For now just have them be user setable. > > With this patch, 'scons examples' always fails for me on synth, with > errors like: > > scons: Building targets ... > builder_unit_test(["kernel/doc/examples/examples.passed"], > ["bin/imppy.sh", "kernel/doc/examples/chain.py"]) > cd /synth1/home/ben/imp/kernel/doc/examples && > /synth1/home/ben/imp/bin/imppy.sh python chain.py > /dev/null > ERROR: Nonbonded list is missing (7) (5.34008, 3.38164, 5.22317) 1 and > (1) (7.81002, 2.4397, 4.51924)1 size is 75 That was dumb. In cleaning it up for submit I disabled rebuilding of nbl. Oops. Strange that it didn't get triggered on my machine.

Here is an improved patch

While I am at it, here is a patch which moves the "-O3" flag for gcc to being controlled by the release flag as it makes non release building take a really long time and be hard to debug.

Finally, I cleaned up the example slightly

Index: kernel/include/IMP/score_states/NonbondedListScoreState.h =================================================================== --- kernel/include/IMP/score_states/NonbondedListScoreState.h (revision 561) +++ kernel/include/IMP/score_states/NonbondedListScoreState.h (working copy) @@ -42,15 +42,18 @@ /** 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_; - typedef std::vector<std::pair<Particle*, Particle*> > NBL; - NBL nbl_; + 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_; + ParticlePairs nbl_;

+ struct NBLTooLargeException{}; + protected:

Float get_slack() const {return slack_;} @@ -93,7 +96,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 +191,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);

@@ -201,7 +231,7 @@ /** The value type is an ParticlePair. */ typedef boost::filter_iterator<BoxesOverlap, - NBL::const_iterator> NonbondedIterator; + ParticlePairs::const_iterator> NonbondedIterator;

//! Iterates through the pairs of non-bonded particles NonbondedIterator nonbonded_begin() const { @@ -217,6 +247,9 @@ nbl_.end(), nbl_.end()); }

+ const ParticlePairs get_nonbonded() const { + return nbl_; + }

unsigned int number_of_nonbonded() const { IMP_check(get_nbl_is_valid(), "Must call update first", Index: kernel/src/score_states/NonbondedListScoreState.cpp =================================================================== --- kernel/src/score_states/NonbondedListScoreState.cpp (revision 561) +++ 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,26 @@ (*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; + if (mc > slack_ || !get_nbl_is_valid()) { + unsigned int rebuild_attempts=0; + do { + try { + ++rebuild_attempts; + ++number_of_rebuilds_; + set_nbl_is_valid(false); + 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()), @@ -154,7 +135,7 @@ { nbl_is_valid_= tf; if (!nbl_is_valid_) { - NBL empty; + ParticlePairs empty; // free memory to make sure it shrinks std::swap(empty, nbl_); } Index: kernel/src/score_states/AllNonbondedListScoreState.cpp =================================================================== --- kernel/src/score_states/AllNonbondedListScoreState.cpp (revision 561) +++ kernel/src/score_states/AllNonbondedListScoreState.cpp (working copy) @@ -94,6 +94,7 @@ } if (P::update(mc, cost)) { mc_->reset(); + mcr_->reset(); } IMP_IF_CHECK(EXPENSIVE) { check_nbl(); @@ -334,7 +335,9 @@ << " " << gr(ps[i]) << " and " << ps[j]->get_index() << " " << dj << gr(ps[j]) - << " size is " << number_of_nonbonded() << std::endl); + << " size is " << number_of_nonbonded() + << " distance is " << distance(di, dj) + << " max is " << mc_->get_max() << std::endl); } } }

Index: tools/__init__.py =================================================================== --- tools/__init__.py (revision 561) +++ tools/__init__.py (working copy) @@ -53,7 +53,9 @@ def _add_release_flags(env): """Add compiler flags for release builds, if requested""" if env.get('release', False): - env.Append(CPPDEFINES='NDEBUG') + env.Append(CPPDEFINES=['NDEBUG']) + if env['CC'] == 'gcc': + env.Append(CCFLAGS=['-O3'])

def CheckGNUHash(context): """Disable GNU_HASH-style linking (if found) for backwards compatibility""" @@ -186,7 +188,7 @@ pass env.Prepend(SCANNERS = _SWIGScanner) if env['CC'] == 'gcc': - env.Append(CCFLAGS="-Wall -g -O3") + env.Append(CCFLAGS="-Wall -g") if env.get('include', None) is not None: env['include'] = [os.path.abspath(x) for x in \ env['include'].split(os.path.pathsep)]

Index: kernel/doc/examples/chain.py =================================================================== --- kernel/doc/examples/chain.py (revision 561) +++ kernel/doc/examples/chain.py (working copy) @@ -7,27 +7,28 @@ # another.

#IMP.set_log_level(IMP.VERBOSE) +np=20 radius =1.0 rk= IMP.FloatKey("radius") m= IMP.Model() # The particles in the chain chain= IMP.Particles() -for i in range(0,20): +for i in range(0,np): p= IMP.Particle() pi= m.add_particle(p) d= IMP.XYZDecorator.create(p) - d.set_x(random.uniform(0,10)) - d.set_y(random.uniform(0,10)) - d.set_z(random.uniform(0,10)) + d.randomize_in_box(IMP.Vector3D(0,0,0), + IMP.Vector3D(10,10,10)) d.set_coordinates_are_optimized(True) p.add_attribute(rk, radius, False) - # create a bond between successive particles - if (i != 0): - bp= IMP.BondedDecorator.create(p) - bpr= IMP.BondedDecorator.cast(chain.back()) - b= IMP.custom_bond(bp, bpr, 1.5*radius, 10) chain.append(p)

+# create a bond between successive particles +for i in range(1, len(chain)): + bp= IMP.BondedDecorator.create(chain[i]) + bpr= IMP.BondedDecorator.cast(chain[i-1]) + b= IMP.custom_bond(bp, bpr, 1.5*radius, 10) + # If you want to inspect the particles # Notice that each bond is a particle for p in m.get_particles():