Skip to content
GitLab
Closed
Issue created by Luc Hermitte@lhermitteContributor

Improper management of NaN in `GetParameterFloat()`

Description

GetParameterFloat("somefloatparameter") will return 3.40282e+38 (std::numeric_limit<float>::max()) instead of nan when an OTB application is executed with -somefloatparameter NaN.

Steps to reproduce

Run otbcli_OrthoRectification -io.in sigma0nan.tif -io.out ortho10nan.tiff -map utm -map.utm.zone 31 -map.utm.northhem 1 -outputs.mode autosize -outputs.spacingx 10. -outputs.spacingy -10. -opt.gridspacing 40 -outputs.default NaN

In the logs, we will see that the output default value will be 3.40282e+38 instead of the expected NaN.

Use case

There is a real use case for this in S1TIling. After calibration, removed noise will be set to 0 which is not to be confused with nodata extra pixel added outside the rectified image. A really efficient and simple way to distinguish some specific values, from nodata values that shall not be taken into account in the orthorectification would be to use nan as the nodata value.

See s1-tiling/s1tiling#87 (closed)

Unfortunately, OTB doesn't permit to have application parameters set to nan.

Technical origin of the issue.

The bug originates:

  • from a property of nan values: whatever the comparison, the result will always be false. For instance, with auto d = nan;, then nan != nan and nan == nan will always be false!.
  • and the incorrect clamping algorithm used in otb::NumericalParameter::SetValue().

Instead of

m_Value = (value < m_MinimumValue) ? m_MinimumValue : (value < m_MaximumValue) ? value : m_MaximumValue;

it should be

m_Value = (value < m_MinimumValue) ? m_MinimumValue : (m_MaximumValue < value) ? m_MaximumValue : value;

or even better just:

#include "otbAlgoClamp.h"
...
m_Value = otb::clamp(value, m_MinimumValue, m_MaximumValue); // std::clamp in C++17

See the following demonstration on godbolt: https://godbolt.org/z/3jGMK1crb

Discussion

This will not make OTB magically compatible with NaN values. Some applications will need to specifically take nan into account to support it (was we cannot compare with if (val == some_nan_value)). Yet, it'll be a start to correctly take these "numbers" into account in a few applications, and into new applications that could take advantage of nan properties.