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

Re: [IMP-dev] unaryfunction returns



Well, the set of all patches runs passes the tests on flute when run in valgrind (other than the dna read which fails for a virgin svn copy). And all pass (without valgrind) on my mac. I suspect the problem is changes you made to the patches I submitted, but it is also possible that the patches simply expose bugs created by the problems in attribute table that a later patch fixed. Either way, you have a working set of patches.

On Tue, Aug 26, 2008 at 7:55 PM, Ben Webb <">> wrote:
Daniel Russel wrote:
> Here is the patch that was discussed a while ago which makes
> UnaryFunction::evaluate_with_derivative return a std::pair instead of
> using one pass by reference to return. It also removes the hack in IMP.i
> to get the return value right.

I'm not sure sure why you call it a hack, since it's the regular way to
deal with reference return values in SWIG. And that code deals with any
errors the Python API (or any other scripting language) may return. But
regardless, the code (patch attached) does not work on synth:

% scons test
...
RuntimeError: Previously freed object is not valid: 0x89c1224
 File "kernel/src/Object.cpp", line 32

unit tests FAILED

       Ben
--
">                      http://salilab.org/~ben/
"It is a capital mistake to theorize before one has data."
       - Sir Arthur Conan Doyle

Index: kernel/include/IMP/UnaryFunction.h
===================================================================
--- kernel/include/IMP/UnaryFunction.h  (revision 669)
+++ kernel/include/IMP/UnaryFunction.h  (working copy)
@@ -14,9 +14,13 @@
 namespace IMP
 {

+typedef std::pair<Float, Float> FloatPair;
+
 //! Abstract single variable functor class for score functions.
 /** These functors take a single feature value, and return a corresponding
    score (and optionally also the first derivative).
+
+    \ingroup kernel
 */
 class IMPDLLEXPORT UnaryFunction : public RefCountedObject
 {
@@ -36,7 +40,7 @@
                        given feaure.
      \return Score
   */
-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const = 0;
+  virtual FloatPair evaluate_with_derivative(Float feature) const = 0;

  virtual void show(std::ostream &out=std::cout) const = 0;
 };
Index: kernel/include/IMP/internal/evaluate_distance_pair_score.h
===================================================================
--- kernel/include/IMP/internal/evaluate_distance_pair_score.h  (revision 669)
+++ kernel/include/IMP/internal/evaluate_distance_pair_score.h  (working copy)
@@ -9,6 +9,7 @@
 #define __IMP_EVALUATE_DISTANCE_PAIR_SCORE_H

 #include "../Vector3D.h"
+#include <boost/tuple/tuple.hpp>

 namespace IMP
 {
@@ -43,7 +44,7 @@
  if (da && distance >= MIN_DISTANCE) {
    Float deriv;

-    score = f->evaluate_with_derivative(shifted_distance, deriv);
+    boost::tie(score, deriv) = f->evaluate_with_derivative(shifted_distance);

    Vector3D d= delta/distance *deriv;
    d0.add_to_coordinates_derivative(d, *da);
Index: kernel/include/IMP/unary_functions/Linear.h
===================================================================
--- kernel/include/IMP/unary_functions/Linear.h (revision 669)
+++ kernel/include/IMP/unary_functions/Linear.h (working copy)
@@ -28,9 +28,8 @@
    return (feature-offset_)*slope_;
  }

-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const {
-    deriv= slope_;
-    return evaluate(feature);
+  virtual FloatPair evaluate_with_derivative(Float feature) const {
+    return std::make_pair(evaluate(feature), slope_);
  }

  void set_slope(Float f) {
Index: kernel/include/IMP/unary_functions/ClosedCubicSpline.h
===================================================================
--- kernel/include/IMP/unary_functions/ClosedCubicSpline.h      (revision 669)
+++ kernel/include/IMP/unary_functions/ClosedCubicSpline.h      (working copy)
@@ -40,12 +40,10 @@

  //! Calculate score and derivative with respect to the given feature.
  /** \param[in] feature Value of feature being tested.
-      \param[out] deriv Partial derivative of the score with respect to
-                        the feature value.
      \exception ValueException Feature is out of defined range.
      \return Score
   */
-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const;
+  virtual FloatPair evaluate_with_derivative(Float feature) const;

  void show(std::ostream &out=std::cout) const {
    out << "Closed cubic spline of " << values_.size() << " values from "
Index: kernel/include/IMP/unary_functions/WormLikeChain.h
===================================================================
--- kernel/include/IMP/unary_functions/WormLikeChain.h  (revision 669)
+++ kernel/include/IMP/unary_functions/WormLikeChain.h  (working copy)
@@ -62,10 +62,8 @@

  //! Calculate the WormLikeChain energy given the length
  /** \param[in] l Current length in angstroms
-      \param[out] deriv force in kcal/angstrom mol
-      \return Score
   */
-  virtual Float evaluate_with_derivative(Float fl, Float& deriv) const {
+  virtual FloatPair evaluate_with_derivative(Float fl) const {
    unit::Angstrom l(fl);
    if (l < unit::Angstrom(0)) l=unit::Angstrom(0);
    unit::Piconewton doubled;
@@ -81,9 +79,9 @@
    // convert from picoNewton
    unit::YoctoKilocaloriePerAngstrom du= unit::convert_J_to_Cal(doubled);

-    deriv = (du*unit::ATOMS_PER_MOL).get_value();
+    Float deriv = (du*unit::ATOMS_PER_MOL).get_value();
    //std::cout << "Which converts to " << d << std::endl;
-    return evaluate(fl);
+    return std::make_pair(evaluate(fl), deriv);
  }

  void show(std::ostream &out=std::cout) const {
Index: kernel/include/IMP/unary_functions/Harmonic.h
===================================================================
--- kernel/include/IMP/unary_functions/Harmonic.h       (revision 669)
+++ kernel/include/IMP/unary_functions/Harmonic.h       (working copy)
@@ -56,20 +56,17 @@
      \return Score
   */
  virtual Float evaluate(Float feature) const {
-    Float d;
-    return evaluate_with_derivative(feature, d);
+    return evaluate_with_derivative(feature).first;
  }

  //! Calculate harmonic score and derivative with respect to the given feature.
  /** \param[in] feature Value of feature being tested.
-      \param[out] deriv Partial derivative of the score with respect to
-                        the feature value.
      \return Score
   */
-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const {
+  virtual FloatPair evaluate_with_derivative(Float feature) const {
    Float e = (feature - mean_);
-    deriv = k_ * e;
-    return 0.5 * k_ * e * e;
+    Float deriv = k_ * e;
+    return std::make_pair(0.5 * k_ * e * e, deriv);
  }

  void show(std::ostream &out=std::cout) const {
Index: kernel/include/IMP/unary_functions/Cosine.h
===================================================================
--- kernel/include/IMP/unary_functions/Cosine.h (revision 669)
+++ kernel/include/IMP/unary_functions/Cosine.h (working copy)
@@ -43,11 +43,9 @@

  //! Calculate score and derivative with respect to the given feature.
  /** \param[in] feature Value of feature being tested.
-      \param[out] deriv Partial derivative of the score with respect to
-                        the feature value.
      \return Score
   */
-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const;
+  virtual FloatPair evaluate_with_derivative(Float feature) const;

  void show(std::ostream &out=std::cout) const {
    out << "Cosine function with force " << force_constant_
Index: kernel/include/IMP/unary_functions/HarmonicLowerBound.h
===================================================================
--- kernel/include/IMP/unary_functions/HarmonicLowerBound.h     (revision 669)
+++ kernel/include/IMP/unary_functions/HarmonicLowerBound.h     (working copy)
@@ -37,16 +37,13 @@
  //! Calculate lower-bound harmonic score and derivative for a feature.
  /** If the feature is greater than or equal to the mean, the score is zero.
      \param[in] feature Value of feature being tested.
-      \param[out] deriv Partial derivative of the score with respect to
-                        the feature value.
      \return Score
   */
-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const {
+  virtual FloatPair evaluate_with_derivative(Float feature) const {
    if (feature >= Harmonic::get_mean()) {
-      deriv = 0.0;
-      return 0.0;
+      return std::make_pair(0.0f, 0.0f);
    } else {
-      return Harmonic::evaluate_with_derivative(feature, deriv);
+      return Harmonic::evaluate_with_derivative(feature);
    }
  }

Index: kernel/include/IMP/unary_functions/OpenCubicSpline.h
===================================================================
--- kernel/include/IMP/unary_functions/OpenCubicSpline.h        (revision 669)
+++ kernel/include/IMP/unary_functions/OpenCubicSpline.h        (working copy)
@@ -25,7 +25,7 @@
      \param[in] minrange Feature value at first spline point
      \param[in] spacing  Distance (in feature space) between points
   */
-  OpenCubicSpline(const std::vector<Float> &values, Float minrange,
+  OpenCubicSpline(const Floats &values, Float minrange,
                  Float spacing);

  virtual ~OpenCubicSpline() {}
@@ -39,12 +39,10 @@

  //! Calculate score and derivative with respect to the given feature.
  /** \param[in] feature Value of feature being tested.
-      \param[out] deriv Partial derivative of the score with respect to
-                        the feature value.
      \exception ValueException Feature is out of defined range.
      \return Score
   */
-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const;
+  virtual FloatPair evaluate_with_derivative(Float feature) const;

  void show(std::ostream &out=std::cout) const {
    out << "Open cubic spline of " << values_.size() << " values from "
Index: kernel/include/IMP/unary_functions/HarmonicUpperBound.h
===================================================================
--- kernel/include/IMP/unary_functions/HarmonicUpperBound.h     (revision 669)
+++ kernel/include/IMP/unary_functions/HarmonicUpperBound.h     (working copy)
@@ -37,16 +37,13 @@
  //! Calculate upper-bound harmonic score and derivative for a feature.
  /** If the feature is less than or equal to the mean, the score is zero.
      \param[in] feature Value of feature being tested.
-      \param[out] deriv Partial derivative of the score with respect to
-                        the feature value.
      \return Score
   */
-  virtual Float evaluate_with_derivative(Float feature, Float& deriv) const {
+  virtual FloatPair evaluate_with_derivative(Float feature) const {
    if (feature <= Harmonic::get_mean()) {
-      deriv = 0.0;
-      return 0.0;
+      return std::make_pair(0.0f, 0.0f);
    } else {
-      return Harmonic::evaluate_with_derivative(feature, deriv);
+      return Harmonic::evaluate_with_derivative(feature);
    }
  }

Index: kernel/src/singleton_scores/AttributeSingletonScore.cpp
===================================================================
--- kernel/src/singleton_scores/AttributeSingletonScore.cpp     (revision 669)
+++ kernel/src/singleton_scores/AttributeSingletonScore.cpp     (working copy)
@@ -8,7 +8,7 @@
 #include "IMP/singleton_scores/AttributeSingletonScore.h"
 #include "IMP/UnaryFunction.h"
 #include "IMP/Particle.h"
-
+#include <boost/tuple/tuple.hpp>
 namespace IMP
 {

@@ -20,8 +20,8 @@
                                        DerivativeAccumulator *da) const
 {
  if (da) {
-    Float d;
-    float r= f_->evaluate_with_derivative(b->get_value(k_), d);
+    Float d, r;
+    boost::tie(d,r) = f_->evaluate_with_derivative(b->get_value(k_));
    b->add_to_derivative(k_, d, *da);
    return r;
  } else {
Index: kernel/src/singleton_scores/TunnelSingletonScore.cpp
===================================================================
--- kernel/src/singleton_scores/TunnelSingletonScore.cpp        (revision 669)
+++ kernel/src/singleton_scores/TunnelSingletonScore.cpp        (working copy)
@@ -8,7 +8,7 @@

 #include "IMP/singleton_scores/TunnelSingletonScore.h"
 #include "IMP/decorators/XYZDecorator.h"
-
+#include <boost/tuple/tuple.hpp>
 namespace IMP
 {

@@ -58,7 +58,7 @@
      // look below if changed
      Float dist= -std::min(std::min(rd, hdu), hdd) - radius;
      if (accum) {
-        score= f_->evaluate_with_derivative(dist, deriv_scalar);
+        boost::tie(score, deriv_scalar)= f_->evaluate_with_derivative(dist);
      } else {
        score= f_->evaluate(dist);
      }
Index: kernel/src/unary_functions/Cosine.cpp
===================================================================
--- kernel/src/unary_functions/Cosine.cpp       (revision 669)
+++ kernel/src/unary_functions/Cosine.cpp       (working copy)
@@ -18,11 +18,11 @@
         - force_constant_ * std::cos(periodicity_ * feature + phase_);
 }

-Float Cosine::evaluate_with_derivative(Float feature, Float& deriv) const
+FloatPair Cosine::evaluate_with_derivative(Float feature) const
 {
-  deriv = force_constant_ * periodicity_
-          * std::sin(periodicity_ * feature + phase_);
-  return evaluate(feature);
+  Float deriv = force_constant_ * periodicity_
+    * std::sin(periodicity_ * feature + phase_);
+  return std::make_pair(evaluate(feature), deriv);
 }

 }  // namespace IMP
Index: kernel/src/unary_functions/OpenCubicSpline.cpp
===================================================================
--- kernel/src/unary_functions/OpenCubicSpline.cpp      (revision 669)
+++ kernel/src/unary_functions/OpenCubicSpline.cpp      (working copy)
@@ -66,8 +66,7 @@
           * (spacing_ * spacing_) / 6.;
 }

-Float OpenCubicSpline::evaluate_with_derivative(Float feature,
-                                                Float& deriv) const
+FloatPair OpenCubicSpline::evaluate_with_derivative(Float feature) const
 {
  size_t lowbin = static_cast<size_t>((feature - minrange_) / spacing_);
  // handle the case where feature ~= maxrange
@@ -79,11 +78,11 @@
  Float a = 1. - b;
  float sixthspacing = spacing_ / 6.;

-  deriv = (values_[highbin] - values_[lowbin]) / spacing_
-          - (3. * a * a - 1.) * sixthspacing * second_derivs_[lowbin]
-          + (3. * b * b - 1.) * sixthspacing * second_derivs_[highbin];
+  Float deriv = (values_[highbin] - values_[lowbin]) / spacing_
+    - (3. * a * a - 1.) * sixthspacing * second_derivs_[lowbin]
+    + (3. * b * b - 1.) * sixthspacing * second_derivs_[highbin];

-  return evaluate(feature);
+  return std::make_pair(evaluate(feature), deriv);
 }

 }  // namespace IMP
Index: kernel/src/unary_functions/ClosedCubicSpline.cpp
===================================================================
--- kernel/src/unary_functions/ClosedCubicSpline.cpp    (revision 669)
+++ kernel/src/unary_functions/ClosedCubicSpline.cpp    (working copy)
@@ -82,8 +82,8 @@
           * (spacing_ * spacing_) / 6.;
 }

-Float ClosedCubicSpline::evaluate_with_derivative(Float feature,
-                                                  Float& deriv) const
+FloatPair
+ClosedCubicSpline::evaluate_with_derivative(Float feature) const
 {
  size_t lowbin = static_cast<size_t>((feature - minrange_) / spacing_);
  size_t highbin = lowbin + 1;
@@ -99,11 +99,11 @@
  Float a = 1. - b;
  float sixthspacing = spacing_ / 6.;

-  deriv = (values_[highbin] - values_[lowbin]) / spacing_
+  Float deriv = (values_[highbin] - values_[lowbin]) / spacing_
          - (3. * a * a - 1.) * sixthspacing * second_derivs_[lowbin]
          + (3. * b * b - 1.) * sixthspacing * second_derivs_[highbin];

-  return evaluate(feature);
+  return std::make_pair(evaluate(feature), deriv);
 }

 }  // namespace IMP
Index: kernel/src/restraints/DihedralRestraint.cpp
===================================================================
--- kernel/src/restraints/DihedralRestraint.cpp (revision 669)
+++ kernel/src/restraints/DihedralRestraint.cpp (working copy)
@@ -15,6 +15,8 @@
 #include "IMP/restraints/DihedralRestraint.h"
 #include "IMP/decorators/XYZDecorator.h"

+#include <boost/tuple/tuple.hpp>
+
 namespace IMP
 {

@@ -80,7 +82,7 @@

  if (accum) {
    Float deriv;
-    score = score_func_->evaluate_with_derivative(angle, deriv);
+    boost::tie(score, deriv) = score_func_->evaluate_with_derivative(angle);

    // method for derivative calculation from van Schaik et al.
    // J. Mol. Biol. 234, 751-762 (1993)
Index: kernel/src/triplet_scores/AngleTripletScore.cpp
===================================================================
--- kernel/src/triplet_scores/AngleTripletScore.cpp     (revision 669)
+++ kernel/src/triplet_scores/AngleTripletScore.cpp     (working copy)
@@ -8,7 +8,7 @@
 #include "IMP/triplet_scores/AngleTripletScore.h"
 #include "IMP/decorators/XYZDecorator.h"
 #include "IMP/UnaryFunction.h"
-
+#include <boost/tuple/tuple.hpp>
 namespace IMP
 {

@@ -46,7 +46,7 @@

  if (da) {
    Float deriv;
-    score = f_->evaluate_with_derivative(angle, deriv);
+    boost::tie(score, deriv) = f_->evaluate_with_derivative(angle);

    Vector3D unit_rij = rij.get_unit_vector();
    Vector3D unit_rkj = rkj.get_unit_vector();
Index: kernel/pyext/IMP.i
===================================================================
--- kernel/pyext/IMP.i  (revision 672)
+++ kernel/pyext/IMP.i  (working copy)
@@ -14,8 +14,17 @@

 /* Return derivatives from unary functions */
 %include "typemaps.i"
-%apply double &OUTPUT { IMP::Float& deriv };

+namespace IMP {
+  %typemap(out) std::pair<Float,Float> {
+     PyObject *tup= PyTuple_New(2);
+     PyTuple_SetItem(tup, 0, PyFloat_FromDouble($1.first));
+     PyTuple_SetItem(tup, 1, PyFloat_FromDouble($1.second));
+     $result= tup;
+  }
+}
+
+
 %pythoncode %{
 def check_particle(p, a):
   if (not p.get_is_active()):

_______________________________________________
IMP-dev mailing list
">
https://salilab.org/mailman/listinfo/imp-dev