You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Luc Maisonobe <lu...@spaceroots.org> on 2013/09/16 18:04:21 UTC
[math] fluent API for the ODE integrators
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.
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?
Luc
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] fluent API for the ODE integrators
Posted by Gilles <gi...@harfang.homelinux.org>.
On Mon, 16 Sep 2013 18:04:21 +0200, Luc Maisonobe wrote:
> [...]
>
> What do you think?
I've never used this package, nor did I read much of its code;
but I trust his main developer to know how to bring improvements
into it.
Even if this is not likely the end of history, for this package as
well as for the others that have been worked on, and improved, over
the last years.
Best,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] fluent API for the ODE integrators
Posted by Phil Steitz <ph...@gmail.com>.
On 9/17/13 2:07 PM, Thomas Neidhart wrote:
> On 09/17/2013 04:22 PM, Evan Ward wrote:
>> 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.
This sounds reasonable to me. Two things you might want to think about:
0) It might make sense to actually split config / state data into
three parts instead of two:
a) problem instance definition (the coefficients, for example go here)
b) engine configuration (step size, other engine config)
c) algorithm state (what you call integrator state)
1) Be careful with mutable == state. Item c) above might logically
include some runtime immutable stuff.
>>>
>>> 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. :)
> I like the state idea too, a pattern I use a lot myself.
>
>>> 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.
> I agree with Evan, your proposed solution may work technically to
> preserve binary compatibility, but will break client code anyway.
>
> Such a change would only be acceptable for a major release imho. Otoh we
> have already quite some content for a 3.3 release. Let's try to release
> this in, let's say 1 month, and then work on a 4.0 release, cleaning up
> all the API drawbacks/flaws we have accumulated in the last years.
>
> As you already said, several users raised their concern about all the
> deprecated stuff in commons-math, I think we should not create more
> headaches (especially for ourselves ;-).
Big +1 here. Lets get 3.3 out and get this right in 4.0 without
trying to preserve compat.
Phil
>
> Thomas
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] fluent API for the ODE integrators
Posted by Thomas Neidhart <th...@gmail.com>.
On 09/17/2013 04:22 PM, Evan Ward wrote:
> 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. :)
I like the state idea too, a pattern I use a lot myself.
>> 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.
I agree with Evan, your proposed solution may work technically to
preserve binary compatibility, but will break client code anyway.
Such a change would only be acceptable for a major release imho. Otoh we
have already quite some content for a 3.3 release. Let's try to release
this in, let's say 1 month, and then work on a 4.0 release, cleaning up
all the API drawbacks/flaws we have accumulated in the last years.
As you already said, several users raised their concern about all the
deprecated stuff in commons-math, I think we should not create more
headaches (especially for ourselves ;-).
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] fluent API for the ODE integrators
Posted by Evan Ward <ev...@nrl.navy.mil>.
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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org