Handle no data in SARCalibration
Issue description
Current version of SARCalibration (v9.0) doesn't permit to distinguish nodata pixels from pixels whose physical value after noise removal drops to 0.
"Starting with IPF 2.90, the no-value pixels at image borders are set to 0 value by the IPF. The masking of the borders thus does not require specific processing."
In S1Tiling, a workaround has been implemented to make sure nodata on the sides
stays at 0, while pixel noise-corrected to 0 are fixed to a
LOWER_SIGNAL_VALUE
(= 1e-7 by default).
See s1-tiling/s1tiling#87 (closed) and
s1-tiling/s1tiling#159 (closed)
Supporting those kinds of nodata situations directly in SARCalibration would simplify S1TIling workflow, but also the workflow of many other users.
Possible solutions
About nodata
Given S1 products, we could just continue to assume nodata to always be 0.
But... may be, it'd be better to have a -nodata
parameter defaulted to 0. I
don't know.
LOWER_SIGNAL_VALUE
About IMO, we should just force the final output to LOWER_SIGNAL_VALUE
option, when
set (it has to be optional to keep the previous behaviour as the default
behaviour), and when -removenoise
is True.
Possible implementation
SarRadiometricCalibrationFunction<TInputImage, TCoordRep>::EvaluateAtIndex(const IndexType& index) const
{
...
const std::complex<float> pVal = this->GetInputImage()->GetPixel(index);
// as long as the nodata value is 0, it will be simple with complex numbers
// that's why I'm not sure we need to handle cases like nodata = -32768 ...
if (pVal == m_nodata) return m_nodata;
const RealType digitalNumber2 = (pVal.real() * pVal.real()) + (pVal.imag() * pVal.imag());
// why taking the sqrt of the previous value to multiply it by itself? ...
// see a future Issue I'll open.
...
if (m_ApplyLookupDataCorrection)
{
if (m_EnableNoise && m_NoiseLut)
{
sigma = otb::quiet::max(0., sigma - m_NoiseLut->GetValue(index[0], index[1]));
// See next section about NaN's regarding otb::quiet::max.
// Given lower_signal_value option is set, and result would have been 0.0
if (sigma == 0. && m_using_lower_signal_value)
return m_lower_signal_value;
// no need to handle NaN values as further computations will continue to
// return NaNs... But we could also guard the function for this case.
}
RealType lutVal = static_cast<RealType>(m_Lut->GetValue(index[0], index[1]));
sigma /= lutVal * lutVal;
}
...
About NaN values
Given peculiar handling of NaN numbers in C and C++ (1), we tried to use
BandMath to replace all 0's from an original S1 image with NaNs, thanks to
exp='im1b1==im1b1?im1b1:0./0.
(See
#1798 (comment 110682))
SARCalibration without noise removal keeps the NaN on the sides (which is what we were looking for).
However as soon as noiseremoval
is requested, NaN values are replaced with 0's.
This is because of the following:
// otbSarRadiometricCalibrationFunction.hxx
// SarRadiometricCalibrationFunction<>::EvaluateAtIndex {...
if (m_EnableNoise && m_NoiseLut)
{
sigma = std::max(0., sigma - m_NoiseLut->GetValue(index[0], index[1]));
}
Indeed with current implementation of std::max
in GNU libstdc++,
std::max(whatever, nan)
will always return whatever
. On the contrary,
std::max(nan, whatever)
will always return nan.
// current implementation of std::max in libstdc++
max(const _Tp& __a, const _Tp& __b)
{
if (__a < __b) // always false when a or b is NaN
return __b;
return __a;
}
Also, it seems the behaviour of std::max(nan, ...)
is undefined ("UB").
In consequence, we have to take control over max() definition to guarantee a
defined behaviour when NaN values are passed.
=> recommendation
See: https://godbolt.org/z/633Y5ojT7
max
without UB on NaN values
1. Define a flavour of // The following is strongly inspired from
// Walter E. Brown - Correctly calculating min, max and more - Meeting C++ online
// https://www.youtube.com/watch?v=V1U6AXEgNbE
// PS: I've used lambdas instead of functions to simplify the definitions.
// Inline (!!!) functions would be perfect.
namespace quiet {
auto out_of_order = [](double a, double b) {
// std::isless has a guaranteed non signalling behaviour when NaN values are passed
return std::isless(b, a);
};
auto max = [](double a, double b) {
return out_of_order(a, b) ? a : b;
};
}
2. Update SARCalibration code
// Replace
sigma = std::max(0., sigma - m_NoiseLut->GetValue(index[0], index[1]));
// With
sigma = otb::quiet::max(0., sigma - m_NoiseLut->GetValue(index[0], index[1]));
Note: we shall not change parameters orders from the moment max() is "correctly" implemented.
Or...
Or... we could simply guard the function. Still, this NaN related issue may appear in other OTB filters and applications -- we have never tried to correctly handle NaN values in OTB as an alternate nodata value.
So. Instead of
if (pVal == m_nodata) return m_nodata;
we could guard the function with:
if (std::isnan(pVal) || pVal == m_nodata) return pVal;
(1) What's so special about NaN
- Arithmetic functions on nan's will return nan
- comparisons on NaN will always return
false
, but they may also raiseFE_INVALID
; this is whystd::isless
can be preferred.