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

Hi @evan.ward

Next Orekit version will be a major version. Therefore, I would like to restart the dialogue on this subject. I find it very interesting and I think it can be very useful to improve Orekit performance.

Are you still interested in implementing this feature?

For my part, I am still available to do the Field implementation.

Best regards,
Bryan

Hi @bcazabonne ,

Yes still very interested, though finding the time to do it is always the hard part. I don’t see having time to do it in the next couple weeks. :slight_smile: What is the timeline for releasing Orekit 11?

Regards,
Evan

Finding the time is usually the main issue for fixing bugs and implementing new functionnalities. But there is no emergency for implementing this feature. You can start it when you have more time available.

I don’t think there is a current deadline for releasing Orekit 11.0. As this version will include a lot of new features and bug fixes, I think it will be released in few months.

Bryan

Looking through the Orekit code, there are 77 places that call transformPosition, transformVector, or transformLine.

There are 28 places that call transformPVCoordinates. Of those it is not immediately clear how many use acceleration. Many of the calls to transformPVCoordinates are methods that perform frame transformation for the user and return PVA such as Orbit.getPVCoordinates(Frame) or implementations of PVCoordinatesProvider.getPVCoordinates(...).

So in the interest of setting the scope smaller on something achievable I will focus on implementing a StaticTransform since that would be the most straight forward to use, would handle most of the calls to transform__, and should give the largest performance increase.

I think there is also a wealth of performance improvements to be made by adapting existing APIs to allow Orekit to not calculate quantities that the user does not want to know. Plenty of future opportunities for when users desire improved performance.

Regards,
Evan

Thank you for the analysis Evan.
Once you have added the StaticTransform class, I will take care of the FieldStaticTransform class.

Bryan

Implemented StaticTransform and merged for #903: Implement StaticTransform (#903) · Issues · Orekit / Orekit · GitLab

Observed a speed up of 2 (twice as fast) for the cases where nutation caching works well.

Thank you very mucj @evan.ward! It is a very nice feature for the 11.2 release.

It can try to implement the FieldStaticTransfom class now. I hope it will improve the performance of the orbit determination :slight_smile:

Bryan