Replaced the misused clone method in step interpolators

by a dedicated copy method that handles steps finalization
automatically and hence may trigger DerivativeException.
This should simplify usage and avoid nasty errors caused
by forgotten finalization

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/math/trunk@590660 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/math/ode/AbstractStepInterpolator.java

@@ -38,7 +38,7 @@ import java.io.IOException;
  */
 
 public abstract class AbstractStepInterpolator
-  implements StepInterpolator, Cloneable {
+  implements StepInterpolator {
 
   /** previous time */
   protected double previousTime;
@@ -163,32 +163,31 @@ public abstract class AbstractStepInterpolator
   }
 
   /** Copy the instance.
+   * <p>The copied instance is guaranteed to be independent from the
+   * original one. Both can be used with different settings for
+   * interpolated time without any side effect.</p>
+   * @return a deep copy of the instance, which can be used independently.
+   * @throws DerivativeException if this call induces an automatic
+   * step finalization that throws one
+   * @see #setInterpolatedTime(double)
+   */
+   public StepInterpolator copy() throws DerivativeException {
 
-  * <p>The copied interpolator should have been finalized before the
+     // finalize the step before performing copy
-  * copy, otherwise the copy will not be able to perform correctly any
+     finalizeStep();
-  * interpolation and will throw a {@link NullPointerException}
-  * later. Since we don't want this constructor to throw the
-  * exceptions finalization may involve and since we don't want this
-  * method to modify the state of the copied interpolator,
-  * finalization is <strong>not</strong> done automatically, it
-  * remains under user control.</p>
 
-  * <p>The copy is a deep copy: its arrays are separated from the
+     // create the new independent instance
-  * original arrays of the instance.</p>
+     return doCopy();
 
-  * <p>This method has been redeclared as public instead of protected.</p>
+   }
 
-  * @return a copy of the instance.
+   /** Really copy the finalized instance.
-
+    * <p>This method is called by {@link #copy()} after the
-  */
+    * step has been finalized. It must perform a deep copy
-  public Object clone() {
+    * to have an new instance completely independent for the
-    try {
+    * original instance.
-      return super.clone();
+    */
-    } catch (CloneNotSupportedException cnse) {
+   protected abstract StepInterpolator doCopy();
-      // should never happen
-      return null;
-    }
-  }
 
   /** Shift one step forward.
    * Copy the current time into the previous time, hence preparing the

src/java/org/apache/commons/math/ode/ClassicalRungeKuttaStepInterpolator.java

@@ -68,13 +68,9 @@ class ClassicalRungeKuttaStepInterpolator
     super(interpolator);
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new ClassicalRungeKuttaStepInterpolator(this);
   }
 

src/java/org/apache/commons/math/ode/ContinuousOutputModel.java

@@ -92,11 +92,14 @@ public class ContinuousOutputModel
 
   /** Append another model at the end of the instance.
    * @param model model to add at the end of the instance
+   * @exception DerivativeException if some step interpolators from
+   * the appended model cannot be copied
    * @exception IllegalArgumentException if the model to append is not
    * compatible with the instance (dimension of the state vector,
    * propagation direction, hole between the dates)
    */
-  public void append(ContinuousOutputModel model) {
+  public void append(ContinuousOutputModel model)
+    throws DerivativeException {
 
     if (model.steps.size() == 0) {
       return;
@@ -127,8 +130,7 @@ public class ContinuousOutputModel
     }
 
     for (Iterator iter = model.steps.iterator(); iter.hasNext(); ) {
-      AbstractStepInterpolator ai = (AbstractStepInterpolator) iter.next();
+      steps.add(((AbstractStepInterpolator) iter.next()).copy());
-      steps.add(ai.clone());
     }
 
     index = steps.size() - 1;
@@ -176,8 +178,7 @@ public class ContinuousOutputModel
       forward     = interpolator.isForward();
     }
 
-    ai.finalizeStep();
+    steps.add(ai.copy());
-    steps.add(ai.clone());
 
     if (isLast) {
       finalTime = ai.getCurrentTime();

src/java/org/apache/commons/math/ode/DormandPrince54StepInterpolator.java

@@ -77,16 +77,13 @@ class DormandPrince54StepInterpolator
 
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new DormandPrince54StepInterpolator(this);
   }
 
+
   /** Reinitialize the instance
    * @param equations set of differential equations being integrated
    * @param y reference to the integrator array holding the state at

src/java/org/apache/commons/math/ode/DormandPrince853StepInterpolator.java

@@ -46,7 +46,6 @@ class DormandPrince853StepInterpolator
   public DormandPrince853StepInterpolator() {
     super();
     yDotKLast = null;
-    yTmp      = null;
     v         = null;
     vectorsInitialized = false;
   }
@@ -87,18 +86,11 @@ class DormandPrince853StepInterpolator
 
     }
 
-    // the step has been finalized, we don't need this anymore
-    yTmp = null;
-
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new DormandPrince853StepInterpolator(this);
   }
 
@@ -135,8 +127,6 @@ class DormandPrince853StepInterpolator
       yDotKLast[k] = new double[dimension];
     }
 
-    yTmp = new double[dimension];
-
     v = new double[7][];
     for (int k = 0; k < v.length; ++k) {
       v[k]  = new double[dimension];
@@ -225,7 +215,13 @@ class DormandPrince853StepInterpolator
   protected void doFinalize()
     throws DerivativeException {
 
+    if (currentState == null) {
+      // we are finalizing an uninitialized instance
+      return;
+    }
+
     double s;
+    double[] yTmp = new double[currentState.length];
 
     // k14
     for (int j = 0; j < currentState.length; ++j) {
@@ -311,9 +307,6 @@ class DormandPrince853StepInterpolator
   /** Last evaluations. */
   private double[][] yDotKLast;
 
-  /** Temporary state vector. */
-  private double[] yTmp;
-
   /** Vectors for interpolation. */
   private double[][] v;
 
@@ -407,6 +400,6 @@ class DormandPrince853StepInterpolator
 
   };
 
-  private static final long serialVersionUID = 4165537490327432186L;
+  private static final long serialVersionUID = 7152276390558450974L;
 
 }

