Re: [Orekit Developers] multi-satellite orbit determination API

[Hank Grabowski <hank@applieddefense.com>]

The changes sound fine but if you are concerned about API compatibility could this be addressed with overloaded methods and default method in the interface definition? I have only dabbled with the OD system so am only going on what you wrote in this e-mail so that suggest may be an over simplification. Also, the OD libraries are relatively new and this is a major release so breaking compatibility isn't a deal breaker anyway so it may not be worth that brain damage anyway, but I thought I'd throw that out there.

On Fri, Jun 30, 2017 at 3:44 AM, MAISONOBE Luc <luc.maisonobe@c-s.fr> wrote:

Hi all,

The propagation part of multi-satellite handling has been committed in the
git repository. Now, I need to add multi-satellite orbit determination
before I can release Orekit 9.0 as it will require API changes.

The classes and methods users see are the various measurements constructors,
modifiers constructors, and least squares estimator constructor and
estimate method.

In order to avoid duplicating code, I propose to still use only the
existing classes and have them handle any number of spacecrafts,
including only one. the other option would be to have a set of
one spacecraft classes and another set for n spacecrafts. I don't
really like this second option, we already have too much duplication
in Orekit yet. For the methods signatures that require 1 or n
parameters depending on the case, I propose to switch this n-ary
parameter to the end and use the '...' specifier to allow user
code to either pass only one for mono satellite need, to pass
explicitly two hard-coded parameters for cases like the Grace
mission with exactly two satellites involved, and to pass an array
when either the number is too high or it can vary from run to run
(for example for constellations).

For example, the estimator constructor would be changed from

 BatchLSEstimator(NumericalPropagatorBuilder propagatorBuilder,
                  LeastSquaresOptimizer optimizer)

to

 BatchLSEstimator(LeastSquaresOptimizer optimizer,
                  NumericalPropagatorBuilder... propagatorBuilder)

Another change will be that the estimate method now will return
an array of propagators instead of a single propagator, even
in mono-satellite cases.

These changes affect users, as they at least need to switch arguments
for the constructor and add a [0] at the end of the call to estimate
to retrieve only the first propagator. These are breaking changes so
they can only be introduced now.

The measurement estimation in ObservedMeasurement interface would
be changed from

  EstimatedMeasurement<T> estimate(int iteration, int evaluation,
                                   SpacecraftState state)

to

  EstimatedMeasurement<T> estimate(int iteration, int evaluation,
                                   SpacecraftState... state)

This change does not affect users (at least those who did not create
their own measurements) as a call with one state only compiles
correctly, and indeed I don't end users will call this method
directly, it is intended for internal calls.

For measurements constructors, we need to specify which propagator
it is related to. So I propose to add an integer parameter representing
the index in the NumericalPropagatorBuilder implicit array at the end
of the BatchLSEstimator constructor for representing this. As the
most common cas will still be only one propagator builder for mono-satellite
cases, the existing constructor without an index will still be available
and will call the new constructor with a default index set to 0. With
such a change, existing user code will still work without change.

I will start doing this today, I am already really late for the 9.0
release. So if you disagree with these changes, please complain
quickly.

best regards,
Luc