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

Enabling Grisu conversion generates different output #1033

Closed
DanielaE opened this issue Feb 7, 2019 · 8 comments
Closed

Enabling Grisu conversion generates different output #1033

DanielaE opened this issue Feb 7, 2019 · 8 comments

Comments

@DanielaE
Copy link
Contributor

DanielaE commented Feb 7, 2019

Commit 355eb6d switched the Grisu conversion algorithm active for shortest roundtrip (default) formatting. Alas, I'm a bit puzzled about the outcome compared to the snprintf-based conversion: it delivers different outcomes.

Consider this test code:

#include <fmt/core.h>
#include <string>
#include <charconv>
#include <array>

template <typename T>
std::string toChars(T Value) {
  std::array<char, 32> Buffer;
  auto b = &Buffer.front();
  auto e = &Buffer.back() + 1;
  return {b, std::to_chars(b, e, Value).ptr};
}

int main() {
  const double doublePi = 3.14159265358979;
  const float floatPi = doublePi;

  auto GrisuF = fmt::format("{}", floatPi);
  auto GrisuD = fmt::format("{}", doublePi);
  auto ToChrF = toChars(floatPi);
  auto ToChrD = toChars(doublePi);
}

My observed outcome:

              Debugger       snprintf      Grisu           std::to_chars
doublePi 3.1415926535897900  3.14159  3.14159265358979    3.14159265358979
floatPi	 3.14159274          3.14159  3.1415927410125732  3.1415927

Is this as designed?

grisu-test-float

@vitaut
Copy link
Contributor

vitaut commented Feb 8, 2019

Grisu2 with 11 bits of extra precision which is what {fmt} implements gives the shortest output in 99.5% of cases. 3.1415927410125732 is still correct just not the shortest (it is actually shortest, see comment below). I plan to switch to Grisu3 and add a fallback for the remaining 0.5%, but in the meantime you can either disable Grisu by defining FMT_USE_GRISU to 0 or using the g specifier which will give the same result as before i.e. the default precision of 6 like printf.

@DanielaE
Copy link
Contributor Author

DanielaE commented Feb 8, 2019

To be clear, I totally understand what's going on here and don't doubt the correctness of the conversion. My point was only if this is the output as designed. I got confused from my observation of the ranges test failing on my machine with Grisu-based conversion active since the commit mentioned above. There are two test cases in ranges-test.cc which excercise the float values from my snippet code above in pair and tuple and compare the string result with "3.14159". This will fail for obvious reasons. Nobody seems to complain ....

@vitaut
Copy link
Contributor

vitaut commented Feb 8, 2019

My point was only if this is the output as designed.

Yes, this is as designed, but will be improved by the next release.

@Boddlnagg
Copy link

I plan to switch to Grisu3

Have you considerey Ryu? According to their benchmarks, it's still significantly faster. Also, Microsoft chose Ryu for their implementation of std::to_chars: https://www.reddit.com/r/cpp/comments/a2mpaj/how_to_use_the_newest_c_string_conversion/eazo82q/

@vitaut
Copy link
Contributor

vitaut commented Feb 15, 2019

Ryu is only moderately faster according to a benchmark made by Stephan Lavavej (70-90% IIRC compared to their highly optimized version of Ryu). This might seem like a lot but note that Milo Yip's version of Grisu is 9x faster than sprintf (not 90%) and there is still plenty of room for improvement. Also it only compares low level formatting routines while integration with higher level formatting is the most interesting part. So I plan to finish integration of Grisu since the algorithm is already implemented and possibly look into Ryu in the future, because I don't expect it provide dramatic perf improvements.

@vitaut
Copy link
Contributor

vitaut commented Apr 3, 2019

I ran dtoa-benchmark with Ruy and the difference is about 30% not 70-90% from somewhat optimized Grisu2 (Grisu3 will be slightly slower, but not too much, because less that 0.5% need fallback). More importantly, Ruy is slower on short output which is the opposite of what we want.
Screen Shot 2019-04-03 at 3 34 18 PM

@vitaut
Copy link
Contributor

vitaut commented Apr 20, 2019

Regarding the original question, 3.1415927410125732 is actually the correct shortest representation that round trips through double:

  auto pi = static_cast<float>(M_PI);
  auto r1 = atof("3.1415927");
  assert(r1 == pi); // fails
  auto r2 = atof("3.1415927410125732");
  assert(r2 == pi); // succeeds

@vitaut
Copy link
Contributor

vitaut commented Sep 13, 2020

float is not promoted to double as of #1360:

  const double doublePi = 3.14159265358979;
  const float floatPi = doublePi;
  fmt::print("{}\n", floatPi);
  fmt::print("{}\n", doublePi);

prints

3.1415927
3.14159265358979

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

3 participants