src/java/org/apache/commons/math/ode/DummyStepInterpolator.java

@@ -61,6 +61,21 @@ public class DummyStepInterpolator
     super(y, forward);
   }
 
+  /** Copy constructor.
+   * @param interpolator interpolator to copy from. The copy is a deep
+   * copy: its arrays are separated from the original arrays of the
+   * instance
+   */
+  public DummyStepInterpolator(DummyStepInterpolator interpolator) {
+    super(interpolator);
+  }
+
+  /** Really copy the finalized instance.
+   */
+  protected StepInterpolator doCopy() {
+    return new DummyStepInterpolator(this);
+  }
+
   /** Compute the state at the interpolated time.
    * In this class, this method does nothing: the interpolated state
    * is always the state at the end of the current step.

src/java/org/apache/commons/math/ode/EulerStepInterpolator.java

@@ -61,16 +61,13 @@ class EulerStepInterpolator
     super(interpolator);
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new EulerStepInterpolator(this);
   }
 
+
   /** Compute the state at the interpolated time.
    * This is the main processing method that should be implemented by
    * the derived classes to perform the interpolation.

src/java/org/apache/commons/math/ode/GillStepInterpolator.java

@@ -66,16 +66,13 @@ class GillStepInterpolator
     super(interpolator);
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new GillStepInterpolator(this);
   }
 
+
   /** Compute the state at the interpolated time.
    * This is the main processing method that should be implemented by
    * the derived classes to perform the interpolation.

src/java/org/apache/commons/math/ode/GraggBulirschStoerStepInterpolator.java

@@ -219,16 +219,13 @@ class GraggBulirschStoerStepInterpolator
 
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new GraggBulirschStoerStepInterpolator(this);
   }
 
+
   /** Compute the interpolation coefficients for dense output.
    * @param mu degree of the interpolation polynom
    * @param h current step

src/java/org/apache/commons/math/ode/HighamHall54StepInterpolator.java

@@ -52,16 +52,13 @@ class HighamHall54StepInterpolator
     super(interpolator);
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new HighamHall54StepInterpolator(this);
   }
 
+
   /** Compute the state at the interpolated time.
    * @param theta normalized interpolation abscissa within the step
    * (theta is zero at the previous time step and one at the current time step)

src/java/org/apache/commons/math/ode/MidpointStepInterpolator.java

@@ -63,16 +63,13 @@ class MidpointStepInterpolator
     super(interpolator);
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new MidpointStepInterpolator(this);
   }
 
+
   /** Compute the state at the interpolated time.
    * This is the main processing method that should be implemented by
    * the derived classes to perform the interpolation.

src/java/org/apache/commons/math/ode/RungeKuttaFehlbergIntegrator.java

@@ -179,7 +179,7 @@ public abstract class RungeKuttaFehlbergIntegrator
     // set up an interpolator sharing the integrator arrays
     AbstractStepInterpolator interpolator;
     if (handler.requiresDenseOutput() || (! switchesHandler.isEmpty())) {
-      RungeKuttaStepInterpolator rki = (RungeKuttaStepInterpolator) prototype.clone();
+      RungeKuttaStepInterpolator rki = (RungeKuttaStepInterpolator) prototype.copy();
       rki.reinitialize(equations, yTmp, yDotK, forward);
       interpolator = rki;
     } else {

src/java/org/apache/commons/math/ode/RungeKuttaIntegrator.java

@@ -172,7 +172,7 @@ public abstract class RungeKuttaIntegrator
     // set up an interpolator sharing the integrator arrays
     AbstractStepInterpolator interpolator;
     if (handler.requiresDenseOutput() || (! switchesHandler.isEmpty())) {
-      RungeKuttaStepInterpolator rki = (RungeKuttaStepInterpolator) prototype.clone();
+      RungeKuttaStepInterpolator rki = (RungeKuttaStepInterpolator) prototype.copy();
       rki.reinitialize(equations, yTmp, yDotK, forward);
       interpolator = rki;
     } else {

src/java/org/apache/commons/math/ode/StepInterpolator.java

@@ -62,12 +62,14 @@ public interface StepInterpolator
     
   /**
    * Set the time of the interpolated point.
-   * <p>Setting the time outside of the current step is now allowed
+   * <p>Setting the time outside of the current step is now allowed, but
-   * (it was not allowed up to version 5.4 of Mantissa), but should be
+   * should be used with care since the accuracy of the interpolator will
-   * used with care since the accuracy of the interpolator will
    * probably be very poor far from this step. This allowance has been
    * added to simplify implementation of search algorithms near the
    * step endpoints.</p>
+   * <p>Setting the time changes the instance internal state. If a
+   * specific state must be preserved, a copy of the instance must be
+   * created using {@link #copy()}.</p>
    * @param time time of the interpolated point
    * @throws DerivativeException if this call induces an automatic
    * step finalization that throws one
@@ -92,4 +94,15 @@ public interface StepInterpolator
    */
   public boolean isForward();
 
