[ 
https://issues.apache.org/jira/browse/MATH-595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13052117#comment-13052117
 ] 

Luc Maisonobe commented on MATH-595:
------------------------------------

Thanks for the report and for the patch, Dennis.
Your approach is really interesting. I agree with the changes you propose and 
am glad you set them up in a compatible way for existing code.

I have only really minor remarks. I noticed you have already proposed the patch 
checking the box for allowing its inclusion as Apache code, but could you also 
put the Apache header in them. Of course, we could do that ourselves, but 
having the original author do it is more satisfying (at least to myself). There 
are also some checkstyle errors like line breaks. There is also a problem 
applying the patch on the StepNormalizer.java file to the current trunk version 
(most of the patch is rejected, I don't know why).

> StepNormalizer extension with normalization mode and bounds settings
> --------------------------------------------------------------------
>
>                 Key: MATH-595
>                 URL: https://issues.apache.org/jira/browse/MATH-595
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Dennis Hendriks
>              Labels: patch
>             Fix For: 3.0
>
>         Attachments: step-normalizer-mode-bounds.patch
>
>
> While I was the StepNormalizer to get output at fixed interval points instead 
> of the points determined by the ODE integrator, I noticed the following:
> 1. It returns the first point (t0), and integer multiples of the step size 
> add to it (t0+h, t0+2h, ...). Let's call these the 'normalized points'. I 
> wanted it to return values at multiples of the step size, regardless of the 
> start vaule. This would be a different semantics of 'normalized points'.
> 2. It returns the first point (t0), and the last point only if it is a 
> 'normalized point'. I wanted it to never return the first point, and always 
> return the last point, regardless of the 'normalized points'. If one used a 
> StepHandler directly, and not a StepNormalizer, the first point would not be 
> given to the handler, and the last point would. Thus, there is a difference 
> in first/last point handling for 'normal' StepHandlers and StepNormalizer 
> with FixedStepHandler.
> Therefore I generalized the StepNormalizer:
> 1. It now has two modes (StepNormalizerMode enumeration). The first is 
> INCREMENT, which has the old semantics for 'normalized points'. The second is 
> MULTIPLES, which has the newly proposed semantics.
> 2. It is now possible to specify whether the first and last points should be 
> given to the underlying fixed step size step handler (StepNormalizerBounds 
> enumeration).
> The changes should be transparent to already existing code, as I used the old 
> semantics for existing constructors. The only change is in the calculation 
> that determines whether the next 'normalized point' is in the current step 
> interval. For forward integration, the end point was considered to be in the 
> interval, while for backward integration the end point was considered NOT to 
> be in the interval. I changed this so that the end point is in the interval 
> regardless of the direction. This is the only non-backward compatible change. 
> I personally consider the old behavior to be wrong... This change is also 
> required for the correct functioning of the new bounds settings.
> I also added a whole bunch of unit tests to test all different combinations 
> of normalization modes, bounds settings (first/last inclusion), integration 
> direction, and whether the 'normalized points' coincide/overlap the 
> first/last points. This results in 2*4*2*2=32 unit tests.
> I believe that the generalizations that I introduced make the StepNormalizer 
> more powerful, and make it useful in more scenarios.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to