I am currently working on issue 1332. Doing so, I take the opportunity to redesign some parts that are lacking, like adding proper quadratic clock models, making sure all coefficients of clock models can be estimated and are used…
I stumbled upon an ambiguity on ambiguities
It seems to me Phase, OneWayGNSSPhase and InterSatellitesPhase do have their own ParameterDriver for ambiguities, but at the same time there are PhaseAmbiguityModifier, OneWayGNSSPhaseAmbiguityModifier and InterSatellitesPhaseAmbiguityModifier that add different parameter drivers, with some key to separate various ambiguities.
I guess we should only have one way to specify ambiguities, either in the base measurements or as modifiers. For sure, ambiguities are mandatory for phase measurements, so I wonder if we should remove the modifiers and keep only the ones in the base measurements or if we should keep the modifiers.
The first option, including the ambiguities in the measurements is logical from a theoretical point of view, but it may add a large overhead as thousands of parameters drivers will be created and linked together in the infamous ParametersDriversList.DelegatingDriver. The second option put the burden of adding the modifier to users, but it allows finer control, which indeed is necessary to handle signal loss of lock. So the second option is the one I would prefer.
I’ll trust your logic since I too would like to avoid dealing with issues in the infamous ParametersDriversList
Would there be a way to add a “default” modifier at construction?
I know it’s not the way modifiers should be dealt with but if it’s justified and properly documented, it looks ok to me.
I am not sure about a default modifier and it would introduce again some overhead as we would build one internally. Another point is what would we do if users add an explicit modifier later on?
I was thinking about deprecating the getAmbiguityDriver methods, temporarily replacing them by a search loop for the driver in the modifiers list, so users get an exception if they did not add a modifier by themselves. The idea would be they build a small number of modifiers (one for each pair transmitter/receiver) and set up time spans in these modifiers to deal with loss of lock.
I think I found some middle ground.
The idea would still be to deprecate getAmbiguityDriver and have it temporarily loop over the drivers from the modifiers, but if the loop fails (i.e. if getAmbiguityDriver is called before the modifier has been set up, then it would lazily add one.
So ambiguities would always be handled by a modifier, either one explicitly set by user or one added on the fly.
I will open an issue for this change
I finally changed my mind.
I found a way to keep the ambiguity managed directly at the phase measurements level, as it is really intrinsic to this kind of measurements and cannot be ignored.
So I will deprecate the upper level modifiers.
The trick was to set up a cache that contains all ambiguity drivers built so far, using the names of the emitter and receiver and the wavelength as a key. This way, we only build the minimum number of parameter drivers we need. When the measurements are built, they have the cache as a new parameter, so if another measurement with the same triplet emitter/receiver/wavelength has already been built we will just recover the same parameter driver, hence we avoid cluttering the ParameterDriversList. For compatibility purposes, a singleton default cache has been temporarily set up, it will be removed in Orekit 13.0.
