Zero-duration short-circuit?

Orekit development
1

Hello!

I’m wondering whether there’s a reason that methods like implementations of Propagator::propagate and the various shiftedBy implementations don’t “short-circuit” when the propagation’s target date is the same as the current state.

Is it something to do with the EventDetector framework?

I ask because - e.g. - propagating for zero seconds results in non-zero changes to state. In the Keplerian case at least, the changes seem to diverge when iterated.

A simple example:

    /** Test propagating for zero seconds */
    @Test
    void testPropZeroSeconds() {
        final Orbit orb =
                new CartesianOrbit(new PVCoordinates(new Vector3D(3.698108937485445E7, -2.025379543625723E7, 0.0),
                                                     new Vector3D(1476.9302283162829, 2696.7038817827524, 0.0)),
                                   FramesFactory.getEME2000(),
                                   AbsoluteDate.J2000_EPOCH,
                                   Constants.WGS84_EARTH_MU);
        
        final Orbit propdZero = new KeplerianPropagator(orb).propagate(orb.getDate()).getOrbit();
        
        final Vector3D ogPos = orb.getPVCoordinates().getPosition();
        final Vector3D ogVel = orb.getPVCoordinates().getVelocity();
        
        final Vector3D propdPos = propdZero.getPVCoordinates().getPosition();
        final Vector3D propdVel = propdZero.getPVCoordinates().getVelocity();
        
        final double[] diffArr = MathArrays.concatenate(ogPos.subtract(propdPos).toArray(),
                                                        ogVel.subtract(propdVel).toArray());
        
        DoubleStream.of(diffArr).forEach(System.out::println);
    }
2

Hello,

I think what you’re seeing is directly related to these discussions.
So yeah there is no check on the fact that dt is zero, so propagation methods are still called and introduce noise. On another note, it also introduces a performance bottleneck for LofOffset attitude providers: when called on an orbit, it shifts by a null duration which is not computationally free. I’ve meant several times to bring it up on the forum, so now is the opportunity I guess.

Cheers,
Romain.

1 Like
3

Thanks, Romain.

I believe I’ve run into issues with LofOffset before as a default in SpacecraftState, though it looks like that’s been changed. I wrote a SpacecraftStateFactory locally to get around it.

Anyway, it should be pretty straight forward to add if (dt == 0.0) checks pretty much everywhere, I’d only be concerned with messing something up in the EventDetector/Handler framework within - e.g. - NumericalPropagator.

If anything, adding them to shiftedBy methods should be completely free.

4

Actually if you look in AbstractIntegratedPropagator, you see this:

if (getInitialState().getDate().equals(tEnd)) {
                // don't extrapolate
                return getInitialState();
            }

So here there is no actual propagation. This now looks like an inconsistency with analytical propagators, that do make computations even if dt=0. I think we should change the latter to align with the former. I don’t believe there is anything to worry about event detections, as none can be detected at t0.

Cheers,
Romain.

1 Like
5

Ah, indeed! Yes, FWIW I think skipping unnecessary calculation is the right move.

Should I close this and open an issue?

6

Before going ahead I would like some other opinions, in particular from the rest of the dev team @orekit_dev_team

7

I agree with using short circuit were possible.
We should take care about lack of continuity, but I guess looking if some tests break or not would be sufficient to accept the change.

8

Hello @Serrof , @Ryan,

After taking a deeper look, the slight difference comes from some computation made inside the propagateOrbit method of the KeplerianPropagator

        do {
            // we use a loop here to compensate for very small date shifts error
            // that occur with long propagation time
            orbit = orbit.shiftedBy(date.durationFrom(orbit.getDate()));
        } while (!date.equals(orbit.getDate()));

More specifically, it happens because this is a CartesianOrbit which call one of the Orbit constructor :

    protected Orbit(final TimeStampedPVCoordinates pvCoordinates, final Frame frame, final double mu)
        throws IllegalArgumentException {
        ensurePseudoInertialFrame(frame);
        this.date = pvCoordinates.getDate();
        this.mu = mu;
        if (pvCoordinates.getAcceleration().getNormSq() == 0) {
            // the acceleration was not provided,
            // compute it from Newtonian attraction
            final double r2 = pvCoordinates.getPosition().getNormSq();
            final double r3 = r2 * FastMath.sqrt(r2);
            this.pvCoordinates = new TimeStampedPVCoordinates(pvCoordinates.getDate(),
                                                              pvCoordinates.getPosition(),
                                                              pvCoordinates.getVelocity(),
                                                              new Vector3D(-mu / r3, pvCoordinates.getPosition()));
        } else {
            this.pvCoordinates = pvCoordinates;
        }
        this.frame = frame;
    }

Long story short, i also think this can be done.

Cheers,
Vincent

1 Like
9

Actually, by looking at the inheritors of AbstractAnalyticalPropagator, the problem is that many of them do not use osculating orbits, so there is some computation (conversion here) even if the delta of time is zero. So I think the easiest solution is to modify propagateOrbit in (Field)KeplerianPropagator only. And I guess leave the shiftedBy method alone, anyway it’s a method to use with care.

If you’re ok with that @Ryan you can open an issue on the forge

1 Like