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

Conversion from std::chrono::duration<double> to units::seconds_t is lossy. #291

Open
BenFrantzDale opened this issue Aug 16, 2022 · 1 comment

Comments

@BenFrantzDale
Copy link

Please include the following information in your issue:

  1. Using ea6d126 (Sun Oct 11 06:46:02 2020 +0800)
  2. Apple clang version 13.1.6 (clang-1316.0.21.2.5)

Related to #290

Conversion from std::chrono::duration<double> to units::seconds_t goes through std::chrono::nanoseconds, so it's inexact.

@BenFrantzDale
Copy link
Author

I think changing the c'tor to this addresses the issue:

      template<class Rep, class Period, 
               class = std::enable_if_t<std::is_arithmetic<Rep>::value && traits::is_ratio<Period>::value>>
      inline constexpr unit_t(const std::chrono::duration<Rep, Period>& value) noexcept :
          nls(units::convert<unit<Period, category::time_unit>, Units>(static_cast<T>(std::chrono::duration_cast<std::chrono::duration<Rep, Period>>(value).count())))
        {
        }

although that may open a hole for egregious precision issues when using unit_t with integral value_type. Perhaps this conversion should be allowed if std::is_floating_point_v<value_type> and otherwise only if the ratio between unit_t's ratio and Period is less than std::ratio<1> (i.e., only implicit integer conversion if unit_t is finer-grained.)

But really, I just want to allow implicit conversion when the std::chrono::duration representation is exactly the same as unit_t.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant