I had mentioned this a while ago and would like to bring it up again since now seems like a good time to deal with it: I would like to remove the particle storage in the Restraint base class.
The reasons for this are: - a significant fraction of the restraints don't use it so it is just taking up space (not significant) and presenting an opportunity for bugs since you could accidently look at the wrong storage or use the default get_interacting_particles when it is inappropriate. - if a Restraint does used it, it imposes an unstructured storage mechanism on the restraint: that is, restrictions on the set of particles (since that it is always two particles, or always a set of pairs) are ignored. This makes a number of the restraints more complicated. - for the Restraints which do use it, the amount of code savings in minimal to negative (it is negative in the case where the raw storage mechanism is exposed since that has to be done through using statements. - If it is removed, to port restraints which currently use it you would simply add: IMP_LIST(protected, Particle, particle, Particle*) to your class declaration and IMP_LIST_IMPL(Restraint, Particle, particle,Particle*,,,); in your .cpp file. if you used the default get_interacting_particles, (you are probably doing a bad thing), you would also have to add ParticlesList get_interacting_particles() const { return ParticlesList(1, Particles(particles_begin(), particles_end())); } in your header.
Daniel Russel wrote: > I had mentioned this a while ago and would like to bring it up again > since now seems like a good time to deal with it: I would like to remove > the particle storage in the Restraint base class.
Sounds reasonable to me. But why not go further and port existing restraints not by using the IMP_LIST macros but by giving them containers? Do you see any need for a restraint containing a particle list that a container would not fulfill?
Ben
Ben Webb wrote: > Daniel Russel wrote: >> I had mentioned this a while ago and would like to bring it up again >> since now seems like a good time to deal with it: I would like to >> remove the particle storage in the Restraint base class. > > Sounds reasonable to me. But why not go further and port existing > restraints not by using the IMP_LIST macros but by giving them > containers? Do you see any need for a restraint containing a particle > list that a container would not fulfill? No particular reason, just more work and requires actual interface changes. I'd be for it in general though.
Daniel,
you mentioned in your original email: " if you used the default get_interacting_particles, (you are probably doing a bad thing), you would also have to add ParticlesList get_interacting_particles() const { return ParticlesList(1, Particles(particles_begin(), particles_end())); } in your header. "
The DOMINO optimizer is using get_interacting_particles to build the restraint graph of the particles. In addition, In assembler Frido and I use this function for analysis purposes.
If you are going a head with your change - can you please make sure that all of the existing restraints ( maybe except for non-bonded) implement this function? thanks, Keren. On Jan 16, 2009, at 12:25 PM, Daniel Russel wrote:
> Ben Webb wrote: >> Daniel Russel wrote: >>> I had mentioned this a while ago and would like to bring it up >>> again since now seems like a good time to deal with it: I would >>> like to remove the particle storage in the Restraint base class. >> >> Sounds reasonable to me. But why not go further and port existing >> restraints not by using the IMP_LIST macros but by giving them >> containers? Do you see any need for a restraint containing a >> particle list that a container would not fulfill? > No particular reason, just more work and requires actual interface > changes. I'd be for it in general though. > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev
Indeed. I thought I put that in my email, but I guess I forgot :-) Actually, now, a restraint must implement it as it is a pure virtual method.
On Jan 17, 2009, at 3:45 AM, Keren Lasker wrote:
> Daniel, > > you mentioned in your original email: > " > if you used the default get_interacting_particles, (you are probably > doing a bad thing), you would also have to add > ParticlesList get_interacting_particles() const > { > return ParticlesList(1, Particles(particles_begin(), > particles_end())); > } > in your header. > " > > The DOMINO optimizer is using get_interacting_particles to build the > restraint graph of the particles. In addition, In assembler Frido > and I use this function for analysis purposes. > > If you are going a head with your change - can you please make sure > that all of the existing restraints ( maybe except for non-bonded) > implement this function? > thanks, > Keren. > On Jan 16, 2009, at 12:25 PM, Daniel Russel wrote: > >> Ben Webb wrote: >>> Daniel Russel wrote: >>>> I had mentioned this a while ago and would like to bring it up >>>> again since now seems like a good time to deal with it: I would >>>> like to remove the particle storage in the Restraint base class. >>> >>> Sounds reasonable to me. But why not go further and port existing >>> restraints not by using the IMP_LIST macros but by giving them >>> containers? Do you see any need for a restraint containing a >>> particle list that a container would not fulfill? >> No particular reason, just more work and requires actual interface >> changes. I'd be for it in general though. >> _______________________________________________ >> IMP-dev mailing list >> IMP-dev@salilab.org >> https://salilab.org/mailman/listinfo/imp-dev > > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev
participants (3)
-
Ben Webb -
Daniel Russel -
Keren Lasker