Improve Performance of Transform

Hi All,

I was profiling an application recently that computes access times from a TLE and it spent a significant portion (~14%) of its time in Transform.compositeAcceleration(...) and Transform.compositeRotationAcceleration(...). This seemed odd to me since the computation should only need to transform positions, not velocities or accelerations. The current API for frame transforms always computes transforms with acceleration information. Thanks to Luc’s previous performance work computing the constituent Transforms is very quick. So much that composing the Transforms now uses a significant fraction of the computing time. There should also be some potential time savings available from not computing unused derivatives of the transformation in the TransformProvider.

So this proposal is to allow the user to request Transforms that only include the information they need. Specifically three new classes would be added:

  • StaticTransform that only transforms positions and vectors
  • KinematicTransform that also transforms velocities, extends StaticTransform
  • DynamicTransform that also transforms accelerations, extends KinematicTransform

Similarly three methods would be added to Frame: getStaticTransformTo(...), getKinematicTransformTo(...), and getDynamicTransformTo(...). Supporting methods would be added to TransformProvider. Existing TransformProviders would be updated to use the new methods.

Then performance of the new implementation would be compared to the old using JMH to measure it. If the speedup is less than 1.5 (50% faster) the new code would not be merged into Orekit.

Finally all the code in Orekit that uses frame transforms would be reviewed to use the appropriate transform. For example TopocentricFrame.getElevation(...) would use a StaticTransform.

I think this could be done in a backward compatible way without duplicating much code by having Transform extend DynamicTransform. Might be a moot point since I won’t be able to implement this by the end of May and the next release is a major one.

One complicating factor is that Orekit does not have a type that just combines position and velocity coordinates (PVCoordinates includes acceleration). This could create confusion in instances like the following. If done as part of a major release we move the acceleration parts of PVCoordinates to a new PVACoordinates class and similarly with Transform.transforPVCoordinates(...).

PVCoordinates pv = ...;
KinematicTransform kinematic = frame.getDynamicTransformT(other, date);
// It is a bit unclear if the following transforms the acceleration of pv
PVCoorinates newPV = kinematic.transformPVCoordinates(pv);

What do you think?

Regards,
Evan

Hi Evan,

Big +1 for your proposal.

I recently performed some test performance on Orekit Orbit Determination. I found the same problem of performance with FieldTransform class (i.e. the field version of the Transform class).
I used an Orbit Determination with 230k measurements and about 45% of the total duration of the Orbit Determination is spent inside the FieldTransform constructor, mainly in FieldTransform.compositeAcceleration(...), FieldTransform.compositeRotationAcceleration(...), FieldTransform.compositeVelocity(...), FieldTransform.compositeRotation(...), FieldTransform.compositeRotationRate(...) and FieldTransform.compositeTranslation(...) methods.

Here a “non user-friendly” diagram showing the portion of time spent into the different methods during an orbit determination process (I used Yourkit tool):

Therefore, it can be a good improvement if you proposal is also performed for the FieldTransform class too. In that respect, FieldStaticTransform, FieldKinematicTransform and FieldDynamicTransform have to be added too.

I agree, there is a lot of thing to finish for the next release. However, it can be a good improvement for the next major release!
What I can propose is, once the change performed for the Transform class, I can do it for the FieldTransform class.

Regards,
Bryan

This is a great idea!
It also makes me wonder if we should not add stripped-down versions of DerivativeStructure to Hipparchus. These versions would be Derivative{1,2} (or UnivariateDerivative{1,2}) and would handle only single parameter derivatives and only up to fixed derivation orders 1 and 2 respectively. In fact, this would be a rebirth of oooold classes I wrote in C++ back in 1993! I guess that could reduce some overhead when we use the fule DerivativeStructure class and configure it with parameters=1 and order=1 (or 2).
This could be done for Hipparchus 1.7, it is really straightforward, I’ll look if I can have a look at it. I can’t work on Orekit for now :face_with_thermometer:, but I can (and do) work on Hipparchus :shushing_face:

1 Like

Thanks! That would be a big help!

Could be useful. We would need to measure the performance. Are you thinking of using it in place of PV or PVA coordinates in Orekit?

Just make sure you take the time you need to fully recover. You’re more important than the code.

1 Like