Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit integer to floating point conversion triggers warning #111

Open
ossama-othman opened this issue Aug 24, 2020 · 2 comments
Open
Assignees
Labels

Comments

@ossama-othman
Copy link
Owner

Describe the bug
Building MaRC with clang++ 10 or better triggers a warning diagnostic at compile-time related to implicit conversion of a 64 bit integer (int64_t) to a double floating point value:

In file included from ../lib/marc/scale_and_offset.h:15:
../lib/marc/details/scale_and_offset.h:120:40: warning: implicit conversion
      from 'const long' to 'double' changes value from 9223372036854775807
      to 9223372036854775808 [-Wimplicit-int-float-conversion]
                else if (max * scale > T_max)
                                     ~ ^~~~~
In file included from ../lib/marc/extrema.h:15:
../lib/marc/Map_traits.h:106:41: warning: implicit conversion from 'long' to
      'const double' changes value from 9223372036854775807 to
      9223372036854775808 [-Wimplicit-int-float-conversion]
            constexpr double type_max = std::numeric_limits<T>::max();
                             ~~~~~~~~   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Scale_Offset_Test.cpp:38:33: warning: implicit conversion from 'long' to
      'const double' changes value from 9223372036854775807 to
      9223372036854775808 [-Wimplicit-int-float-conversion]
    constexpr double T_max    = std::numeric_limits<T>::max();
                     ~~~~~      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The crux of the problem is that MaRC "reads" all data from the source images as double values. This isn't a problem for 8, 16, and 32 bit integers since their maximum values can be stored in a 64 bit double without loss of precision since the significand of a double value has enough bits to store it (e.g. IEEE 754 double is 53 bits wide). However, that isn't the case for 64 bit integers since the largest 64 bit value is obviously won't fit into the smaller double type signficand (e.g. 53 bits).

To Reproduce
Steps to reproduce the behavior:

  1. ./configure CXX=clang++
  2. make check

Expected behavior
No warning related to implicit conversion of an integer to a floating point value should be triggered.

Desktop (please complete the following information):

  • OS: Linux
  • Compiler: clang++ 10
@ossama-othman ossama-othman added this to the MaRC 1.0 Release milestone Aug 24, 2020
@ossama-othman
Copy link
Owner Author

Related bug documented in lib/marc/map_parameters.h:

        /**
         * @brief Maximum valid physical value.
         *
         * @note This value corresponds to the %FITS "DATAMAX"
         *       keyword.
         *
         * @bug On platforms that implement the IEEE 754 floating
         *      point standard, the use of @c double as the underlying
         *      @c DATAMAX type will cause loss of precision if the
         *      %FITS @c DATAMAX values is an integer that requires
         *      more than 53 bits since the significand of a 64 bit
         *      IEEE 754 floating point value is only 53 bits wide.
         *      Special handling will be need to be implemented to
         *      handle such a case.
         */
        MaRC::optional<double> datamax_;

@ossama-othman
Copy link
Owner Author

The warning could be silenced by using an explicit cast, but that would only hide the problem in this case. Ideally the code should be changed so that no cast is necessary.

@ossama-othman ossama-othman self-assigned this Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant