-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Symmetry constraint: add some checks #126
Comments
What about movers that include both clones and By nested symmetry, do you mean that you don't want the reference to itself be a symmetry copy? That shouldn't be a problem - constraints in IMP should already do the right thing and work in the right order. |
Let's call them references and clones. We were worried that if you moved the references+the clones together, like in a SuperRigidBody, the constraint wouldn't work properly because the frame has changed. E.g. the two spokes of the NPC - if you move them all as a unit, like rotating them in/out of the NPC plane, the symmetry will no longer be about the center of the NPC, right? Now if you try to move the clones and some other molecule, I'd rather raise an error than secretly modify the mover to make it work. |
If you move the reference and clone, all that will happen will be that you wasted time moving the clone. The constraint overwrites the coordinates of the clone with those of the (transformed) reference. The symmetry transformation acts only on the reference and completely ignores the coordinates of the clone. So in a sense the transformation acts on a fixed coordinate system - in the case of the NPC it'll always do a 45 degree rotation in the plane about the origin, regardless of where the reference is. |
BTW, I think raising an error is the best route to go in for mixed reference+clone moves, or even just a warning. But definitely remove any movers that only move the clones, because that is entirely wasted work. |
OK, to follow up on something you said earlier: What will the user tries to use a symmetry copy as a reference for another molecule? Say creates a dimer of dimers: dof.create_symmetry(m1,m2,trans1)
dof.create_symmetry([m1,m2],[m3,m4], trans2) Obviously this is the same as setting up m2, m3, and m4 to all be symmetry copies of m1. Will IMP handle this any less efficiently/predictably? If so we should probably disallow it. How about if the user does this: dof.create_symmetry(m1,m2,trans1)
dof.create_symmetry(m2,m3,trans2) Same question. Is that gonna work? Or should we say that you can't use a clone as a reference? |
Both cases should be absolutely fine and shouldn't run any more slowly. IMP tracks the inputs and outputs of constraints - in the second case, it sees that the second constraint has an input (m2) that is an output of the first constraint, so it makes sure to satisfy the first constraint first. If you do something dumb like
then obviously that won't work, but no need to check for that in PMI - the dependency graph generator should catch that. |
-- disable any movers that include clones
-- prevent nested symmetry (for now). We'd like to do this later but we have to make sure you propagate the constraint correctly (like in nested rigid bodies)
The text was updated successfully, but these errors were encountered: