Subject: Re: [IMP-dev] First significant code review
From: Daniel Russel <>
Date: Wed, 12 Dec 2007 18:12:48 -0800
Cc: IMP developers' list <>
I don't like this patch, because it now requires you to use
ScoreFuncParams in some places and regular ScoreFuncs in others, and
call the generator function yourself (even from Python). We should
decide on one or the other.
The reason I introduced the split, and, I think the reason for having
both the Params and the SF itself are that some restraints simply do
use a function, where as others generate a family of functions. They
are conceptually different, and so I think, as long as there are two
different class, the interfaces should reflect that. (the other reason
is I didn't want to have to change BasicScoreFuncParams every time I
added a new subclass).
(I will quite happily write the code, unless
Daniel really wants to, for the solution.)
I'll pass :-)
One suggestion: we have a hierarchy of ScoreFunc classes, and just
the Restraint constructors create local copies to keep their own state
(we can just use the copy constructor for this).
I don't think I understand the proposal.
Two proposal (perhaps the first is the same as yours)
1) to have ScoreFunc which has a clone method and translate method
(and perhaps a scale method). Then, a compound restraint can take one,
clone it and shift the the origin to handle generating a family.
2) Have a factory per ScoreFunc which then takes a shift and a scale
parameter and creates a corresponding score function. The factory can
be easily generated with a macro.
As a side note, I propose we change the name to UnaryFunction because
it is simply a function of one variable and I can see wanting
functions of more than one. And having an abbreviation in the name is
inconsistent. Also, the names mean and stdev have to go, since they
don't make sense with statistical potentials.