Error in derivatives calculation wrt to station parameters in atmospheric modifiers

Hello everyone,

during the implementation of a custom measurement class and its modifiers, I noticed that the derivative of the modification wrt station parameters is always zero. To illustrate the problem I’ll take the RangeModifierUtil and BaseRangeTroposphericDelayModifier as an example.

From my understanding in RangeModifierUtil the derivatives of the modifier wrt station parameters are calculated using finite differences and added to the unmodified derivative like this:

if (driver.isSelected()) {
    // update estimated derivatives with derivative of the modification wrt station parameters
    double parameterDerivative = estimated.getParameterDerivatives(driver)[0];
    parameterDerivative += Differentiation.differentiate(d -> modelEffect.evaluate(station, state),
    3,10.0 * driver.getScale()).value(driver);
    estimated.setParameterDerivatives(driver, parameterDerivative);
    }

The modelEffect in this example is the rangeErrorTroposphericModel function of the BaseRangeTroposphericDelayModifier class.
However, I believe that this function does not consider station parameters as intended since for the calculation of the elevation following is used

final double elevation  = station.getBaseFrame().getElevation(position,
    state.getFrame(),state.getDate());

From the documentation and my testing, it seems that getBaseFrame does return the base frame of the station without considering e.g. station offsets from ParameterDrivers. Thus this function is constant and ultimately its derivative is always zero.

This is of course not a major problem since the impact is most likely negligible but I’d like to hear your thoughts on this.

Greetings,

Benedikt

Hi @BS_space,

Welcome to the forum!

That is true.
I think you found a bug here, thank you for that.
(I’m not 100% sure because I’m no expert in the classes you’ve dived into)
Could you please open an issue on Orekit forge?

Thanks!
Maxime

I agree, that’s a bug. Good catch! It looks to be a bit difficult to solve…
This also concerns the Ionospheric delay as well as all station-based measurement types. But fortunately, the fix is only in one class :slight_smile: .

Could you provide a unit test presenting the theoritical derivatives vs the ones computed by Orekit?

Thanks
Bryan

Hi all,

I remember looking at observation models a while ago and I think the station offset parameters were often skipped in computations. So this might hide a bigger issue than just one modifier class unfortunately. This qualifies for a patch release on version 12.0 I would say.

Best,
Romain.

Hi all,
thanks a lot for your feedback!

Could you please open an issue on Orekit forge 1?

Yes of course!

So this might hide a bigger issue than just one modifier class unfortunately.

Yes, I totally agree. The RangeModifier was taken just as an example. However, in the measurement models (the theoreticalEvaluation, not the modifiers), I think this issue is already overcome by using the getOffsetToInertial method which does account for station parameters and uses automatic differentiation.
I believe that the derivatives wrt to station parameters in the modifiers should ultimately also be computed with automatic differentiation rather than finite differences.
However, I have no idea how difficult this is to implement :wink:

On the other hand, sticking to finite differences might also require a bit more work and touching more files. E.g. the interface for the tropospheric and ionospheric models differ a bit in the sense that the ionospheric one only allows for pathDelay computation based on a TopocentricFrame aka a BaseFrame rather than from elevation directly. Yet maybe this is still the easiest solution e.g. by providing some function in the Groundstation class that returns the OffsetFrame as a TopocentricFrame?!.

These are just my thoughts on this and I might be totally off here :smiley:

Could you provide a unit test presenting the theoretical derivatives vs the ones computed by Orekit?

Yes, I can. Unfortunately, I am quite busy at the moment and I might have to come back to this later this year. This would be also done in the same way normal merge requests are handled I assume?

Greetings,
Benedikt

Good point. Because getOffsetToInertial takes a lot of time, we could also have a signature of getOffsetToInertial returning a StaticTransform instead of Transform. It is to my mind a 2nd issue linked to yours.