hi all (i.e. ben & daniel),
i tried to deactivate particles and optimize only selected parts of a model. when using the em restraint i observed an oddity: restraints are not evaluated, if ONE particle in the set is not active. i do not think this is a sensible thing, for example my em restraint was not evaluated at all... i attached a patch where a restraint is omitted only if ALL particles are inactive. if you agree that it makes more sense, please commit it to imp.
best
frido
Friedrich Foerster wrote: > i tried to deactivate particles and optimize only selected parts of a > model. when using the em restraint i observed an oddity: restraints are > not evaluated, if ONE particle in the set is not active. i do not think > this is a sensible thing, for example my em restraint was not evaluated > at all... > i attached a patch where a restraint is omitted only if ALL particles > are inactive. > if you agree that it makes more sense, please commit it to imp.
I don't think it makes sense to change this in the Restraint base class, since that's a dangerous 'default' behavior. Turning off a particle in IMP effectively deletes it, so it shouldn't interact any more with any other particle. You could change this method just for your EM restraint, but then you'd also have to check inside the restraint whether each particle is active at evaluation time.
If you just don't want these particles to move, you can call the set_is_optimized method on these particles' x, y and z coordinates.
Ben
Ben Webb wrote: > I don't think it makes sense to change this in the Restraint base class, > since that's a dangerous 'default' behavior. Turning off a particle in > IMP effectively deletes it, so it shouldn't interact any more with any > other particle. You could change this method just for your EM restraint, > but then you'd also have to check inside the restraint whether each > particle is active at evaluation time. > I think the current behavior is kind of stupid. As it stands, if you inactivate a particle and forget to update all the restraints, your optimization just silently fails (since some restraints just stop being used). This is really bad (as Frido presumably discovered). We can print warnings if restraints are inactivated, but this is kind of awkward as you then pollute the log for anyone who wants to depend on the "correct" behavior.
In addition, we don't have support for the automatic deactivation in the States, so these now work differently than the Restraints, which is a bit silly.
If we want to think about it like deleting the particle, then we should make it a reported error to touch an inactive particle. This is trivial to implement. It would then be the restraints responsibility to check that the particles are all active. > If you just don't want these particles to move, you can call the > set_is_optimized method on these particles' x, y and z coordinates. > Agreed.
Ben Webb wrote: > Friedrich Foerster wrote: > >> i tried to deactivate particles and optimize only selected parts of a >> model. when using the em restraint i observed an oddity: restraints are >> not evaluated, if ONE particle in the set is not active. i do not think >> this is a sensible thing, for example my em restraint was not evaluated >> at all... >> i attached a patch where a restraint is omitted only if ALL particles >> are inactive. >> if you agree that it makes more sense, please commit it to imp. >> > > I don't think it makes sense to change this in the Restraint base class, > since that's a dangerous 'default' behavior. Turning off a particle in > IMP effectively deletes it, so it shouldn't interact any more with any > other particle. You could change this method just for your EM restraint, > but then you'd also have to check inside the restraint whether each > particle is active at evaluation time. > the problem goes beyond the EM restraint: the ExcludedVolume restraint is also not evaluated if only one particle is switched off. if the particle switch does evil things, probably it should deleted anyway. i am happy to use set_is_optimized instead - but as it stands now, the switch remains a trap the next person is bound to fall into. > If you just don't want these particles to move, you can call the > set_is_optimized method on these particles' x, y and z coordinates. > > Ben >
So does anyone thin that getting rid of the Is_active check entirely in Restraint/Model and replacing it will asserts in particle is a bad idea?
On Feb 11, 2008, at 8:47 PM, Friedrich Foerster wrote:
> Ben Webb wrote: >> Friedrich Foerster wrote: >> >>> i tried to deactivate particles and optimize only selected parts >>> of a >>> model. when using the em restraint i observed an oddity: >>> restraints are >>> not evaluated, if ONE particle in the set is not active. i do not >>> think >>> this is a sensible thing, for example my em restraint was not >>> evaluated >>> at all... >>> i attached a patch where a restraint is omitted only if ALL >>> particles >>> are inactive. >>> if you agree that it makes more sense, please commit it to imp. >>> >> >> I don't think it makes sense to change this in the Restraint base >> class, >> since that's a dangerous 'default' behavior. Turning off a >> particle in >> IMP effectively deletes it, so it shouldn't interact any more with >> any >> other particle. You could change this method just for your EM >> restraint, >> but then you'd also have to check inside the restraint whether each >> particle is active at evaluation time. >> > the problem goes beyond the EM restraint: the ExcludedVolume restraint > is also not evaluated if only one particle is switched off. if the > particle switch does evil things, probably it should deleted anyway. i > am happy to use set_is_optimized instead - but as it stands now, the > switch remains a trap the next person is bound to fall into. >> If you just don't want these particles to move, you can call the >> set_is_optimized method on these particles' x, y and z coordinates. >> >> Ben >> > > > -- > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% > % Friedrich Foerster % > % Andrej Sali Lab % > % University of California at San Francisco % > % MC 2552 % > % Byers Hall Room 501 % > % 1700 4th Street % > % San Francisco, CA 94158-2330, USA % > % % > % phone: +1 (415) 514-4258 % > % fax: +1 (415) 514-4231 % > % % > % www.salilab.org/~frido % > % % > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% > > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev
fine with me. that should save others from being trapped. btw: the is_optimized flag eventually did what i expected from the is_active flag.
frido
Daniel Russel wrote: > So does anyone thin that getting rid of the Is_active check entirely > in Restraint/Model and replacing it will asserts in particle is a bad > idea? > > > On Feb 11, 2008, at 8:47 PM, Friedrich Foerster wrote: > > >> Ben Webb wrote: >> >>> Friedrich Foerster wrote: >>> >>> >>>> i tried to deactivate particles and optimize only selected parts >>>> of a >>>> model. when using the em restraint i observed an oddity: >>>> restraints are >>>> not evaluated, if ONE particle in the set is not active. i do not >>>> think >>>> this is a sensible thing, for example my em restraint was not >>>> evaluated >>>> at all... >>>> i attached a patch where a restraint is omitted only if ALL >>>> particles >>>> are inactive. >>>> if you agree that it makes more sense, please commit it to imp. >>>> >>>> >>> I don't think it makes sense to change this in the Restraint base >>> class, >>> since that's a dangerous 'default' behavior. Turning off a >>> particle in >>> IMP effectively deletes it, so it shouldn't interact any more with >>> any >>> other particle. You could change this method just for your EM >>> restraint, >>> but then you'd also have to check inside the restraint whether each >>> particle is active at evaluation time. >>> >>> >> the problem goes beyond the EM restraint: the ExcludedVolume restraint >> is also not evaluated if only one particle is switched off. if the >> particle switch does evil things, probably it should deleted anyway. i >> am happy to use set_is_optimized instead - but as it stands now, the >> switch remains a trap the next person is bound to fall into. >> >>> If you just don't want these particles to move, you can call the >>> set_is_optimized method on these particles' x, y and z coordinates. >>> >>> Ben >>> >>> >> -- >> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% >> % Friedrich Foerster % >> % Andrej Sali Lab % >> % University of California at San Francisco % >> % MC 2552 % >> % Byers Hall Room 501 % >> % 1700 4th Street % >> % San Francisco, CA 94158-2330, USA % >> % % >> % phone: +1 (415) 514-4258 % >> % fax: +1 (415) 514-4231 % >> % % >> % www.salilab.org/~frido % >> % % >> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% >> >> _______________________________________________ >> 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 >
Daniel Russel wrote: > So does anyone thin that getting rid of the Is_active check entirely > in Restraint/Model and replacing it will asserts in particle is a bad > idea?
Yes, I agree - if people really want to delete particles, they will have to make sure their restraints are deleted also. But we certainly need to allow users to delete particles - without that applications such as grand canonical MC are impossible. The is_active stuff was Bret's proposal for this.
I also propose that we change the member from 'is_active' to 'is_deleted'. This will make it more obvious than the particle has really been deleted and cannot be used any more. ('inactive' is too easy to interpret as 'do not optimize'.)
Unless anybody else really wants to do it, I will write the code and a bunch of unit tests for this today.
Finally, we have a class of restraints which commonly act on "all atoms". For example, any kind of nonbonded restraint is often set up this way. I think it would be rather annoying for users to have to recreate these restraints whenever they remove a particle, so I suggest that the NonbondedListScoreState class provides a remove_particles() method to remove particles. (This can also be useful for removing still-active particles from a nonbonded list, if desired.) For symmetry, perhaps the set_particles() method should also be renamed to add_particles(). But this can all be done later.
Ben
> recreate these restraints whenever they remove a particle, so I > suggest > that the NonbondedListScoreState class provides a remove_particles() > method to remove particles. (This can also be useful for removing > still-active particles from a nonbonded list, if desired.) For > symmetry, > perhaps the set_particles() method should also be renamed to > add_particles(). But this can all be done later. I agree, although we should put some thought in to it. We currently have more or less one sort of container (implemented using the IMP_CONTAINER macro), plus a couple of slightly odd ones. The IMP_CONTAINER containers use Indexes to identify things
It is probably worth creating another type IMP_DYNAMIC_CONTAINER which uses ints to identify things in the container and which supports deleting. This way we have a clear distinction. I can make this change since I wrote IMP_CONTAINER.
participants (3)
-
Ben Webb
-
Daniel Russel
-
Friedrich Foerster