Stripping down ObservableSatellite?

Orekit development
1

I am opening a new topic following the last comments from Remote clock model in OneWayGNSSRangeBuilder.

The ObservableSatellite class was added in 2019 (commit 735d5e4), in order to associate clock offsets to propagation, for the sake of GNSS measurements. It has evolved several times since then, adding clock drift, then clock acceleration, then quadratic clock models, then name.

As the topic referenced above states, this design clearly still has serious flaws. Clocks are really core elements in the GNSS world, and the quadratic model that is broadcast in navigation messages is in fact not really useful at the level of precision we are dealing with in Orekit. These models correspond to post-processing and are intended for end users (i.e. your smartphone or car navigation system), when Orekit either generates measurements or process them, it has to deal with much more irregular clocks, with offsets that jump all over the place. The quadratic is good for meter level positioning, not for centimeter level positioning.

This is the reason why, over the last few months, I have added the ClockModel interface, with several implementations (QuadraticClockModel, obviously, despite its limitations, but also AggregatedClockModel, AbstractCombinedClocksPair, ClocksSum, ClocksDifference, SampledClockModel, ConstantClockModel, PerfectClockModel). These are the classes that real GNSS users need.

So all the improvements in ObservableSatellite are now worthless, and I think we should strip down the class and remove anything related to clocks there. We should also move the code related to clock offsets in all GNSS measurements out of the measurements themselves, and set up dedicated modifiers that take generic ClockModel instances (one for emitter, one for receiver), hence allowing the users much more flexibility than the crude worthless quadratic model.

I initially thought we should completely ditch the ObservableSatellite class and go back to simple integer parameter representing the index of the propagator in multi-satellite cases. This would however not meet all needs, as we also associated a name with the propagator, and we use that to retrieve the proper ambiguity in the AmbiguityCache for phase measurements. So perhaps ObservableSatellite could remain, but as just an association of a propagator index with a name.

What do you think?

2

Hi @luc , thank you for opening this topic, I think that this discussion is very interesting. I am a newcomer in Orekit, so my opinion is not so relevant, but from my perspective ObservableSatelliteis still an important class (at least for the purpose that I have in mind). It is used in most of the interfaces that concern propagation and observables (as all the detectors for generation in the measurements scheduler). I guess that if you deleted completely the class, then you would carry out a remarkable refactoring of the source code. Thus, maybe, it seems better to remove the part about the clock as you were mentioning and move the clock model as a pure modifier. I was looking also at the classes about clock from Rinex files and, following your suggestion, I guess that I should handle it as a custom modifier which I have to implement from scratch (but I still have to check all the details in the documentation). Thank you very much for your support.

3

@baubin has initiated a massive refactoring of the measurement system, so now is probably a good time to do this as well

4

" So perhaps ObservableSatellite could remain, but as just an association of a propagator index with a name."

This is sort of what I did in my MR. ObservableSatellite still has a clock, but I broke the class inheritance down so that the MeasurementObject parent class that holds the clock is separate from ObservableSatellite, which is now basically just a propagator index / name as you say. GroundStation also inherits its clock from this parent class, and I created a new class Satellite that serves the same function as GroundStation for the OneWayGNSS measurement classes.

I think your basic idea about the clocks is good, but since the clocks are always going to be attached to either the Satellite observer, the GroundStation observer, or the ObservableSatellite measured object, what I would suggest we do instead is have MeasurementObject contain an abstract parent ClockModel class which can be supplied by the user when he creates ObservableSatellite or Satellite or GroundStation, and then that clock and its measurement paramters will be read in by the class. If the user does not want to create a clock, we can have a default constructor that still creates a QuadraticClockModel for them internally. Thoughts?

P.S. @Serrof and I discussed my original MR, and we agreed that while a restructuring of the type I’ve been working on has legitimate reasons to be extremely large, it would also be nearly impossible to vet properly. As a result, we’ve agreed to break the MR down into pieces and integrate them one at a time. So I’ll close the massive MR and instead put up an MR for my first section, which actually fortuitously deals with this issue anyway and is purely a code structural change that does not affect the code’s external interface in any way.

