I am working on tropospheric models these days and find our current design lacking in several places.
The first one concerns the Saastamoinen model. The javadoc states it is a modified Saastamoinen with reference to Guochang Xu, GPS - Theory, Algorithms and Applications, Springer, 2007, which is correct. It would however be better in my humble opininion to call it ModifiedSaastamoinen and add a CanonicalSaastamoinen that implements the original paper and is in line with other existing references like for example navipedia, with a denominator in \cos 2\varphi and no \delta R term.
The second one is that many models are built using temperature, pressure and humidity as fixed parameters whereas latitude and longitude are allowed to vary as the geodetic point is in the pathDelay signature. This is despite we also have GPT and GPT2 models that allow temperature to be time varying.
The third concern is that is seems awkward that in GPT and GPT2, it is the latitude and longitude that are expected to be constant as they are in the models constructors, this is inconsistent with the other tropospheric models.
I would like to introduce a new architecture. My current idea is to have a container for pressure, temperature and humidity (probably as a water partial pressure rather than a relative humidity), to have a provider or converter interface that converts relative humidity to water partial pressure), to have a provider interface that provides pressure temperature and hygrometry based on location and time (GPT and GPT2 would probably implement that too). The tropospheric models would then be built from these providers and converters instead of fixed parameters.
I totally agree, I always thought the fixed atmospheric conditions were a “flaw” in Orekit.
If you keep “Saastamoinen” for the old one (the modified version) and just add CanonicalSaastamoinen, it can maybe be added to a minor version of Orekit.
+1 But I agree with @MaximeJ remark. Maybe we could add a boolean or an interface (with by default the δR term for non regression) to SaastamoinenModel in order to limit API change and propose both implementations in a single class?
That’s a mistake, the latitude/longitude shall be in the weatherParameters() signature. First because they can change and secondly because one can have the need to estimate them.
An interface is a good idea. Note that there is already one and I think it is better to improve it instead of introducing a new one.
There is another change besides \delta R: the denominator, and the constructors are already complex and some take a DataProviderManager just for the sake of loading the \delta R, adding more complexity seems odd to me, so I would prefer another class.
The WeatherModel interface seems odd to me, as it implies internal state is updated (it returns void), so it prevents being used in multi-threaded contexts. A different interface would allow putting these changes in a minor version, changing the existing interface signature can be done only in a major version (I don’t think it is possible to use a default implementation for the new methods, but honestly I did not check).