Davide Baà wrote:
> here is the code for the Morse potential.
There are a few problems that should be resolved with this code before
we can commit it. Of course, I'd be happiest if you took care of those,
wrote a nice patch for them, and dropped it in the IMP patches
directory, but if people really want to use this potential I can
probably help out with some of this.
> class Morse : public Harmonic
I don't think Morse should be a subclass of Harmonic, unless you really
think Morse can be a drop-in replacement for Harmonic. If you just want
code reuse, you can pull that code out into an internal header or
something. But in this case it looks like you are just getting the mean
and k, so you could replace get_mean() with just "mean_".
> virtual Float evaluate(Float feature, Float threshold, Float alpha,
> Float De) const {
I'm not sure how this could work as intended since the signature is
different to the base class (you are going to overload rather than
override). Your constructor should store threshold, alpha, De in the
object and evaluate should read them from there, not from its own
parameters.
> virtual FloatPair evaluate_with_derivative(Float feature) const {
You don't seem to be calculating the first derivatives for the Morse
potential at all... that's going to upset any optimizer that relies on
these.
Finally, we'll need some test cases. You can use one of those in
modules/core/test/unary_functions as an example.
Ben
--
http://salilab.org/~ben/
"It is a capital mistake to theorize before one has data."
- Sir Arthur Conan Doyle