I’ve thought up a new feature that would be nice to have in event detection and I would like to gauge your interest and hear your thougts. Currently event detectors cannot reliably detect events based on total time in a particular state. For example, triggering an event after 10 minutes of time in contact with the ground does not work reliably in the current implementation. Similar functions would be observation time, eclipse time, time in view of a GPS satellite, etc. All these functions are integrals of a step function where the step location is determined by another event detector. A simple implementation of the example is:
class Counter implements EventHandler<ElevationDetector>, GFunction {
/** Trigger an event when time in contact crosses this value, in s. */
private final double limit;
private double total = 0;
private AbsoluteDate start = null;
public void init(SpacecraftState s, AbsoluteDate t) {
start = g(s) > 0 ? s.getDate() : null;
total = 0;
}
public Action eventOccurred(SpacecraftState s, EventDetector d, boolean increasing) {
if (increasing) {
start = s.getDate();
else {
total += s.getDate().durationFrom(start);
start = null;
}
return Action.CONTINUE;
}
// defines g function of a new Functional detector
public double apply(SpacecraftState s) {
return (start == null ? total : total + s.getDate().durationFrom(start)) - limit;
}
}
What is the problem? Once an event has been detected (but not occured) it cannot be cancelled or delayed. So acceptStep(...) first checks each event detector in the interval and finds that ElevationDetector first has a set event and then 1 second later the total time in contact crosses its limit. The set event has not occurred yet so the g function doesn’t know that the satellite sets a little too early and will never reach the limit value this pass. After the ElevationDetector’s set event occurs other event detectors are not reevaluated so the increasing time in contact event is allowed to occur as well even though its g function remained negative during the step. Then a second almost simultaneous event occurs because the last event to occur was increasing and the g function’s value is now negative.
This behavior does not violate the JavaDoc [1] because the JavaDoc only seems to allow changing the definition of the g function when an event occurs for the same event detector. The current implementation will correctly handle changes to the definition of a g function caused by a different event detector that add a new event or move an event closer to the current time, but not changes that delay or cancel an event.
Implementation Alternatives
No changes, don’t support events based on time in contact. Where’s the fun in that?
No changes to Orekit. A workaround is to return Action.RESET_DERIVATIVES from the eventOccured(...) method above. This causes the current step to be truncated, the propagator to compute a new step and then evaluate all the event detectors on the new step. It is a waste to recompute the step if nothing in the derivatives changed and causes the propagator to take a slightly different trajectory than it would otherwise. It also doesn’t work for propagators with non-resettable state. A brief look at performance of this option at [2]
Change implementation to recheck all event detectors after each event. Downside is that it makes propagation O(n^2) in the number of event detectors even if they all only return Action.CONTINUE. The performance hit will probably disproportionately affect analytical propagators which spend most the propagation time evaluating events and step handlers, as apposed to NumericalPropagator which spends most of its time integrating the equations of motion.
Add a new Action.RESET_EVENTS. When this action is returned all event detectors would be reevaluated.
Add a way to group event detectors and reevaluate detectors within a group. Option 3 can still be O(n^2) if there are multiple event detectors return RESET_EVENTS that don’t interact with each other, i.e. false sharing. This would eliminate the false sharing by creating groups that can be reevaluated independently. Needs more thought before it is ready for implementation. Could always implement option 2 or 3 now and this this later if necessary.
Options 2 and 3 should be fairly straight forward to implement. I’m leaning towards option 3.
This is an interesting side effect that is really worth considering. At the beginning of the Orekit project, events were mainly intended for the use case of simple operational forecasts where everything was only dependent on current state and no cross coupling between events. However, we clearly observed that since these events were available, projects used them also to implement some mission logic with side effects. One example was for example the attitude sequences (switching from day to night attitude at eclipse entry and switching back, pointing to a target when it becomes visible…). These use cases were not expected but this move must be taken into account, it corresponds to users needs.
Looking at your analysis, I also find option 3 is the cleaner solution. As we intend to publish a major version next year (9.3 to be release soon will probably be the last in the 9.x series), then changing a public interface
like Action is something that can be done for 10.0.
I experimented with a #4 style thing and it gets even more complicated than it at first sounds, which is already pretty complicated even at first glance. One way to get what you are trying to do is a composition type of thing where it propagates to an event and then a new segment picks up with the different detector. That is clunky from a code perspective obviously but it is achievable. You seen this sort of thing on multi-segment propagations in other things like GMAT but it is done in a composed sequence kind of way it’s just different segments of a trajectory.
In terms of your solution it sounds like you get #3 with #1 already so you’ve confirmed that a RESET_EVENTS type of methodology doesn’t have other artifacts. It sounds reasonable to me to go with #3. Per your trying to get around the O(n^2) issue of multiple reset events getting fired off at once is there a way to just do that once per step evaluation? I haven’t looked at the code in there in a while but something along the lines of a “reset” flag that gets OR’d and aggregated so each detector executes once through and then if at the end there was a reset tell everyone to evaluate again? Something tells me it’s more linear than that so the answer to my question is “no” but it was just a suggestion.
It always seems to grow beyond what it was designed to be. I didn’t expect it either even just a year or so ago when I revamped the event detectors.
I think since Action is an enum it is binary and source compatible to add a new value. I can probably add the new action that preserves the semantics of the existing actions. I would like to change slightly the semantics of Action.CONTINUE so that there is no unnecessary rechecking of other event detectors (i.e. delete the for loop at [5]), but that would be a slight change in behavior so it should wait for a major release. Certainly we would need coordinated releases of Hipparchus and Orekit so both analytical and integrated propagator have the same event detection behavior. I’ll start implementation in a separate branch so we can merge it when ready.
Good to know that I should avoid #4. Thanks for the other workaround. If I understand correctly the workaround you’re suggesting is stopping propagation at the relevant events and then restarting with different event detectors. This would have many of the same drawbacks as #1, recomputing steps unnecessarily and taking a slightly different trajectory. I’m also not sure it would reduce the number of event detectors since both detectors would still need to be added to both propagation segments.
As you guessed events are linearizable, to borrow a term from concurrent programming. That property is documented in [3]. Events have to be linearizable because each event determines whether the next one occurs or not by returning Action.STOP or not. So most of the logic in [4] is concerned with making sure events occur in the correct order.
I have it all implemented now and will push shortly. I believe all the changes are backward compatible. After re-evaluating I don’t see a need to make backward incompatible changes because there would be no efficiency gain. So I think this could be include in 9.3 if we do another hipparchus release, or we could wait till the next release.
