This post comes after noticing that most of the run time of AbstractMeasurement#signalTimeOfFlight is spent in the computation of the FieldTransform between transmitter and receiver frames. This is true even if the two frames coincide, and the transform is the identity.
We traced down the bottleneck to the implementation of FieldShiftingPVCoordinatesProvider#getPosition(FieldAbsoluteDate, Frame), which does not contain the usual “shortcut” to avoid computing and applying the identity transformation when the current and output frames are the same.
At the same time, I see that (Field)ShiftingPVCoordinatesProvider is only used in AbstractMeasurement for this specific case.
As both (Field)ShiftingPVCoordinatesProvider and (Field)AbsolutePVCoordinates implement the (Field)PVCoordinatesProvider interface and use the same hypotheses (i.e. constant acceleration) to compute the shifted position and velocity, why is (Field)ShiftingPVCoordinatesProvider needed in the first place?
The only addition in the latter is the overriding of the default implementation of getPosition((Field)AbsoluteDate, Frame), but this implementation could be easily moved to (Field)AbsolutePVCoordinates.
Besides reducing the amount of code to maintain, replacing (Field)ShiftingPVCoordinatesProvider by (Field)AbsolutePVCoordinates should also speedup the computation of the signal time of flight in AbstractMeasurement, as the latter already implements the “shortcut” to avoid identity transformations.
thank you for pointing this out.
There is definitely a performance issue to fix soon in a patch in the get[…]Transform for Field in general, it’s much cheaper to check for the frame equality than to create the identity.
About the use of AbsolutePVCoordinates, I believe that historically it had been introduced as an alternative to Orbit, but yeah it could be used in measurement I guess? @luc
(Field)ShiftingPVCoordinatesProvider could be then replaced by (Field)AbsolutePVCoordinates everywhere
Also you’re right AbsolutePVCoordinates should override getPosition(frame, date), the default implementation is slower.
I think there is another issues as well: the toTaylorProvider of AbsolutePVCoordinates could basically return itself, no need to overide anything
If we agree to implement these overrides and replace (Field)ShiftingPVCoordinatesProvider by (Field)AbsolutePVCoordinates, I will be happy to contribute to these changes. Let me know!
If we remove altogether tho, then TimestampedPVCoordinates will depend on AbsolutePVCoordinates in toTaylorProvider, which is a cyclic dependency between a class and their inheritor. This has triggered in me a deeper reflection that I’ll resolved for Orekit 14.0
In the meantime, you can open a performance issue to use AbsolutePVCoordinates in measurements and open an MR with patch-13.1.2 as target branch