Annoyance with current restraint design
I have some restraints which I want to have be semi-dynamic, meaning, I want to be able to reset the list of particles which they act on. This doesn't work so cleanly with restraints taking lists of ParticleIndexes as then you need one version of set_particles to be called from the constructor (where the Restraint::model_ is not set) and another version to be called by outsiders after the model pointer has been set (or pass the model pointer to both and ignore the stored pointer).
This seems like a reason for writing restraints which take Particles rather than ParticleIndexes. It may also be worth dropping the Restraint::model_ pointer entirely and just using the pointer stored in particles. So
DistanceRestraint::DistanceRestraint(Particle*, Particle*, ScoreFunc*) ExclusionVolumeRestraint(Particles, ScoreFuncParams)
Any thoughts?
Daniel Russel wrote: > I have some restraints which I want to have be semi-dynamic, meaning, > I want to be able to reset the list of particles which they act on. > This doesn't work so cleanly with restraints taking lists of > ParticleIndexes as then you need one version of set_particles to be > called from the constructor (where the Restraint::model_ is not set) > and another version to be called by outsiders after the model pointer > has been set (or pass the model pointer to both and ignore the stored > pointer). > > This seems like a reason for writing restraints which take Particles > rather than ParticleIndexes.
Agreed. As we already discussed, Particle* is easier to pass from Python than ParticleIndex anyway. The only downsides are that 1) There's no easy way to check for a 'bad' particle this way (a corrupt Particle could have a pointer to the model, but the model may not have the corresponding pointer to the Particle). With particle indexes of course you just check whether the index is in range. 2) This works only as long as the model's particle storage is std::vector<Particle*>. For example, we couldn't switch to std::vector<Particle> because then the Particle* could change when the vector is resized. 3) To be really careful, we should add checks that all particles live in the same model.
I don't think these are huge problems, however.
Ben
Ben Webb wrote: > Agreed. As we already discussed, Particle* is easier to pass from Python > than ParticleIndex anyway. The only downsides are that > 1) There's no easy way to check for a 'bad' particle this way (a corrupt > Particle could have a pointer to the model, but the model may not have > the corresponding pointer to the Particle). With particle indexes of > course you just check whether the index is in range. Sure we can (untested code) for Restraint.h: void set_model(Model* model){ IMP_assert(particles_.empty() || model == particles_[0]->get_model(), "Model* different from Particle Model*"); model_=model; }
int add_particle(Particle *p) { IMP_assert(p != NULL, "Can't add NULL particle"); particles_.push_back(p); IMP_assert(particles_[0]->get_model() == particles_.back()->get_model(), "All particles in restraint must be from the same model."); IMP_assert(particles_.back()->get_model() ->get_particle(particles_.back()->get_index()) == particles_.back(), "Model does not have pointer to particle."); IMP_assert(model_== NULL || model_==particles_.back()->get_model(), "Restraint model pointer and particle model pointer " << "don't match."); return particles_.size()-1; }
> 2) This works only as long as the model's particle storage is > std::vector<Particle*>. For example, we couldn't switch to > std::vector<Particle> because then the Particle* could change when the > vector is resized. > True. But other things would break too if we did that, so lets not :-)
participants (2)
-
Ben Webb -
Daniel Russel