5

No, this would not be sufficient as a single measurement object could have several different clocks. This is typically the case for GNSS satellites as the clock generated on different signals (L1, E1, E5a, E5b…) are slightly offset to each other (which is why there are Differential Signal Bias files), and sometimes there are clock combinations (ionosphere-free).

6

A list of clocks then?

7

It depends on context. At work, I had to set up a dedicated mapping (what I called clock semantic) in order to known what each clock really represents. This semantic could be that a clock is aligned with the satellite system master clock, or it is a iono-free clock and references two signals, or it is a mono-frequency clock and is linked to an absolute bias, but I guess other different usages will come up (for example when dealing with composite clocks that reference other clocks, with possible changes in the clocks set as clocks may wander too far and be kicked out of the set).

However, as offsets are almost constants on a time scale of one or a few days, associating one reference clock with one measurement object as you propose is still feasible, providing we could supply the offset (as a ClockModel itself, most probably a ConstantClockModel) to be added to the reference clock when building/processing each measurement. This clock should really be labelled as a reference, or as a basis form something more accurate. This is what I do at work, I just ask myself if this solution is the best one.

8

Well if you’d like to take a look at what I did, the MR is up. The ClockModel is not a user input now, except for the Satellite class. But it could easily be made one.

9

Also just an FYI - most of the measurement classes seem to only look at clock offset. But RangeRate actually looks at clock drift.

if (!isTwoWay()) {
            // clock drifts, taken in account only in case of one way
            final ObservableSatellite satellite    = getSatellites().get(0);
            final double              dtsDot       = satellite.getClockDriftDriver().getValue(transitState.getDate());
            final double              dtgDot       = getStation().getClockDriftDriver().getValue(stationPV.getDate());

            final double clockDriftBiais = (dtgDot - dtsDot) * Constants.SPEED_OF_LIGHT;

            rangeRate = rangeRate + clockDriftBiais;
        }

A ConstantClockModel or generic ClockModel wouldn’t work here.

10

ClockModel.getOffset(date) returns a ClockOffset, which contains both an offset, a rate and and acceleration. Yes, this is weird, an offset within an offset, my bad.

1 Like
12

Sorry for the previous post. I looked at the code some more and reconsidered.

One thing I’m worried about though is the ParameterDrivers in QuadraticClockModel for offset/drift/acceleration. If the measurement classes accepted a ClockModel instead of a QuadraticClockModel, there would be no code-internal automated way to pull the ParameterDrivers since they’re only native to the quadratic clock.

13

You are right, but I am not sure the clock model should do that. What about putting the parameter driver at the clock modifier level, to be added to the clock model?

14

Another suggestion: keeping the parameter drivers native to quadratic model only but add a getParametersDrivers getter at the interface level. The sampled/aggregated clock models would return an empty list, the constant clock model would return a singleton, the sum/difference would return a combination of the underlying models.

15

That was essentially the solution I thought of as well, except right now QuadraticClockModel is the only clock that seems to have parameter drivers - ConstantClockModel only stores the offset as a double. Want me to add it to the MR?

16

If you think you can keep it small enough to not overload the MR, yes. It could also deserve a dedicated MR.

17

What we’re doing right now is adding the MR in pieces to keep the MR size sane and reviewable. How about you take a look at the part 1 I’ve got up now (I CCd you) and let me know what you think. If you like it, we can add part 1 to the issue-1745 branch and then I’ll update the clock structure as part 2.

18

OK, I’ll do that in the upcoming days.
It is night time on this side of the Atlantic ocean and I had a long day. I will logout from my computer now. :yawning_face:

1 Like
19

@luc after taking a look at this and doing some preliminary coding, I actually think I’d prefer to make it a separate MR. Adjusting the clocks is proving a little trickier than I thought it would be initially, and my MR is pretty complex already.

20

Fine.
I’ll look at your MR probably during this week.