RE: [Orekit Developers] Problem with angle event detectors in OREKIT

[Tanguy Yannick <Yannick.Tanguy@cnes.fr>]

[...]
>>
>> We propose to solve the problem by extending the Events Detection function:
>> allow to specify the type of the zeros of the g function to be  
>> detected (at low level:  
>> org.apache.commons.math3.ode.events.EventState.evaluateStep):
>> - all zeros
>> - ascending zeros only
>> - descending zeros only
>
>This sounds really interesting.
>
>>
>> The advantages are:
>> - all angular detectors work well with a simple g function  
>> (sin(alpha - alphaRef))  without problems of modulo 2 Pi  
>> discontinuity management
>> - for some specific events logs computations the computation time  
>> will be significantly reduced
>
>Note that as the computation needs to compare previous step and  
>current step to correctly bracket the roots, we still need to switch  
>some internal variables around the ignored zeros. Of course, as we  
>know user is not interested in this zero, we simply updates these  
>state preserving data and do not attempt to refine a zero that will be  
>ignored later. So there will be some savings, but we have to make sure  
>we do not forget some important parts of the algorithm.
>
>> - the detector will fit better to user needs (no need to "filter"  
>> the useful detections).
>>
>> This solution needs to modify a few classes :
>> OREKIT :
>> - EventDetector : add a new int getSlopeSelection to know if we are  
>> interested in descending, ascending g function, or both
>
>I'm not sure I understand the purpose of this method. Do you intend it  
>to be overridden by users on all detectors or do you intend it to be  
>implemented directly at AbstractDetector level based on a fixed  
>boolean field provided at construction ? I would prefern the second  
>case, since the type of events user is interested in seems to me to be  
>of the same level as the maxCheck and threshold parameters.

This method (getSlopeSelection) is used by EventState in the acceptStep method, to know if the event has to be checked.
The AbstractDetector class is modified to add a new constructor (if the other constructors are used, both decreasing and increasing g functions are detected).
The side effect is that there are some EventDetector implementations to modify.

>> - EventState : depending on the slope selection, detect or not the  
>> event (mapping with Commons Math solvers)
>> - All classes implementing EventDetector
>
>If the type of events desired by users is set as a constructor  
>parameter and if we preserve both the current constructors without  
>this setting (which would default to select all events) and add a new  
>constructor, then modifying the current EventDetectors would be more  
>backawrd compatible: we would mainly add a new setting but would not  
>force people to use it. Existing code would still work without a change.

Fine, that's exactly what we did.

>>
>> Commons Math :
>> - EventHandler : new slopeSelection parameter (see EventDetector)
>> - EventState : uses the new parameter to decide to call the solver or not
>
>Please ask on the Apache Commons Math lists for this change.
>

Ok, I will create a ticket. This modification needs to be done in both Commons Math and OREKIT. 
I guess OREKIT 6.0 will be based on Commons Math 3.0, so theses features may wait a while. 
As this modification does not affect backward compatibility, I think it can be integrated later, and we will keep our local modifications in our modified versions.

>>
>> Do yo think this is a useful feature for OREKIT ? If it make sense,  
>> we can create features in both Commons Math and OREKIT and  
>> contribute our source code.
>
>Concerning Orekit, I think it is useful.

Ok, we will join source code and some unitary tests.

Yannick

>best regards,
>Luc

>>
>> Best regards,
>>
>> Yannick