Standardise crosslinking classes

[sethaxen]

IMP.pmi.restraints.crosslinking.AtomicCrossLinkMSRestraint and IMP.pmi.restraints.crosslinking.CrossLinkingMassSpectrometryRestraint share a great deal of code and do similar things but have trivial differences in implementation and variable names. It would probably be worthwhile to unify the classes by either having a single class with a flag that differentiates between the two uses or a base class that handles all common functionality and children that differentiate the two restraint uses.

[benmwebb]

Agreed. I'd be very wary of making major changes to either class though unless we have good coverage first - otherwise we don't know if we broke something.

[sethaxen]

I agree. There's a lot to untangle there too. I'll start documenting the things that confuse me as I uncover them.

For some reason, CrossLinkingMassSpectrometryRestraint adds its linear restraint self.rslin to the model and output, even though a comment indicates that this is a dummy restraint for visualization. AtomicCrossLinkMSRestraint has a linear restraint set self.rs_lin, which is never used and is not returned by get_restraint_for_rmf. In fact, get_restraint_for_rmf is not implemented at all. Instead, there's a method called create_restraints_for_rmf which creates dummy restraints, but this doesn't seem to be called anywhere in PMI.

[Pellarin]

For some reason, CrossLinkingMassSpectrometryRestraint adds its linear
restraint self.rslin to the model

Absolutely, it shouldn't be. However, the slope is set to zero, so it does
not affect the modeling.

Riccardo Pellarin, PhD

Institut Pasteur
CNRS UMR 3528
25, rue du Docteur Roux
75724 Paris Cedex 15, France
riccardo.pellarin@pasteur.fr
+33 (0)1 44 38 93 63

On Tue, Oct 11, 2016 at 8:46 PM, Seth Axen notifications@github.com wrote:

I agree. There's a lot to untangle there too. I'll start documenting the
things that confuse me as I uncover them.

For some reason, CrossLinkingMassSpectrometryRestraint adds its linear
restraint self.rslin to the model, even though a comment
https://github.com/salilab/pmi/blob/9af974/pyext/src/restraints/crosslinking.py#L92
indicates that this is a dummy restraint for visualization.
AtomicCrossLinkMSRestraint has a linear restraint set self.rs_lin, which
is never used and is not returned by get_restraint_for_rmf. In fact,
get_restraint_for_rmf is not implemented at all. Instead, there's a
method called create_restraints_for_rmf which creates dummy restraints,
but this doesn't seem to be called anywhere in PMI.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#214 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACXOkO55WcT1xaPFORMAgcNfz5d9WFR3ks5qy9l1gaJpZM4KRibe
.

[cgreenberg]

I suggest deleting PMI::restraints::AtomicCrossLinkingRestraint and just calling the core code (which references isd::AtomicCrossLinkingRestraint) using a flag from CrossLinkingMSRestraint. Other than a small change in the scoring function, the data structure should be the same. This is my fault; I added a bunch of debugging stuff to the Atomic restraint which isn't needed.