Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adaptable max checking interval for event detection #270

Closed
JakeRandellOC opened this issue Aug 24, 2023 · 5 comments
Closed

Adaptable max checking interval for event detection #270

JakeRandellOC opened this issue Aug 24, 2023 · 5 comments
Assignees
Milestone

Comments

@JakeRandellOC
Copy link

A key feature that is missing from the existing event detecting architecture, is the ability to dynamically change the maximum checking interval based on the current ODEStateAndDerivative or some condition. A key use-case for this would be to be able to detect short infrequent events in a large search space, where a small interval would have to be set in order to capture every event, at the expense of a long computation time.

The suggestion is to replace the current maxCheckInterval input of the EventState constructor with some interface that allows the interval to be set depending dynamically. The interface could look like:

public interface AdaptableInterval {
    double maxCheckInterval(ODEStateAndDerivative s);
}

A default implementation would simply always return the same value, but users could register a custom method using something like:

EventState detector = new EventState(handler, state -> maxCheckInterval(s) ? 1.0 : 100.0, convergence, maxIterationCount, solver);

See the original forum post here and the associated Orekit issue here.

@maisonobe maisonobe self-assigned this Aug 28, 2023
@maisonobe maisonobe added this to the 3.0 milestone Aug 28, 2023
@maisonobe
Copy link
Contributor

This has been implemented in branch issue-270.
Note that in order to use this from Orekit, one need to change

return detector.getMaxCheckInterval();

by

return s -> detector.getMaxCheckInterval();

at line 889 of AbstractIntegratedPropagator.java

and to change

return detector.getMaxCheckInterval();

by

return s -> detector.getMaxCheckInterval().getReal();

at line 889 of FieldAbstractIntegratedPropagator.java

However, I am working on a larger change in Orekit to solve issue 1178, so I will include this change.

I would however like to see if people agree with this before I merge everything.

@wardev
Copy link
Contributor

wardev commented Aug 28, 2023

I think it is a helpful feature, one that I've thought about before, but didn't know a good way to implement.

"The Rules of Event Detection" should be updated for the change, at the bottom of [1] to formally specify what conditions the new maxCheckInterval function has to meet in order for an event to be detected. Perhaps others don't use that part of the docs, but I find the formal language helpful when thinking through what my event detectors will do.

I don't think the implementation at [2] meets the request. Consider an integrator that takes a large step (e.g. for a DSST or analytic propagator). Using the maxCheck computed for the initial state for the whole step, potentially many times, may not be the behavior the requester wanted. And it would make it hard to formally specify what I have to set the max check to in order to find the event.

Maybe a better approach would be to call the maxCheck function each time through the loop in [2] so that each maxCheck interval is chosen by the maxCheck function.

Or alternatively pass the step to the maxCheck function and let it return a list of points in time to evaluate. There are many options. :)

The current implementation will always check at the beginning and end of the integrator's step. I don't think we should change that as that would require much more effort. Could be worth examining removing that constraint in a separate feature, but I think this feature already can deliver something useful without dealing with all the complexities that removing that assumption would entail.

[1] https://hipparchus.org/hipparchus-ode/apidocs/org/hipparchus/ode/events/package-summary.html

[2]

final int n = FastMath.max(1, (int) FastMath.ceil(FastMath.abs(dt) / detector.getMaxCheckInterval().currentInterval(s1)));

@maisonobe
Copy link
Contributor

Thanks Evan.
I have taken all your comments into account.

@wardev
Copy link
Contributor

wardev commented Aug 30, 2023

Thanks Luc.

I think a more formal criteria might be the following.

Given times t, t_e1, t_e2 such that t < t_e1 < t_e2, g(t_e1) = 0, g(t_e2) = 0, and g(t_i) != 0 on the intervals {[t, t_e1>, <t_e1, t_e2>} (i.e. t_e1, t_e2 are the next two event times after t) then t_e1 will be detected if maxCheck(t, y(t)) < t_e2. (I.e. the max check interval must be less than the time until the second event.)

@maisonobe
Copy link
Contributor

Thanks Evan, I have included your formulation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants