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

RE: [Orekit Users] Upgrading to Orekit v9.0 : how to handle the Fields ?



Thank you Luc for this fast and detailed answer.
We will attempt to implement both methods in all our AttitudeProviders, applying your suggestions for unit tests, and see where it leads us.

Kind regards,
Yannick


-----Original Message-----
From: orekit-users-request@orekit.org [mailto:orekit-users-request@orekit.org] On Behalf Of MAISONOBE Luc
Sent: Wednesday, August 30, 2017 8:02 PM
To: orekit-users@orekit.org
Subject: Re: [Orekit Users] Upgrading to Orekit v9.0 : how to handle the Fields ?


"JEANDROZ, Yannick" <yannick.jeandroz@airbus.com> a écrit :

> Hello,

Hi Yannick,

>
>
>
> First, many thanks for the release of Orekit v9.0. There is  
> obviously a tremendous amount of work behind it. Some of the new  
> features look very appealing to us.
>
> We are currently in the process of switching our existing code base  
> to v9.0. Most of this process has been painless so far : the few  
> modifications that we had to perform in order to solve compilation  
> errors were very intuitive. Good job on maintaining compatibility  
> despite significant improvements !

Thanks for the kind words, this is greatly appreciated.

>
> However, we have hit a difficulty with our custom AttitudeProviders.  
> Since commit 73f7d313 they are expected to implement a method  
> getAttitude() that works with Fields. We were not familiar with this  
> Fields feature, but it seems extremely powerful. In order to benefit  
> from the new features, we have to refactor our code, and that is  
> fine. In particular, it seems that Fields are now required for orbit  
> determination.

Yes, the fields are mandatory for orbit determination. They are not
limited to this use case, though, for example they can be used for
Taylor algebra, which itself is useful for high order uncertainties
propagation or Monte-Carlo simulation.

>
> However, it seems that Fields have been implemented in Orekit v9.0  
> as a kind of add-on. They do not replace existing methods, probably  
> to avoid breaking compatibility ? This means that the  
> AttitudeProviders have to implement two getAttitude() methods, the  
> regular one and the new Fields one. We have attempted to implement  
> this without code duplication by implementing the Fields method,  
> then rewriting the regular method as a kind of wrapper around the  
> Fields method. We have failed so far. Looking at Orekit  
> AttitudeProviders, it seems that code duplication could not be  
> avoided there either.

Yes, the two methods are both required. This allows for example to
use the same force model in both a regular propagation, an
orbit determination, or a Taylor algebra computation.

I would not recommend using the field version underneath the double
version, as this may impair the performances. If this is not a concern
to you, then you can do that, using Decimal64 as the field because
it is simply a wrapped double.

>
> If the two implementations of getAttitude() are completely separate,  
> I am afraid that bugs could lead to different behaviors between  
> "regular" propagation, and orbit determination. Those bugs would  
> certainly be tricky to isolate.

Yes. In order to mitigate this, care must be taken in the unit tests.
For Orekit itself, we regularly use test cases that use the Decimal64
field and compare the results with double. Beware that the results
will not be strictly identical as the optimizing compiler may order the
operations differently in both cases. Consider for example the code
needed to add three values. With fields, you may write it as
a.add(b).add(c) and this enforces b is added to a first and then c is
added. With doubles, you write it as a + b + c and the compiler is free
to compile it as (a + b) + c, or a + (b + c) or even as (a + c) + b,
mainly depending on what values are already in the registers. The
result may be different in the last few bits due to rounding, and if
the result is in an iterative loop, the few bits errors may diverge
afterwards.

>
>
>
> Did we understand this evolution properly, or are we missing something here ?

I think you understood the evolution perfectly.

>
> Is there a long-term plan for a tighter integration of the Fields ?  
> Maybe by replacing existing classes with Field-based ones ? Knowing  
> this could allow us to plan our refactoring accordingly.

I do have ideas about a tighter implementation, but it really needs
maturing (probably at least one or two years of tinkering with various
options). This would alo involve a major effort to set up the tooling.

So do not expect a seamless integration before that. In the
meantime, we will have to rely on manual maintenance of the consistency.
This is not ideal, but there are some technical challenges and limitations
due to the Java language (no operator overloading, no overriding of methods
base on the return type only, type erasure in generics...)

best regards,
Luc

>
>
>
> Thank you very much for your input.
>
>
>
> Yannick
>
>
>
> ***************************************************************
> Ce courriel (incluant ses eventuelles pieces jointes) peut contenir  
> des informations confidentielles et/ou protegees ou dont la  
> diffusion est restreinte. Si vous avez recu ce courriel par erreur,  
> vous ne devez ni le copier, ni l'utiliser, ni en divulguer le  
> contenu a quiconque. Merci d'en avertir immediatement l'expediteur  
> et d'effacer ce courriel de votre systeme. Airbus Defence and Space  
> et les sociétés Airbus Group declinent toute responsabilite en cas  
> de corruption par virus, d'alteration ou de falsification de ce  
> courriel lors de sa transmission par voie electronique.
> This email (including any attachments) may contain confidential  
> and/or privileged information or information otherwise protected  
> from disclosure. If you are not the intended recipient, please  
> notify the sender immediately, do not copy this message or any  
> attachments and do not use it for any purpose or disclose its  
> content to any person, but delete this message and any attachments  
> from your system. Airbus Defence and Space and Airbus Group  
> companies disclaim any and all liability if this email transmission  
> was virus corrupted, altered or falsified.
> ---------------------------------------------------------------------
> Airbus Defence and Space SAS (393 341 516 RCS Toulouse) - Capital:  
> 29.821.072 EUR - Siege social: 31 rue des Cosmonautes, ZI du Palays,  
> 31402 Toulouse cedex 4, France



This mail has originated outside your organization, either from an external partner or the Global Internet.
Keep this in mind if you answer this message.




***************************************************************
Ce courriel (incluant ses eventuelles pieces jointes) peut contenir des informations confidentielles et/ou protegees ou dont la diffusion est restreinte. Si vous avez recu ce courriel par erreur, vous ne devez ni le copier, ni l'utiliser, ni en divulguer le contenu a quiconque. Merci d'en avertir immediatement l'expediteur et d'effacer ce courriel de votre systeme. Airbus Defence and Space et les sociétés Airbus Group declinent toute responsabilite en cas de corruption par virus, d'alteration ou de falsification de ce courriel lors de sa transmission par voie electronique.
This email (including any attachments) may contain confidential and/or privileged information or information otherwise protected from disclosure. If you are not the intended recipient, please notify the sender immediately, do not copy this message or any attachments and do not use it for any purpose or disclose its content to any person, but delete this message and any attachments from your system. Airbus Defence and Space and Airbus Group companies disclaim any and all liability if this email transmission was virus corrupted, altered or falsified. 
---------------------------------------------------------------------
Airbus Defence and Space SAS (393 341 516 RCS Toulouse) - Capital: 29.821.072 EUR - Siege social: 31 rue des Cosmonautes, ZI du Palays, 31402 Toulouse cedex 4, France