Performances problem due to drivers with timeSpan

Orekit development
21

I tried something…
I tried to let TimeSpanMap extending CopyOnWriteArrayList and removed the syncronized keys (I maybe removed too much synchronized but I wonder CopyOnWriteArrayList is thread safe). Thanks to that, the propagation duration is divided by 2

=============
Propagation start: 2000-01-01T11:58:55.816Z
Propagation end: 2000-01-08T11:58:55.816Z
Number of maneuvers: 8
Propagation duration: 0.139
=============
Propagation start: 2000-01-01T11:58:55.816Z
Propagation end: 2000-01-08T11:58:55.816Z
Number of maneuvers: 158
Propagation duration: 0.519
=============
Propagation start: 2000-01-01T11:58:55.816Z
Propagation end: 2000-01-08T11:58:55.816Z
Number of maneuvers: 658
Propagation duration: 2.6310000000000002

It does not fully solved the problem. But it is something interesting for simulations containing a lot of maneuvers.

22

Hi Bryan,

Just so that I fully understand: do you (or @pauline) have “active” (selected) parameter drivers (for partial derivatives either directly or indirectly via OD)? If yes which ones? On the thrust vector only, the start/stop dates only, or both?

Cheers,
Romain.

23

No active parameters, that’s propagation only

24

Ok so in that case, the following code is basically identical to a single ConstantThrustManeuver and should be much faster:

      final double thrustMagnitude = 1.;
      final Vector3D thrustDirection = Vector3D.PLUS_I;
      final Vector3D thrustVector = thrustDirection.scalarMultiply(thrustMagnitude);
      final ThrustVectorProvider thrustVectorProvider = new ThrustVectorProvider() {
          @Override
          public Vector3D getThrustVector(AbsoluteDate date, double mass) {
              return thrustVector;
          }
          @Override
          public <T extends CalculusFieldElement<T>> FieldVector3D<T> getThrustVector(FieldAbsoluteDate<T> date, T mass) {
              return null;
          }
      };
      final TimeSpanMap<ThrustVectorProvider> timeSpanMap = new TimeSpanMap<>(thrustVectorProvider);
      final double isp = 100.;
      final ProfileThrustPropulsionModel propulsionModel = new ProfileThrustPropulsionModel(timeSpanMap, isp, "");
      final AbsoluteDate startDate = AbsoluteDate.ARBITRARY_EPOCH;
      final AbsoluteDate endDate = startDate.shiftedBy(10);
      final TimeInterval timeInterval = TimeInterval.of(startDate, endDate);
      final TimeIntervalsManeuverTrigger trigger = TimeIntervalsManeuverTrigger.of(timeInterval);
      final Maneuver maneuver = new Maneuver(null, trigger, propulsionModel);

As such it requires the develop branch, but the trigger could also be reimplemened fairly easily, the point is just to have no ParameterDriver. The nice thing with TimeIntervalsManeuverTrigger though is that it generalizes to N burns, with a single ForceModel.

25

Hi!

I’m coming back on this topic since it is a concern for our application… with potential impacts for the next study

First of all, we unfortunately can’t implement a new propulsion model since we only have one in our software that is generic to all platform and applicable to orbit determination and propagation as well as spacecraft operations and mission analysis. The idea to have only one is to improve code maintenance. In addition, I think that the main changes presented below can be useful for all Orekit users doing propagation with maneuvers.

So, I implemented (during my work time) a merge request with some small improvements in maneuvers handling: Improve maneuver performance (!901) · Merge requests · Orekit / Orekit · GitLab

To summarize, the improvements are significant thanks to 2 main changes. By significant, I mean the computation time (i.e., propagation time) is reduced by a factor of 3, 4 or even 5 depending the number of maneuvers. The two main changes are presented below

Extending CopyOnWriteArrayList is more efficient and preserves thread safety

First significant change is to remove all the synchronized keys in TimeSpanMap and to extend the CopyOnWriteArrayList class. This class introduce thread safety and reduce considerably the computation time if the number of read operations is greater than the number of write operations. Which is I think the case for TimeSpanMap

From what I understand, it provided the same service as synchronized keys, but with a reduced computation time on operations.

Added mechanism to remove time-based force models non applicable inside propagation span

