Proposed bonded/nonbonded list interfaces
Since I needed caching of my bonds so I decided to refactor things to have bonded and nonbonded lists. Here are my proposed interfaces for people's perusal. I then have a DynamicExclusionVolumeRestraint and DynamicBondRestraint which take pointers to these states and get their particle pairs from them.
I don't know the best way to make things accessible from python (I don't think the C++ iterators will work from python, but I haven't tried).
For reference, a BondDecorator is a decorator wrapping a particle which contains information (length, stiffness, type) for a bond.
//! A pair of Particle * which can be used in maps struct ParticlePair { ParticlePair(Particle *a, Particle *b); Particle *get_particle(unsigned int i) const ; IMP_COMPARISONS_2(p_[0], p_[1]); };
//! A state that maintains a list of bonded particles. /** The state uses the BondedDecorator to find bonded pairs in its list of particles and builds a fast lookup structure on them. */ class IMPDLLEXPORT BondedListState : public State { public: BondedListState(const Particles &pis); virtual ~BondedListState();
IMP_STATE("0.5", "Daniel Russel");
void set_particles(const Particles &pis) ;
typedef std::map<ParticlePair, BondDecorator>::const_iterator BondsIterator; BondsIterator bonds_begin() const ; BondsIterator bonds_end() const ; //! Get a bonds between two particles /** \returns BondDecorator() if there is not bond or the appropriate one if there is a bond. */ BondDecorator get_bond(Particle *a, Particle *b) const ; };
//! A state that maintains a list of nonbonded particles. /** */ class IMPDLLEXPORT NonbondedListState : public State { public: /** \note The distance_threshold parameter is just a guideline. Particle pairs with larger distance can be returned. But all with smaller distance will be.
BondedListState will be updated when this one is and deleted when this State is and so should not be added seperately to the Model. */ NonbondedListState(const Particles &pis, Float distance_threshold, BondedList *bl=NULL); virtual ~NonbondedListState();
IMP_STATE("0.5", "Daniel Russel");
void set_particles(const Particles &pis);
/*Eventually these iterators could generate the pairs on the fly from the voxel grid*/ typedef std::vector<ParticlePair>::const_iterator NonbondsIterator; NonbondsIterator nonbonds_begin() const ; NonbondsIterator nonbonds_end() const ; };
Daniel Russel wrote: > Since I needed caching of my bonds so I decided to refactor things to > have bonded and nonbonded lists. Here are my proposed interfaces for > people's perusal. I then have a DynamicExclusionVolumeRestraint and > DynamicBondRestraint which take pointers to these states and get their > particle pairs from them.
The interfaces look reasonable to me. But the list of bonded particles should be static, IMHO - it's a waste to rebuild it at every score evaluation - since it'll only change if you add/remove particles or bonds. But I guess that's an implementation detail.
> I don't know the best way to make things accessible from python (I > don't think the C++ iterators will work from python, but I haven't > tried).
I think we can punt on that. Anybody foolish enough to write a nonbonded term in Python is going to have to put up with incredibly slow performance anyway, so they may as well just do an NxN double loop.
> For reference, a BondDecorator is a decorator wrapping a particle > which contains information (length, stiffness, type) for a bond.
This is the only thing I have a problem with. What do these three parameters mean? I guess length and stiffness translate to the mean and standard deviation of a Gaussian restraint. But this isn't the best place to put this kind of thing - what if the user wanted to use a cubic spline instead? A bond generally just identifies a pair of atoms, and the restraint contains the length/stiffness/whatever information. This could in turn be populated from your force field (e.g. using atom types).
Ben
Ben Webb wrote: > The interfaces look reasonable to me. But the list of bonded particles > should be static, IMHO - it's a waste to rebuild it at every score > evaluation - since it'll only change if you add/remove particles or > bonds. But I guess that's an implementation detail. > What do you mean by static? Only updated occasionally? You only call the set_particles method when the set of particles you care about changes. For most people, that will be exactly never. But I need to be able to update things and you can't easily rebuild the state as there is no way of knowing all the Restraints which need pointers to it short of rebuilding everything.
Static as in the keyword would be bad :-)
> This is the only thing I have a problem with. What do these three > parameters mean? I guess length and stiffness translate to the mean and > standard deviation of a Gaussian restraint. But this isn't the best > place to put this kind of thing - what if the user wanted to use a cubic > spline instead? A bond generally just identifies a pair of atoms, and > the restraint contains the length/stiffness/whatever information. This > could in turn be populated from your force field (e.g. using atom types). > I guess I should have been more careful in how I described things. The BondedState is oblivious to what is actually put in the BondDecorator. It just passes some (unknown) information back (we could just pass back a Particle* instead). What is done with that information is up to the restraint that uses it. I have a restraint that just uses them to make a harmonic potential, but the restraints are trivial and new ones can be written easily.
We do, eventually, need some way of having different types of bonds (with different BondedListStates) all of which are used by the same NonbondedListState. One way to do this would be to have the bond restraints check a bond type field in the bond and only process ones of a recognized type. An alternative would be to have the NonbondedListState have a vector of BondedListStates and look in each of them.
Given that the number of bonds should generally be smaller than the number of non-bonded pairs, it should be a bit more efficient to have the restraints check the bond type.
Daniel Russel wrote: > Ben Webb wrote: >> The interfaces look reasonable to me. But the list of bonded particles >> should be static, IMHO - it's a waste to rebuild it at every score >> evaluation - since it'll only change if you add/remove particles or >> bonds. But I guess that's an implementation detail. >> > What do you mean by static? Only updated occasionally? You only call the > set_particles method when the set of particles you care about changes.
I think that's what I said. ;) But yes, in most cases your set of particles will be set once and then never change. If you only update the bond list at set_particles() time rather than at update() time, that's fine with me.
> I guess I should have been more careful in how I described things. The > BondedState is oblivious to what is actually put in the BondDecorator.
Yes, that's what I figured you meant. It's not the BondedState I have a problem with.
> It just passes some (unknown) information back (we could just pass back > a Particle* instead). What is done with that information is up to the > restraint that uses it. I have a restraint that just uses them to make a > harmonic potential, but the restraints are trivial and new ones can be > written easily.
Sure - I just think you're going to have a lot of trouble forcing all of the necessary information into this bond decorator. In the cubic spline case, that means a vector of floats, for example.
Ben
Ben Webb wrote: > It just passes some (unknown) information back (we could just pass back > > Sure - I just think you're going to have a lot of trouble forcing all of > the necessary information into this bond decorator. In the cubic spline > case, that means a vector of floats, for example. > You can always create a new decorator class which extends the existing one and wraps the return.
That said, I think I want to eventually move the identities of the bonded particles out of the the BondDecorator particle. Once I have done that, there is not any general information in the particle and so the BondDecorator should probably become something more specific. So returning a Particle* now seems like a better idea.
participants (2)
-
Ben Webb -
Daniel Russel