Hi Luc,
On Mon 16 Sep 2013 12:04:21 PM EDT, Luc Maisonobe wrote:
>
> Hello,
>
> I have started (at last) to work on the fluent API for ODE integrators.
> It seems to be well suited for the needs, and would allow a better
> separation of the integrator "engine" and the integrator "state". The
> engine will hold all the coefficients for the linear combinations within
> the integration steps and some general settings like min and max step
> sizes, or absolute and relative error thresholds for adaptive stepsize
> integrators. These data don't change during one integration run, and
> user may want to share them among several successive or parallel
> integration runs. The state will hold more transient values for numerous
> internal variables, like step size, start of step, pending discrete
> events, number of evaluations ... These data are updated continuously
> during an integration run and can certainly not be shared.
>
> Users will only see and configure the engine. Each time they call the
> integrate method on an engine, it will create on the fly a state
> dedicated for this call and everything within the integrator will
> reference this state instance. Users may call the integrate method
> several time to run them in parallel in different threads. As each call
> will have its own state instance, and as the engine itself which is
> shared by all states is immutable, everything should work without problem.
>
> These were the good news.
>
> The bad news are that it is difficult to separate the state from the
> engine as I have mixed everything right from the top level interface,
> which in this case is ODEIntegrator. The interface defines methods like
> getCurrentStepStart (which should refer to state) but also
> addStepHandler/addEventHandler (which should refer to engine). The next
> level interface (FirstOrederIntegrator) adds an integrate method which
> should belong to engine. The AbstractIntegrator class implements many of
> these methods and adds new protected methods which can be used by
> specific integrators.
>
> My current design is to have an internal IntegratorState class defined
> within AbstractIntegrator and providing all the state related fields and
> methods. For now, I don't think there is no need here for interfaces or
> abstract classes, the State is a simple thing. I may introduce a small
> hierarchy later on when I delve into the specific integrators, though.
> The integrate method in AbstractIntegrator would simply be something like:
>
> public void integrate(ExpandableStatefulODE equations, double t) {
> integrate(new IntegratorState(this), equations, t);
> }
>
> protected abstract integrate(IntegratorState state,
> ExpandableStatefulODE equations,
> double t);
>
> The protected integrate method with the state argument would be the same
> as the current integrate methods, just storing and updating variables in
> the State instance rather than in the integrator instance as done today.
>
> This should handle the engine part as desired, with a state instance
> created on the fly for each integration run.
+1 I like it. I think it will make the integrators easier to use and
more useful. :)
>
> Most of the IntegratorState class would be copied from the current
> AbstractIntegrator class, where many state related stuff is managed now.
> The problem is that many of these fields are protected fields
> (stepStart, stepSize, isLastStep, ...) and many methods are protected or
> even public methods (initIntegration, computeDerivatives). Of course,
> there are also the public methods inherited from the top level interface
> like getCurrentStepStart. I cannot simply remove the public method from
> the interface, nor remove the protected fields and methods from the
> AbstractIntegrator class, this would break compatibility...
>
> Most of these methods were never intended to be in the public API of the
> library, but due to Java limitations with cross packages visibility,
> some were put public. In the same spirit, the protected methods are not
> expected to be in the component API as it is not expected that people
> will provide their own implementations of ODE integrators: all the
> implementations and all the callers of the methods are within [math].
>
> So how can I handle these methods and fields? At least, I will deprecate
> them. I will also change all our own implementations to use the new
> IntegratorState class to store and update all these transient variables,
> so from [math] point of view, the deprecated protected methods and
> fields will not be used anymore. What should I do with the remnants and
> unused fields? As long as we are in the 3.X series, they should remain
> here, but how should they behave? Lets take one example: there is one
> method, intended to be called only by the specific integrators
> implementations:
>
> protected double acceptStep(AbstractStepInterpolator,
> double[], double[], double);
>
> It is called by AdamsBashforthIntegrator, AdamsMoultonIntegrator,
> EmbeddedRungeKuttaIntegrator, GraggBulirschStoerIntegrator and
> RungeKuttaIntegrator with lines like:
>
> stepStart = acceptStep(interpolator, y1, yDot1, t);
>
> All these calls will be changed to something like:
>
> state.setStepStart(state.acceptStep(interpolator, y1, yDot1, t));
>
> This means only the acceptStep of the new IntegratorState class will be
> called (and it will almost be a copy of the former method, moved to a
> new class). What should the acceptStep method in the AbstractIntegrator
> class do? It will not be called anymore by ourselves, and will not serve
> any purpose anymore. Still I cannot remove it.
>
> I certainly do not want to duplicate the entire package. We have seen
> users are lost when we do this and get questions on the list due to the
> confusion and the fact people either don't see the new package or see
> several packages and don't know which one to use.
>
> I have found one solution, but it is really ugly. I could temporarily
> add the IntegratorState instance as a field in AbstractIntegrator and
> have the integrator delegate to the State, as follows:
>
> public abstract class AbstractIntegrator
> implements FirstOrderIntegrator {
>
> /** Current step start time.
> * @deprecated as of 3.3 replaced by IntegratorState#stepStart
> */
> protected double stepStart;
>
> /** Last state instance.
> * Corresponds to last call to integrate().
> * @since 3.3
> * @deprecated temporary hack introduced in 3.3,
> to be removed in 4.0
> */
> @Deprecated
> private IntegratorState lastAllocatedState;
>
> /** Container for transient integration data.
> * @since 3.3
> */
> protected class IntegratorState {
>
> /** Current step start time. */
> private double stepStart;
>
> /** Get current step start time.
> * @return current step start time
> */
> public double getCurrentStepStart() {
> return stepStart;
> }
>
> /** Set current step start time.
> * @param current step start time
> */
> public void setCurrentStepStart(double stepStart) {
> this.stepStart = stepStart;
> // TODO: to be removed for 4.0
> AbstractIntegrator.this.stepStart = stepStart;
> }
>
> }
>
> /** {@inheritDoc}
> * @deprecated as of 3.3 replaced by
> * IntegratorState#getCurrentStepStart()
> */
> public double getCurrentStepStart() {
> return lastAllocatedState.getCurrentStepStart();
> }
>
> public void integrate(ExpandableStatefulODE equations, double t) {
>
> final IntegratorState localState = new IntegratorState(this);
>
> // TODO: to be removed for 4.0
> lastAllocatedState = localState;
>
> integrate(localState, equations, t);
>
> }
>
> protected abstract integrate(IntegratorState state,
> ExpandableStatefulODE equations,
> double t);
>
> }
>
> Here, we preserve the stepStart protected field, it will be updated
> automatically by the new code that uses IntegratorState.setStepStart,
> users may read it from dereived classes. We also preserve the
> getCurrentStepStart method, which delegates to the last allocated state.
> What will *not* work however is when people who would have developed
> their own integrator would try to update the field from their derived
> class. They will be able to do it, but won't have the desired effect
> since [math] doesn't use it anymore. Yes, I know, it was bad design
> right from the beginning to have a protected field, my bad.
>
> Of course, this would mean that each new integrate run would override
> the lastAllocatedState instance, but this is similar to the current
> behaviour. If people attempt to call integrate several time in parallel,
> lots of contingency problems will appear right now, so we don't
> introduce a new problem here. The field would be removed from
> AbstractIntegrator in 4.0 when the methods will be removed and all
> access will be limited at IntegratorState level.
>
> What do you think?
IMHO just make the change for 4.0. I think it would be easier for the
users to only have to learn the intricacies of one new API. Especially
since we know the deprecated 3.X API will be confusing and not
completely backward compatible. Just my $0.02.
Best Regards,
Evan
>
> Luc
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
