[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

Hi Hank,


Hank Grabowski <hank@applieddefense.com> a écrit :


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?


This is not possible for constructors which are not inherited and also
not possible for return values as methods cannot be overloaded with
only a difference in the return value.


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.
Anyway, the changes do not seem to be too difficult to implement. I have almost finished the changes and still have something that compiles without errors. One 
or two methods to clean up and I will try a first run...

For users who did not implement any measurement type by themselves, it seems
the only change in their code will be of the order of replacing

 Propagator estimated = estimator.estimate();

with

 Propagator estimated = estimator.estimate()[0];
On the other hand, the ability to estimate several orbits at once is an interesting 
feature. We can think about thing like Grace-like missions, or highly
accurate GNSS measurements, forùation flying or constellations.

best regards,
Luc



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