[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [IMP-dev] First significant code review




On Dec 14, 2007, at 12:27 AM, Ben Webb wrote:

Yes, this is true. For example, a DistanceRestraint uses a 'raw'
ScoreFunc, while an ExclusionVolumeRestraint uses a ScoreFunc generator (to generate a separate ScoreFunc for each DistanceRestraint). But don't you think the average user will be confused by this distinction? I have
no problem with making it internally.
That I can't really say. It makes sense to me. But everyone things my mind works in strange ways. In addition, it needs to be able to support statistical potentials, which is a problem if you want to stick with the names mean and stddev.

Creating and destroying distance restraints is, at least in my code, a relatively common operation (in one of the inner loops) and so I would like it to be as cheap as possible. On that note, we should make the x_,y_,z_ static.

As a side note, I propose we change the name to UnaryFunction

That's a rather vague name IMHO, and doesn't say anything about its use as a scoring function... you may as well just call it MathForm (although
the comments in ScoreFunc suggest that's what it was called originally
anyway). Why don't you like MathForm? We could have UnaryMathForm if you
prefer. ;)
UnaryMathForm is the same. I don't really know what a math form is a priori. UnaryScoreFunction? A bit long, but quite explicit.

I guess you're
arguing for a different base class for these functions, since the
operator() call signature would be different? (std::vector<Float> rather
than Float, I guess.)
I would say we have BinaryX, which takes two and perhaps NaryX which takes a vector, yes. Where X is ScoreFunction or MathForm.

Also, the names mean and stdev have to go, since they don't make sense
with statistical potentials.

That's certainly true for statistical potentials, but most of the
restraints that currently generate DistanceRestraints (e.g.
ExclusionVolume) explicitly build restraints for a minimum or mean
distance. So there'd need to at least be a ScoreFunc class that
supported mean/stdev, that we can derive from, yes?
Well, the setting the mean at least is just translation which is an operation that can be applied to anything. For the current functions stddev just involves a scaling (and isn't actually stddev anyway).