+  /** Copy the instance.
+   * <p>The copied instance is guaranteed to be independent from the
+   * original one. Both can be used with different settings for
+   * interpolated time without any side effect.</p>
+   * @return a deep copy of the instance, which can be used independently.
+   * @throws DerivativeException if this call induces an automatic
+   * step finalization that throws one
+   * @see #setInterpolatedTime(double)
+   */
+   public StepInterpolator copy() throws DerivativeException;
+
 }

src/java/org/apache/commons/math/ode/ThreeEighthesStepInterpolator.java

@@ -68,16 +68,13 @@ class ThreeEighthesStepInterpolator
     super(interpolator);
   }
 
-  /**
+  /** Really copy the finalized instance.
-   * Clone the instance.
-   * the copy is a deep copy: its arrays are separated from the
-   * original arrays of the instance
-   * @return a copy of the instance
    */
-  public Object clone() {
+  protected StepInterpolator doCopy() {
     return new ThreeEighthesStepInterpolator(this);
   }
 
+
   /** Compute the state at the interpolated time.
    * This is the main processing method that should be implemented by
    * the derived classes to perform the interpolation.

src/test/org/apache/commons/math/ode/DormandPrince54StepInterpolatorTest.java

@@ -85,6 +85,50 @@ public class DormandPrince54StepInterpolatorTest
 
   }
 
+  public void testClone()
+    throws DerivativeException, IntegratorException {
+      TestProblem3 pb = new TestProblem3(0.9);
+      double minStep = 0;
+      double maxStep = pb.getFinalTime() - pb.getInitialTime();
+      double scalAbsoluteTolerance = 1.0e-8;
+      double scalRelativeTolerance = scalAbsoluteTolerance;
+      DormandPrince54Integrator integ = new DormandPrince54Integrator(minStep, maxStep,
+                                                                      scalAbsoluteTolerance,
+                                                                      scalRelativeTolerance);
+      integ.setStepHandler(new StepHandler() {
+          public void handleStep(StepInterpolator interpolator, boolean isLast)
+          throws DerivativeException {
+              StepInterpolator cloned = interpolator.copy();
+              double tA = cloned.getPreviousTime();
+              double tB = cloned.getCurrentTime();
+              double halfStep = Math.abs(tB - tA) / 2;
+              assertEquals(interpolator.getPreviousTime(), tA, 1.0e-12);
+              assertEquals(interpolator.getCurrentTime(), tB, 1.0e-12);
+              for (int i = 0; i < 10; ++i) {
+                  double t = (i * tB + (9 - i) * tA) / 9;
+                  interpolator.setInterpolatedTime(t);
+                  assertTrue(Math.abs(cloned.getInterpolatedTime() - t) > (halfStep / 10));
+                  cloned.setInterpolatedTime(t);
+                  assertEquals(t, cloned.getInterpolatedTime(), 1.0e-12);
+                  double[] referenceState = interpolator.getInterpolatedState();
+                  double[] cloneState     = cloned.getInterpolatedState();
+                  for (int j = 0; j < referenceState.length; ++j) {
+                      assertEquals(referenceState[j], cloneState[j], 1.0e-12);
+                  }
+              }
+          }
+          public boolean requiresDenseOutput() {
+              return true;
+          }
+          public void reset() {
+          }
+      });
+      integ.integrate(pb,
+              pb.getInitialTime(), pb.getInitialState(),
+              pb.getFinalTime(), new double[pb.getDimension()]);
+
+  }
+
   public static Test suite() {
     return new TestSuite(DormandPrince54StepInterpolatorTest.class);
   }

src/test/org/apache/commons/math/ode/DormandPrince853StepInterpolatorTest.java

@@ -85,6 +85,50 @@ public class DormandPrince853StepInterpolatorTest
 
   }
 
+  public void testClone()
+  throws DerivativeException, IntegratorException {
+    TestProblem3 pb = new TestProblem3(0.9);
+    double minStep = 0;
+    double maxStep = pb.getFinalTime() - pb.getInitialTime();
+    double scalAbsoluteTolerance = 1.0e-8;
+    double scalRelativeTolerance = scalAbsoluteTolerance;
+    DormandPrince853Integrator integ = new DormandPrince853Integrator(minStep, maxStep,
+                                                                      scalAbsoluteTolerance,
+                                                                      scalRelativeTolerance);
+    integ.setStepHandler(new StepHandler() {
+        public void handleStep(StepInterpolator interpolator, boolean isLast)
+        throws DerivativeException {
+            StepInterpolator cloned = interpolator.copy();
+            double tA = cloned.getPreviousTime();
+            double tB = cloned.getCurrentTime();
+            double halfStep = Math.abs(tB - tA) / 2;
+            assertEquals(interpolator.getPreviousTime(), tA, 1.0e-12);
+            assertEquals(interpolator.getCurrentTime(), tB, 1.0e-12);
+            for (int i = 0; i < 10; ++i) {
+                double t = (i * tB + (9 - i) * tA) / 9;
+                interpolator.setInterpolatedTime(t);
+                assertTrue(Math.abs(cloned.getInterpolatedTime() - t) > (halfStep / 10));
+                cloned.setInterpolatedTime(t);
+                assertEquals(t, cloned.getInterpolatedTime(), 1.0e-12);
+                double[] referenceState = interpolator.getInterpolatedState();
+                double[] cloneState     = cloned.getInterpolatedState();
+                for (int j = 0; j < referenceState.length; ++j) {
+                    assertEquals(referenceState[j], cloneState[j], 1.0e-12);
+                }
+            }
+        }
+        public boolean requiresDenseOutput() {
+            return true;
+        }
+        public void reset() {
+        }
+    });
+    integ.integrate(pb,
+            pb.getInitialTime(), pb.getInitialState(),
+            pb.getFinalTime(), new double[pb.getDimension()]);
+
+  }
+
   public static Test suite() {
     return new TestSuite(DormandPrince853StepInterpolatorTest.class);
   }

src/test/org/apache/commons/math/ode/GraggBulirschStoerStepInterpolatorTest.java

@@ -86,6 +86,50 @@ public class GraggBulirschStoerStepInterpolatorTest
 
   }
 
+  public void testClone()
+  throws DerivativeException, IntegratorException {
+    TestProblem3 pb = new TestProblem3(0.9);
+    double minStep = 0;
+    double maxStep = pb.getFinalTime() - pb.getInitialTime();
+    double scalAbsoluteTolerance = 1.0e-8;
+    double scalRelativeTolerance = scalAbsoluteTolerance;
+    GraggBulirschStoerIntegrator integ = new GraggBulirschStoerIntegrator(minStep, maxStep,
+                                                                          scalAbsoluteTolerance,
+                                                                          scalRelativeTolerance);
+    integ.setStepHandler(new StepHandler() {
+        public void handleStep(StepInterpolator interpolator, boolean isLast)
+        throws DerivativeException {
+            StepInterpolator cloned = interpolator.copy();
+            double tA = cloned.getPreviousTime();
+            double tB = cloned.getCurrentTime();
+            double halfStep = Math.abs(tB - tA) / 2;
+            assertEquals(interpolator.getPreviousTime(), tA, 1.0e-12);
+            assertEquals(interpolator.getCurrentTime(), tB, 1.0e-12);
+            for (int i = 0; i < 10; ++i) {
+                double t = (i * tB + (9 - i) * tA) / 9;
+                interpolator.setInterpolatedTime(t);
+                assertTrue(Math.abs(cloned.getInterpolatedTime() - t) > (halfStep / 10));
+                cloned.setInterpolatedTime(t);
+                assertEquals(t, cloned.getInterpolatedTime(), 1.0e-12);
+                double[] referenceState = interpolator.getInterpolatedState();
+                double[] cloneState     = cloned.getInterpolatedState();
+                for (int j = 0; j < referenceState.length; ++j) {
+                    assertEquals(referenceState[j], cloneState[j], 1.0e-12);
+                }
+            }
+        }
+        public boolean requiresDenseOutput() {
+            return true;
+        }
+        public void reset() {
+        }
+    });
+    integ.integrate(pb,
+            pb.getInitialTime(), pb.getInitialState(),
+            pb.getFinalTime(), new double[pb.getDimension()]);
+
+  }
+
   public static Test suite() {
     return new TestSuite(GraggBulirschStoerStepInterpolatorTest.class);
   }

src/test/org/apache/commons/math/ode/HighamHall54StepInterpolatorTest.java

@@ -85,6 +85,50 @@ public class HighamHall54StepInterpolatorTest
 
   }
 
+  public void testClone()
+  throws DerivativeException, IntegratorException {
+    TestProblem3 pb = new TestProblem3(0.9);
+    double minStep = 0;
+    double maxStep = pb.getFinalTime() - pb.getInitialTime();
+    double scalAbsoluteTolerance = 1.0e-8;
+    double scalRelativeTolerance = scalAbsoluteTolerance;
+    HighamHall54Integrator integ = new HighamHall54Integrator(minStep, maxStep,
+                                                              scalAbsoluteTolerance,
+                                                              scalRelativeTolerance);
+    integ.setStepHandler(new StepHandler() {
+        public void handleStep(StepInterpolator interpolator, boolean isLast)
+        throws DerivativeException {
+            StepInterpolator cloned = interpolator.copy();
+            double tA = cloned.getPreviousTime();
+            double tB = cloned.getCurrentTime();
+            double halfStep = Math.abs(tB - tA) / 2;
+            assertEquals(interpolator.getPreviousTime(), tA, 1.0e-12);
+            assertEquals(interpolator.getCurrentTime(), tB, 1.0e-12);
+            for (int i = 0; i < 10; ++i) {
+                double t = (i * tB + (9 - i) * tA) / 9;
+                interpolator.setInterpolatedTime(t);
+                assertTrue(Math.abs(cloned.getInterpolatedTime() - t) > (halfStep / 10));
+                cloned.setInterpolatedTime(t);
+                assertEquals(t, cloned.getInterpolatedTime(), 1.0e-12);
+                double[] referenceState = interpolator.getInterpolatedState();
+                double[] cloneState     = cloned.getInterpolatedState();
+                for (int j = 0; j < referenceState.length; ++j) {
+                    assertEquals(referenceState[j], cloneState[j], 1.0e-12);
+                }
+            }
+        }
+        public boolean requiresDenseOutput() {
+            return true;
+        }
+        public void reset() {
+        }
+    });
+    integ.integrate(pb,
+            pb.getInitialTime(), pb.getInitialState(),
+            pb.getFinalTime(), new double[pb.getDimension()]);
+
+  }
+
   public static Test suite() {
     return new TestSuite(HighamHall54StepInterpolatorTest.class);
   }