The idea of this commit is to look at all the force models that are time dependent and to see if the time dependency is inside the propagation span or not. If not, the force model is not used during the orbit propagation. If yes, the force model is used. At the end, the the “cleaned” force models are putted back to the list of forceModels so that a user performing propagation from t0 → t1 and after t1 → t2 is not impacted by the clean.
This change is only applicable to Maneuver with a DateBaseManeuverTrigger and the methods are not public to limit the risk of wrong used.

Thanks to this two changes we considerably reduced the propagation time.

Any comment is welcome. I didn’t implemented unit tests yet or analysed the 6 failing tests because I would like to have feedbacks before on the two changes. If everything is OK, we would be please to contribute a patch release with it :slight_smile:

Best regards,
Bryan

26

Sounds like a big performance increase!

I like that the interface was added to ForceModel so that any force can use it.

I think the MR would need some tests. Specifically for concurrency. I don’t see how the CopyOnWriteArrayList is used in TimeSpanMap.

Should the ignored ForceModels be removed before or after setting up the STM? I would think before, so the STM can be smaller, but maybe users would expect entries in the STM for the removed force models.

Regards,

Evan

27

Hi Bryan,

I might be slow but I still don’t understand why you’re not using TimeIntervalsManeuverTrigger. Its a regular AbstractManeuverTrigger so can be used with any PropulsionModel inside Maneuver. I’ve tried it with thousands of burns and it performs orders of magnitude faster than having N ConstantThrustManeuver.

About the proposed time based system from the MR itself, I believe it’s doable more easily at lower level with ForceModelModifier. I don’t have access to an IDE just now but basically with words it would be along these lines:
In the init method, check if start/end date is between the initial and final epochs. If yes, do not modify anything. If no, store the values of the original parameters drivers to use them when calling acceleration on the original model. And return an empty list for the parameters drivers getter.

Cheers,
Romain.

28

Thank you for your comments.

As I said at the beginning of my post, the model we use is generic to anything, even the estimation of the maneuver epochs. So we can’t use the TimeIntervalsManeuverTrigger which do not propose this feature.

I tried to look at the ForceModelModifier class since I’m not familiar with it. I don’t see a lot of use but from my understanding, we will need to create in our tool a new class which implement it and wrap the Maneuver class. If yes, that’s something I would like to avoid since the encountered issues can occur to any user and I would prefer a fix directly in Orekit

Bryan

29

Thank you Evan for your comments

Yes, I will implement those tests.

You’re right. I think before since the STM is integrated with the same force models as the main state equation. So, it shall be initialized accordingly.

Bryan

30

Ok I understand better thanks.

Since you’ve asked, I’ll give you my humble and honest opinion. Take it or leave it. It is certainly not my intention to refrain you or the company you represent to contribute to our beloved library.

I won’t talk about your first point, I don’t feel competent enough to talk about the concurrency problem. So, regarding the second point then, I think you have too many, specific constraints to find a satisfying way to manage this natively with Orekit. In other words, your issue should be dealt with at the user application level, i.e. your SK module. I believe that this could be done at least in two ways:

  • as a preprocess to any propagation, by filtering out ConstantThrustManeuver instances with DatebasedManeuverTrigger outside the interval. Similar to what you want to do in your MR inside NumericalPropagator. In Java this is very easily managed with stream/filter/map/forEach. Not sure about Python tho
  • as an adapter, converting back and forth before and after propagations in your module. Basically turning N ConstantThrustManeuver into 1 Maneuver with TimeIntervalsManeuverTrigger and vice versa. Or with ForceModelModifier. It’s actually pretty natural in programming to convert objects when high level layers use lower level ones

Now, if you must modify the way Orekit works, because it is very, very specific, I wouldn’t introduce a generic framework in ForceModel like you propose. Instead, when it comes to summing up the different accelerations in NumericalPropagator, I would filter out the DateBasedManeuverTrigger forces there. This is already more or less how some STM parameters are managed. It’s far from ideal but at least it the user doesn’t see it, it’s not directly callable.

Finally, if you must stick to your idea and add a new method in ForceModel, please don’t forget to overload it in ForceModelModifier, like the other ones with a default implementation.

Cheers,
Romain.

31

After discussing internally, the contribution is therefore cancelled.

Bryan