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

Re: [Orekit Users] Re. how to deletgate Orbit class??




"Schatzman, James" <Schatzman-James@n-ask.com> a écrit :

Hi Jim,


Well, it does not seem that I am likely to persuade successfully, but I will try one more time to clarify. Propagators return

You are on the way to convince, don't worry, but as Evan wrote, there
are many side effects as orbit is heavily used and the number of implementations
is currently fixed.

SpacecraftState objects, and those objects in turn have Orbit objects as members.
[private final Orbit orbit;]

If I am to keep using code that uses Propagators, then I have to alter the behavior of those Propagators. IMHO, the cleanest way to do this is to wrap the underlying Orbit so as to modify its behavior. Because of internal calls in Orbit, this cannot be done as cleanly as I would like.


One problem I see is that orbit instances are not only created by the
calling applications (which would know about the extended orbit class),
but they are built by the propagators are many places, and there is
some tricky logic to know what type of orbit to create. If you look
for example at the numerical propagator, it can build any type of orbit
as specified when calling setOrbitType before starting the propagation.
The internal class OsculatingMapper uses this orbit type to know what
orbit to create (KeplerianOrbit, CircularOrbit, EquinoctialOrbit, CartesianOrbit).
Other propagators have other logics (some create fixed orbit types,
some rely on the initial orbit to create the propagated orbit...).

So if you add a new orbit type, you could pass it to the propagator,
but the orbit you would retrieve after propagation would not be the
orbit type you want (except for KeplerianPropagator which relies on the
shifted by method of the initial orbit, so you could override this method
and get your own type here).

Having the propagator aware of your new orbit type would therefore imply
to either:
  - add a new entry in OrbitType
  - change the mapping mechanism to a factory method within orbit
    and not within orbitType

The former is difficult because OrbitType is an enumerate and you cannot
extend an enumerate, so adding an entry implies changing the base enumerate
itself, i.e. adding a type in Orekit.

So even if changing Orbit to an interface (and inserting AbstractOrbit between
the interface and the current classes) would partly solve your problem,
I don't think it is a complete solution to your needs.


If SpacecraftState objects contained PVCoordinatesProviders instead of Orbit objects, or some other interface implemented by Orbit (e.g., interface XXX extends TimeStamped, TimeShiftable<XXX>, TimeInterpolable<XXX>, Serializable, PVCoordinatesProvider) ,the problem would go away. It is just a question of how the "orbit" member variable in SpacecraftState is declared.

It is also a problem of how it is built, i.e. is the factory
for this member aware of the correct type to build.


There is a similar issue with the attitude variable in SpacecraftState and the Attitude class. Fortunately, for now I am foregoing adding biases to my attitudes.

I have solved the problem for now by using "smelly" inheritance.

I am surprised you solved your problem due to this building issue.
Did this work for all propagators? Isn't KeplerianPropagator the
only one that does not really care on the orbit type (because it
delegates to the orbit itself)?

best regards,
Luc

Thanks for all the suggestions.

Jim

________________________________________
From: orekit-users-request@orekit.org [orekit-users-request@orekit.org] on behalf of Ward, Evan [Evan.Ward@nrl.navy.mil]
Sent: Thursday, January 25, 2018 9:00 AM
To: orekit-users@orekit.org
Subject: Re: [Orekit Users] Re. how to deletgate Orbit class??

Jim,

On Thu, 2018-01-25 at 14:13 +0000, Schatzman, James wrote:

Thanks for the suggestions. Yes, Propagator is more convenient to work with than Orbit, because it is an interface. However, the Propagator.propagate methods return SpacecraftState objects. The problem is the SpacecraftState class which uses an Orbit object to do all its work.


So return the biased state. Something like:

Orbit truth = ...;
return new SpacecraftState(bias.apply(truth));

A similar construct would work when implementing a PVCP.



Unfortunately, just writing a new class that implements PVCoordinatesProvider would be insufficient, because of the SpacecraftState issue.

Why? PVCP returns a PVC not a SpacecraftState.


So... I would like to post a request for a future enhancement, that Orbit be converted into an interface.


That would be a significant change to the design of the library. Creating a sublcass of Orbit is specifically prevented because the class is designed to work with only the subclasses that currently exist. Before making extensive changes of this kind it is good to have a well defined use case and show that the existing features of the library do not meet the need. Specifically, why do you need to subclass Orbit? If you can explain this I may be able to suggest other features in Orekit that will better serve your needs.

An Orbit is a set of orbital elements at an instant in time. Your subclass does not fit this definition so under the current design it should not be an Orbit. From what I understand of your use case at this point it seems like a good opportunity to apply Joshua Bloch's "Effective Java" Item #16: Favor composition over inheritance.



I think that I have made it work as is, subclassing Orbit and overriding everything in Orbit with delegation so that the base class is non-functional (except for the constructor).


This is a code smell that usually indicates inheritance was the wrong design choice.

Best Regards,
Evan



Jim


________________________________________
From: orekit-users-request@orekit.org<mailto:orekit-users-request@orekit.org> [orekit-users-request@orekit.org<mailto:orekit-users-request@orekit.org>] on behalf of Ward, Evan [Evan.Ward@nrl.navy.mil<mailto:Evan.Ward@nrl.navy.mil>]
Sent: Thursday, January 25, 2018 6:09 AM
To: orekit-users@orekit.org<mailto:orekit-users@orekit.org>
Subject: Re: [Orekit Users] Re. how to deletgate Orbit class??

Hi Jim,

Welcome to the Orekit mailing lists.

On Wed, 2018-01-24 at 23:02 +0100, schatzman-james@n-ask.com<mailto:schatzman-james@n-ask.com> wrote:

There are some errors in the previous code. The methods that return
PVCoordinates need to be to fixed to called the apply() method in BiasModel to
apply the biases, and the methods that take otherFrame/outputFrame make this
slightly harder. Oh well!


It looks like you're only applying the bias in the getPVCoordinates method. Have you considered extending PVCoordinatesProvider instead of Orbit? The Orbit class hierarchy is used to represent different types of orbital elements at a single instant in time, though they can be either osculating or mean elements. From my limited understanding of your problem I think you would be better served implementing a PVCP or extending a Propagator if you need event detection and step handlers. Is there a reason it needs to be an Orbit?

Best Regards,
Evan



So... I can fix that, but I am still concerned about the issues of the
constructor and the protected methods. I could tweak the Orekit source code.
Maybe that would work. I have made some other tweaks but updating my own
version of Orekit as the official version gets updated is a pain.

I am still hoping that there is some way to do this in virgin Orekit. Any
suggestions?

Thanks!

Jim