Issue with parallelized computation involving an orbit propagator

Hello !

I’m trying to perfom some parallelized computations (for station keeping computations) involving an orbit propagator (an integrated ephemeris for faster computations).
The orbit propagator is the same for each thread since I just parallelized the search loop for the station-keeping solution. Something like:

  1. Propagate the orbit during the station keeping cycle duration
  2. Compute orbital delta between propagated orbit at the end of cycle and nominal (i.e., mission) orbit
  3. Search loop to compute SK manoeuvers (with reuse of the IntegratedEphemeris computed for the propagator at step 1.)

However, I’m having the following issue:

java.util.ConcurrentModificationException
at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1597)
at java.base/java.util.HashMap$EntryIterator.next(HashMap.java:1630)
at java.base/java.util.HashMap$EntryIterator.next(HashMap.java:1628)
at org.orekit.propagation.AbstractPropagator.updateUnmanagedData(AbstractPropagator.java:209)
at org.orekit.propagation.AbstractPropagator.updateAdditionalData(AbstractPropagator.java:228)
at org.orekit.propagation.integration.IntegratedEphemeris.updateAdditionalData(IntegratedEphemeris.java:274)
at org.orekit.propagation.analytical.AbstractAnalyticalPropagator.propagate(AbstractAnalyticalPropagator.java:135)
at org.orekit.propagation.AbstractPropagator.propagate(AbstractPropagator.java:293)

It looks like the handling of unmanagedStates in AbstractPropagator class is not thread safe. I really think it is because of the use of HashMap object [1].

What do you think? Could we use a ConcurrentHashMap or synchronize the accesses of unmanagedStates?

I cannot copy or clone the propagator to have one different instance per thread since the method is not availble in Orekit.

Best regards,

Bryan

[1] : https://stackoverflow.com/questions/2688629/is-a-hashmap-thread-safe-for-different-keys

You are right unmanagedStates is not thread safe, but I doubt is should be.
The content of this map is cleared and reset when initializePropagation is run, i.e. for each propagator.
So I think we should add a method to copy or clone the propagator if there is no way to do it for now.

Perhaps you could use PropagatorBuilder in your case to really create a new instance from scratch for each iteration in the search loop?

Thank you for your reply!

Maybe we could just add a public method to get or clear the unmanagedStates and additionalDataProviders to let the users clear them if they want ?

In my case, I don’t need them (but they are added automatically since the code is a generic propagation function that automatically add the additional equations if for instance some tanks are in the database)

This would not prevent the propagate method to call initializePropagation which would then iterate over the unmanaged states, with possible concurrent access.

I totally agree with @luc that it would be better to create new instances than to make the class concurrent.

Using concurrent patterns slows computations down (as well as complicates the code) whether you are making use of concurrency or not, and should only be used when absolutely necessary (such as a shared cache between threads).

We use OREKIT server-side with many threads doing different things on demand, we do not want to have to start a new executable for each task to secure a reliable result.

We get around the shared cached by using thread-local for the data context, and keeping thread concurrency patterns contained to this area would get my vote. It would also potentially be readily swapped out for non-concurrent implementations should performance demand their development.