Hi @luc,
I saw that 32f94b9d827eb611d7314ebe26b6c564769aa3d3 changed EphemerisSegmentPropagator to not store it’s own copy of the coordinates.
I’ve noticed an issue where that causes O(n**2) performance for something that should be O(n). Before that commit EphemerisSegmentPropagator called ephemeris.getCoordinates() once in the constructor. After that commit it calls ephemeris.getCoordinates() on every call to interpolate(AbsoluteDate).
That’s fine if ephemeris.getCoordinates() returns a reference to a list without copying. But some classes implement getCoordinates by transforming from another format. So now instead of one copy being made when the EphemerisSegmentPropagator was created, a copy is being made on every call to interpolate(AbsoluteDate). For example, TrajectoryStateHistory.getCoordinates() from the OCM is implemented as a stream:
return states.stream().map(os -> os.toCartesian(body, mu)).collect(Collectors.toList());
This means that using a EphemerisSegmentPropagator from an OCM is now O(n**2) in the number of state vectors in the OCM. Similarly IIRVSegment has a non-trivial implementation of getCoordinates().
Solution Ideas
As EphemerisSegmentPropagator now depends on how getCoordinates() is implemented, that seems to violate the principle of encapsulation. So I would propose reverting 32f94b9d and restoring the old behavior of calling getCoordinates() once in the constructor. Then there is less dependence on if getCoordinates() is implemented with stream or not. In other words, it allows EphemerisSegmentPropagator to store the coordinates in a format optimized for fast retrieval and interpolation.
Another option could be to update the javadoc for getCoordinates() and require that implementing classes only return a reference from that method and not do any processing of the coordinates.
Another option could be to add another method next to getCoordinates() to only get a few neighbors. That would be better performant when transforming the coordinates than the existing implementation. But would still require transforming the same point several times when doing the propagation.
What do you think?
Best Regards,
Evan