Commit ada1ccc0 authored by Evan Ward's avatar Evan Ward
Browse files

Merge branch '684-infinit-loop' into 'develop'

Resolve "Infinite loop during event handling"

See merge request !70
parents f11f69de 38a8157e
Pipeline #420 passed with stages
in 24 minutes and 24 seconds
......@@ -50,7 +50,7 @@
<orekit.maven-install-plugin.version>3.0.0-M1</orekit.maven-install-plugin.version>
<orekit.mathjax.config>&lt;script type=&quot;text/x-mathjax-config&quot;&gt;MathJax.Hub.Config({ TeX: { extensions: [&quot;autoload.js&quot;]}});&lt;/script&gt;</orekit.mathjax.config>
<orekit.mathjax.enable>&lt;script type=&quot;text/javascript&quot; src=&quot;https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.5/MathJax.js?config=TeX-AMS_CHTML&quot;&gt;&lt;/script&gt;</orekit.mathjax.enable>
<orekit.hipparchus.version>1.6</orekit.hipparchus.version>
<orekit.hipparchus.version>1.7-SNAPSHOT</orekit.hipparchus.version>
<orekit.junit.version>4.12</orekit.junit.version>
<orekit.compiler.source>1.8</orekit.compiler.source>
<orekit.compiler.target>1.8</orekit.compiler.target>
......
......@@ -24,6 +24,13 @@
<action dev="bryan" type="update" issue="682">
Changed visibility of OrbitType parameter drivers' names to public.
</action>
<action dev="evan" type="add" issue="684" due-to="Mikael">
Fix infinite loop in event detection when a RESET_* event causes two other events
to occur simultaneously and discontinuously.
</action>
<action dev="evan" type="add" issue="684">
Add FieldFunctionalDetector.
</action>
<action dev="bryan" type="fix" issue="605">
Added support for Rinex C0, L0, S0 and D0 observation types.
</action>
......
......@@ -404,29 +404,39 @@ public class EventState<T extends EventDetector> {
*/
public boolean tryAdvance(final SpacecraftState state,
final OrekitStepInterpolator interpolator) {
final AbsoluteDate t = state.getDate();
// check this is only called before a pending event.
check(!(pendingEvent && strictlyAfter(pendingEventTime, state.getDate())));
check(!pendingEvent || !strictlyAfter(pendingEventTime, t));
final AbsoluteDate t = state.getDate();
final boolean meFirst;
// just found an event and we know the next time we want to search again
if (strictlyAfter(t, earliestTimeConsidered)) {
return false;
// just found an event and we know the next time we want to search again
meFirst = false;
} else {
// check g function to see if there is a new event
final double g = g(state);
final boolean positive = g > 0;
if (positive == g0Positive) {
// g function has expected sign
g0 = g; // g0Positive is the same
meFirst = false;
} else {
// found a root we didn't expect -> find precise location
final AbsoluteDate oldPendingEventTime = pendingEventTime;
final boolean foundRoot = findRoot(interpolator, t0, g0, t, g);
// make sure the new root is not the same as the old root, if one exists
meFirst = foundRoot && !pendingEventTime.equals(oldPendingEventTime);
}
}
final double g = g(state);
final boolean positive = g > 0;
// check for new root, pendingEventTime may be null if there is not pending event
if ((g == 0.0 && t.equals(pendingEventTime)) || positive == g0Positive) {
// at a root we already found, or g function has expected sign
if (!meFirst) {
// advance t0 to the current time so we can't find events that occur before t
t0 = t;
g0 = g; // g0Positive is the same
return false;
} else {
// found a root we didn't expect -> find precise location
return findRoot(interpolator, t0, g0, t, g);
}
return meFirst;
}
/**
......
......@@ -410,30 +410,39 @@ public class FieldEventState<D extends FieldEventDetector<T>, T extends RealFiel
*/
public boolean tryAdvance(final FieldSpacecraftState<T> state,
final FieldOrekitStepInterpolator<T> interpolator) {
final FieldAbsoluteDate<T> t = state.getDate();
// check this is only called before a pending event.
check(!pendingEvent || !strictlyAfter(pendingEventTime, t));
check(!(pendingEvent && strictlyAfter(pendingEventTime, state.getDate())));
final FieldAbsoluteDate<T> t = state.getDate();
final boolean meFirst;
// just found an event and we know the next time we want to search again
if (strictlyAfter(t, earliestTimeConsidered)) {
return false;
// just found an event and we know the next time we want to search again
meFirst = false;
} else {
// check g function to see if there is a new event
final T g = g(state);
final boolean positive = g.getReal() > 0;
if (positive == g0Positive) {
// g function has expected sign
g0 = g; // g0Positive is the same
meFirst = false;
} else {
// found a root we didn't expect -> find precise location
final FieldAbsoluteDate<T> oldPendingEventTime = pendingEventTime;
final boolean foundRoot = findRoot(interpolator, t0, g0, t, g);
// make sure the new root is not the same as the old root, if one exists
meFirst = foundRoot && !pendingEventTime.equals(oldPendingEventTime);
}
}
final T g = g(state);
final boolean positive = g.getReal() > 0;
// check for new root, pendingEventTime may be null if there is not pending event
if ((g.getReal() == 0.0 && t.equals(pendingEventTime)) || positive == g0Positive) {
// at a root we already found, or g function has expected sign
if (!meFirst) {
// advance t0 to the current time so we can't find events that occur before t
t0 = t;
g0 = g; // g0Positive is the same
return false;
} else {
// found a root we didn't expect -> find precise location
return findRoot(interpolator, t0, g0, t, g);
}
return meFirst;
}
/**
......
/* Contributed in the public domain.
* Licensed to CS Group (CS) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* CS licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.orekit.propagation.events;
import java.util.function.Function;
import org.hipparchus.Field;
import org.hipparchus.RealFieldElement;
import org.orekit.propagation.FieldSpacecraftState;
import org.orekit.propagation.events.handlers.ContinueOnEvent;
import org.orekit.propagation.events.handlers.FieldContinueOnEvent;
import org.orekit.propagation.events.handlers.FieldEventHandler;
/**
* A detector that implements the {@link #g(FieldSpacecraftState)} function using a lambda
* that can be set using {@link #withFunction(Function)}.
*
* <p>For example, to create a simple date detector use:
*
* <pre>
* FieldFunctionalDetector&lt;T&gt; d = new FieldFunctionalDetector&lt;&gt;(field)
* .withGFunction((s) -&gt; s.getDate().durationFrom(triggerDate))
* .withMaxCheck(field.getZero().add(1e10));
* </pre>
*
* @param <T> the type of numbers this detector uses.
* @author Evan Ward
* @since 10.2
*/
public class FieldFunctionalDetector<T extends RealFieldElement<T>>
extends FieldAbstractDetector<FieldFunctionalDetector<T>, T> {
/** The g function. */
private final Function<FieldSpacecraftState<T>, T> function;
/**
* Create an event detector with the default values. These are {@link
* #DEFAULT_MAXCHECK}, {@link #DEFAULT_THRESHOLD}, {@link #DEFAULT_MAX_ITER}, {@link
* ContinueOnEvent}, and a g function that is identically unity.
*
* @param field on which this detector is defined.
*/
public FieldFunctionalDetector(final Field<T> field) {
this(field.getZero().add(DEFAULT_MAXCHECK),
field.getZero().add(DEFAULT_THRESHOLD),
DEFAULT_MAX_ITER,
new FieldContinueOnEvent<>(), value -> field.getOne());
}
/**
* Private constructor.
*
* @param maxCheck maximum checking interval (s)
* @param threshold convergence threshold (s)
* @param maxIter maximum number of iterations in the event time search
* @param handler event handler to call at event occurrences
* @param function the switching function.
*/
private FieldFunctionalDetector(
final T maxCheck,
final T threshold,
final int maxIter,
final FieldEventHandler<? super FieldFunctionalDetector<T>, T> handler,
final Function<FieldSpacecraftState<T>, T> function) {
super(maxCheck, threshold, maxIter, handler);
this.function = function;
}
@Override
public T g(final FieldSpacecraftState<T> s) {
return function.apply(s);
}
@Override
protected FieldFunctionalDetector<T> create(
final T newMaxCheck,
final T newThreshold,
final int newMaxIter,
final FieldEventHandler<? super FieldFunctionalDetector<T>, T> newHandler) {
return new FieldFunctionalDetector<>(newMaxCheck, newThreshold, newMaxIter,
newHandler, function);
}
/**
* Create a new event detector with a new g function, keeping all other attributes the
* same. It is recommended to use {@link #withMaxCheck(T)} and {@link
* #withThreshold(T)} to set appropriate values for this g function.
*
* @param newGFunction the new g function.
* @return a new detector with the new g function.
*/
public FieldFunctionalDetector<T> withFunction(
final Function<FieldSpacecraftState<T>, T> newGFunction) {
return new FieldFunctionalDetector<>(getMaxCheckInterval(), getThreshold(),
getMaxIterationCount(), getHandler(), newGFunction);
}
/**
* Get the switching function.
*
* @return the function used in {@link #g(FieldSpacecraftState)}.
*/
public Function<FieldSpacecraftState<T>, T> getFunction() {
return function;
}
}
......@@ -82,7 +82,7 @@ public abstract class CloseEventsAbstractTest {
// all three times. Then we get a non bracketing exception.
Propagator propagator = getPropagator(10.0);
double t1 = 49, t2 = t1 + 1e-15, t3 = t1 + 4.9;
double t1 = 49, t2 = FastMath.nextUp(t1), t3 = t1 + 4.9;
List<Event<EventDetector>> events = new ArrayList<>();
TimeDetector detector1 = new TimeDetector(t1)
.withHandler(new Handler<>(events, Action.RESET_DERIVATIVES))
......@@ -164,6 +164,112 @@ public abstract class CloseEventsAbstractTest {
Assert.assertEquals(5, events2.get(0).getState().getDate().durationFrom(epoch), 0.0);
}
/**
* Previously there were some branches when tryAdvance() returned false but did not
* set {@code t0 = t}. This allowed the order of events to not be chronological and to
* detect events that should not have occurred, both of which are problems.
*/
@Test
public void testSimultaneousEventsReset() {
// setup
double tol = 1e-10;
Propagator propagator = getPropagator(10);
boolean[] firstEventOccurred = {false};
List<Event<EventDetector>> events = new ArrayList<>();
TimeDetector detector1 = new TimeDetector(5)
.withMaxCheck(10)
.withThreshold(tol)
.withHandler(new Handler<EventDetector>(events, Action.RESET_STATE) {
@Override
public Action eventOccurred(SpacecraftState s, EventDetector detector, boolean increasing) {
firstEventOccurred[0] = true;
return super.eventOccurred(s, detector, increasing);
}
@Override
public SpacecraftState resetState(final EventDetector detector, final SpacecraftState oldState) {
return oldState;
}
});
propagator.addEventDetector(detector1);
// this detector changes it's g function definition when detector1 fires
FunctionalDetector detector2 = new FunctionalDetector()
.withMaxCheck(1)
.withThreshold(tol)
.withHandler(new RecordAndContinue<>(events))
.withFunction(state -> {
if (firstEventOccurred[0]) {
return new TimeDetector(1, 3, 5).g(state);
}
return new TimeDetector(5).g(state);
}
);
propagator.addEventDetector(detector2);
// action
propagator.propagate(epoch.shiftedBy(20));
// verify
// order is important to make sure the test checks what it is supposed to
Assert.assertEquals(5, events.get(0).getState().getDate().durationFrom(epoch), 0.0);
Assert.assertTrue(events.get(0).isIncreasing());
Assert.assertEquals(detector1, events.get(0).getDetector());
Assert.assertEquals(5, events.get(1).getState().getDate().durationFrom(epoch), 0.0);
Assert.assertTrue(events.get(1).isIncreasing());
Assert.assertEquals(detector2, events.get(1).getDetector());
Assert.assertEquals(2, events.size());
}
/**
* When two event detectors have a discontinuous event caused by a {@link
* Action#RESET_STATE} or {@link Action#RESET_DERIVATIVES}. The two event detectors
* would each say they had an event that had to be handled before the other one, but
* neither would actually back up at all. For #684.
*/
@Test
public void testSimultaneousDiscontinuousEventsAfterReset() {
// setup
double t = FastMath.PI;
double tol = 1e-10;
Propagator propagator = getPropagator(10);
List<Event<EventDetector>> events = new ArrayList<>();
SpacecraftState newState = new SpacecraftState(new KeplerianOrbit(
42e6, 0, 0, 0, 0, 0, PositionAngle.TRUE, eci, epoch.shiftedBy(t), mu));
TimeDetector resetDetector = new TimeDetector(t)
.withHandler(new ResetHandler<>(events, newState))
.withMaxCheck(10)
.withThreshold(tol);
propagator.addEventDetector(resetDetector);
List<EventDetector> detectors = new ArrayList<>();
for (int i = 0; i < 2; i++) {
FunctionalDetector detector1 = new FunctionalDetector()
.withFunction(s -> s.getA() - 10e6)
.withThreshold(tol)
.withMaxCheck(10)
.withHandler(new RecordAndContinue<>(events));
propagator.addEventDetector(detector1);
detectors.add(detector1);
}
// action
propagator.propagate(epoch.shiftedBy(10));
// verify
Assert.assertEquals(t, events.get(0).getState().getDate().durationFrom(epoch), tol);
Assert.assertTrue(events.get(0).isIncreasing());
Assert.assertEquals(resetDetector, events.get(0).getDetector());
// next two events can occur in either order
Assert.assertEquals(t, events.get(1).getState().getDate().durationFrom(epoch), tol);
Assert.assertTrue(events.get(1).isIncreasing());
Assert.assertEquals(detectors.get(0), events.get(1).getDetector());
Assert.assertEquals(t, events.get(2).getState().getDate().durationFrom(epoch), tol);
Assert.assertTrue(events.get(2).isIncreasing());
Assert.assertEquals(detectors.get(1), events.get(2).getDetector());
Assert.assertEquals(events.size(), 3);
}
/**
* test the g function switching with a period shorter than the tolerance. We don't
* need to find any of the events, but we do need to not crash. And we need to
......@@ -1046,6 +1152,63 @@ public abstract class CloseEventsAbstractTest {
Assert.assertEquals(-5, events2.get(0).getState().getDate().durationFrom(epoch), 0.0);
}
/**
* Previously there were some branches when tryAdvance() returned false but did not
* set {@code t0 = t}. This allowed the order of events to not be chronological and to
* detect events that should not have occurred, both of which are problems.
*/
@Test
public void testSimultaneousEventsResetReverse() {
// setup
double tol = 1e-10;
Propagator propagator = getPropagator(10);
boolean[] firstEventOccurred = {false};
List<Event<EventDetector>> events = new ArrayList<>();
TimeDetector detector1 = new TimeDetector(-5)
.withMaxCheck(10)
.withThreshold(tol)
.withHandler(new Handler<EventDetector>(events, Action.RESET_STATE) {
@Override
public Action eventOccurred(SpacecraftState s, EventDetector detector, boolean increasing) {
firstEventOccurred[0] = true;
return super.eventOccurred(s, detector, increasing);
}
@Override
public SpacecraftState resetState(final EventDetector detector, final SpacecraftState oldState) {
return oldState;
}
});
propagator.addEventDetector(detector1);
// this detector changes it's g function definition when detector1 fires
FunctionalDetector detector2 = new FunctionalDetector()
.withMaxCheck(1)
.withThreshold(tol)
.withHandler(new RecordAndContinue<>(events))
.withFunction(state -> {
if (firstEventOccurred[0]) {
return new TimeDetector(-1, -3, -5).g(state);
}
return new TimeDetector(-5).g(state);
}
);
propagator.addEventDetector(detector2);
// action
propagator.propagate(epoch.shiftedBy(-20));
// verify
// order is important to make sure the test checks what it is supposed to
Assert.assertEquals(-5, events.get(0).getState().getDate().durationFrom(epoch), 0.0);
Assert.assertTrue(events.get(0).isIncreasing());
Assert.assertEquals(detector1, events.get(0).getDetector());
Assert.assertEquals(-5, events.get(1).getState().getDate().durationFrom(epoch), 0.0);
Assert.assertTrue(events.get(1).isIncreasing());
Assert.assertEquals(detector2, events.get(1).getDetector());
Assert.assertEquals(2, events.size());
}
/**
* test the g function switching with a period shorter than the tolerance. We don't
* need to find any of the events, but we do need to not crash. And we need to
......@@ -2027,4 +2190,36 @@ public abstract class CloseEventsAbstractTest {
}
private static class ResetHandler<T extends EventDetector> extends Handler<T> {
private final SpacecraftState newState;
private final int times;
private long i = 0;
public ResetHandler(List<Event<T>> events, SpacecraftState newState) {
this(events, newState, Integer.MAX_VALUE);
}
public ResetHandler(List<Event<T>> events, SpacecraftState newState, int times) {
super(events, Action.RESET_STATE);
this.newState = newState;
this.times = times;
}
@Override
public Action eventOccurred(final SpacecraftState s, final T detector, final boolean increasing) {
super.eventOccurred(s, detector, increasing);
if (i++ < times) {
return Action.RESET_STATE;
}
return Action.CONTINUE;
}
@Override
public SpacecraftState resetState(T detector, SpacecraftState oldState) {
Assert.assertEquals(0, newState.getDate().durationFrom(oldState.getDate()), 0);
return newState;
}
}
}
......@@ -175,6 +175,112 @@ public abstract class FieldCloseEventsAbstractTest<T extends RealFieldElement<T>
Assert.assertEquals(5, events2.get(0).getState().getDate().durationFrom(epoch).getReal(), 0.0);
}
/**
* Previously there were some branches when tryAdvance() returned false but did not
* set {@code t0 = t}. This allowed the order of events to not be chronological and to
* detect events that should not have occurred, both of which are problems.
*/
@Test
public void testSimultaneousEventsReset() {
// setup
double tol = 1e-10;
FieldPropagator<T> propagator = getPropagator(10);
boolean[] firstEventOccurred = {false};
List<FieldRecordAndContinue.Event<FieldEventDetector<T>, T>> events = new ArrayList<>();
TimeDetector detector1 = new TimeDetector(5)
.withMaxCheck(v(10))
.withThreshold(v(tol))
.withHandler(new Handler<FieldEventDetector<T>>(events, Action.RESET_STATE) {
@Override
public Action eventOccurred(FieldSpacecraftState<T> s, FieldEventDetector<T> detector, boolean increasing) {
firstEventOccurred[0] = true;
return super.eventOccurred(s, detector, increasing);
}
@Override
public FieldSpacecraftState<T> resetState(FieldEventDetector<T> detector, FieldSpacecraftState<T> oldState) {
return oldState;
}
});
propagator.addEventDetector(detector1);
// this detector changes it's g function definition when detector1 fires
FieldFunctionalDetector<T> detector2 = new FieldFunctionalDetector<>(field)
.withMaxCheck(v(1))
.withThreshold(v(tol))
.withHandler(new FieldRecordAndContinue<>(events))
.withFunction(state -> {
if (firstEventOccurred[0]) {
return new TimeDetector(1, 3, 5).g(state);
}
return new TimeDetector(5).g(state);
}
);
propagator.addEventDetector(detector2);
// action
propagator.propagate(epoch.shiftedBy(20));
// verify
// order is important to make sure the test checks what it is supposed to
Assert.assertEquals(5, events.get(0).getState().getDate().durationFrom(epoch).getReal(), 0.0);
Assert.assertTrue(events.get(0).isIncreasing());
Assert.assertEquals(detector1, events.get(0).getDetector());
Assert.assertEquals(5, events.get(1).getState().getDate().durationFrom(epoch).getReal(), 0.0);
Assert.assertTrue(events.get(1).isIncreasing());
Assert.assertEquals(detector2, events.get(1).getDetector());
Assert.assertEquals(2, events.size());
}
/**
* When two event detectors have a discontinuous event caused by a {@link
* Action#RESET_STATE} or {@link Action#RESET_DERIVATIVES}. The two event detectors
* would each say they had an event that had to be handled before the other one, but
* neither would actually back up at all. For #684.
*/
@Test
public void testSimultaneousDiscontinuousEventsAfterReset() {
// setup
double t = FastMath.PI;
double tol = 1e-10;
FieldPropagator<T> propagator = getPropagator(10);
List<FieldRecordAndContinue.Event<FieldEventDetector<T>, T>> events = new ArrayList<>();
FieldSpacecraftState<T> newState = new FieldSpacecraftState<>(new FieldKeplerianOrbit<>(
v(42e6), v(0), v(0), v(0), v(0), v(0), PositionAngle.TRUE, eci, epoch.shiftedBy(t), v(mu)));
TimeDetector resetDetector = new TimeDetector(t)
.withHandler(new ResetHandler<>(events, newState))
.withMaxCheck(v(10))
.withThreshold(v(tol));
propagator.addEventDetector(resetDetector);