From abf941b20d21342cd207df0f8ffe09f41a4d3042 Mon Sep 17 00:00:00 2001 From: Simon Gene Gottlieb Date: Fri, 21 Dec 2018 15:05:19 +0100 Subject: [PATCH] Fix float precision (#649) The issue is that numbers like 2.01 or 3.01 can not be precisely represented with binary floating point numbers. This replaces all occurrences of 'std::numeric_limits::digits10 + 1' with 'std::numeric_limits::max_digits10'. Background: Using 'std::numeric_limits::digits10 + 1' is not precise enough. Converting a 'float' into a 'string' and back to a 'float' will not always produce the original 'float' value. To guarantee that the 'string' representation has sufficient precision the value 'std::numeric_limits::max_digits10' has to be used. --- include/yaml-cpp/node/convert.h | 2 +- src/emitterstate.cpp | 8 +-- test/integration/emitter_test.cpp | 12 ++--- test/node/node_test.cpp | 82 +++++++++++++++++-------------- 4 files changed, 55 insertions(+), 49 deletions(-) diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 45a878a..b8210c7 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -93,7 +93,7 @@ struct convert<_Null> { struct convert { \ static Node encode(const type& rhs) { \ std::stringstream stream; \ - stream.precision(std::numeric_limits::digits10 + 1); \ + stream.precision(std::numeric_limits::max_digits10); \ stream << rhs; \ return Node(stream.str()); \ } \ diff --git a/src/emitterstate.cpp b/src/emitterstate.cpp index 3542aaf..d5f39e7 100644 --- a/src/emitterstate.cpp +++ b/src/emitterstate.cpp @@ -24,8 +24,8 @@ EmitterState::EmitterState() m_seqFmt.set(Block); m_mapFmt.set(Block); m_mapKeyFmt.set(Auto); - m_floatPrecision.set(std::numeric_limits::digits10 + 1); - m_doublePrecision.set(std::numeric_limits::digits10 + 1); + m_floatPrecision.set(std::numeric_limits::max_digits10); + m_doublePrecision.set(std::numeric_limits::max_digits10); } EmitterState::~EmitterState() {} @@ -349,7 +349,7 @@ bool EmitterState::SetMapKeyFormat(EMITTER_MANIP value, FmtScope::value scope) { } bool EmitterState::SetFloatPrecision(std::size_t value, FmtScope::value scope) { - if (value > std::numeric_limits::digits10 + 1) + if (value > std::numeric_limits::max_digits10) return false; _Set(m_floatPrecision, value, scope); return true; @@ -357,7 +357,7 @@ bool EmitterState::SetFloatPrecision(std::size_t value, FmtScope::value scope) { bool EmitterState::SetDoublePrecision(std::size_t value, FmtScope::value scope) { - if (value > std::numeric_limits::digits10 + 1) + if (value > std::numeric_limits::max_digits10) return false; _Set(m_doublePrecision, value, scope); return true; diff --git a/test/integration/emitter_test.cpp b/test/integration/emitter_test.cpp index 2780838..dfd4f34 100644 --- a/test/integration/emitter_test.cpp +++ b/test/integration/emitter_test.cpp @@ -901,18 +901,18 @@ TEST_F(EmitterTest, SingleChar) { TEST_F(EmitterTest, DefaultPrecision) { out << BeginSeq; - out << 1.234f; - out << 3.14159265358979; + out << 1.3125f; + out << 1.23455810546875; out << EndSeq; - ExpectEmit("- 1.234\n- 3.14159265358979"); + ExpectEmit("- 1.3125\n- 1.23455810546875"); } TEST_F(EmitterTest, SetPrecision) { out << BeginSeq; - out << FloatPrecision(3) << 1.234f; - out << DoublePrecision(6) << 3.14159265358979; + out << FloatPrecision(3) << 1.3125f; + out << DoublePrecision(6) << 1.23455810546875; out << EndSeq; - ExpectEmit("- 1.23\n- 3.14159"); + ExpectEmit("- 1.31\n- 1.23456"); } TEST_F(EmitterTest, DashInBlockContext) { diff --git a/test/node/node_test.cpp b/test/node/node_test.cpp index 61ba3e6..18234db 100644 --- a/test/node/node_test.cpp +++ b/test/node/node_test.cpp @@ -391,7 +391,13 @@ TEST(NodeTest, AutoBoolConversion) { EXPECT_TRUE(!!node["foo"]); } -TEST(NodeTest, FloatingPrecision) { +TEST(NodeTest, FloatingPrecisionFloat) { + const float x = 0.123456789; + Node node = Node(x); + EXPECT_EQ(x, node.as()); +} + +TEST(NodeTest, FloatingPrecisionDouble) { const double x = 0.123456789; Node node = Node(x); EXPECT_EQ(x, node.as()); @@ -452,108 +458,108 @@ class NodeEmitterTest : public ::testing::Test { TEST_F(NodeEmitterTest, SimpleFlowSeqNode) { Node node; node.SetStyle(EmitterStyle::Flow); - node.push_back(1.01); - node.push_back(2.01); - node.push_back(3.01); + node.push_back(1.5); + node.push_back(2.25); + node.push_back(3.125); - ExpectOutput("[1.01, 2.01, 3.01]", node); + ExpectOutput("[1.5, 2.25, 3.125]", node); } TEST_F(NodeEmitterTest, NestFlowSeqNode) { Node node, cell0, cell1; - cell0.push_back(1.01); - cell0.push_back(2.01); - cell0.push_back(3.01); + cell0.push_back(1.5); + cell0.push_back(2.25); + cell0.push_back(3.125); - cell1.push_back(4.01); - cell1.push_back(5.01); - cell1.push_back(6.01); + cell1.push_back(4.5); + cell1.push_back(5.25); + cell1.push_back(6.125); node.SetStyle(EmitterStyle::Flow); node.push_back(cell0); node.push_back(cell1); - ExpectOutput("[[1.01, 2.01, 3.01], [4.01, 5.01, 6.01]]", node); + ExpectOutput("[[1.5, 2.25, 3.125], [4.5, 5.25, 6.125]]", node); } TEST_F(NodeEmitterTest, MixBlockFlowSeqNode) { Node node, cell0, cell1; cell0.SetStyle(EmitterStyle::Flow); - cell0.push_back(1.01); - cell0.push_back(2.01); - cell0.push_back(3.01); + cell0.push_back(1.5); + cell0.push_back(2.25); + cell0.push_back(3.125); - cell1.push_back(4.01); - cell1.push_back(5.01); - cell1.push_back(6.01); + cell1.push_back(4.5); + cell1.push_back(5.25); + cell1.push_back(6.125); node.SetStyle(EmitterStyle::Block); node.push_back(cell0); node.push_back(cell1); - ExpectOutput("- [1.01, 2.01, 3.01]\n-\n - 4.01\n - 5.01\n - 6.01", node); + ExpectOutput("- [1.5, 2.25, 3.125]\n-\n - 4.5\n - 5.25\n - 6.125", node); } TEST_F(NodeEmitterTest, NestBlockFlowMapListNode) { Node node, mapNode, blockNode; - node.push_back(1.01); - node.push_back(2.01); - node.push_back(3.01); + node.push_back(1.5); + node.push_back(2.25); + node.push_back(3.125); mapNode.SetStyle(EmitterStyle::Flow); mapNode["position"] = node; - blockNode.push_back(1.01); + blockNode.push_back(1.0625); blockNode.push_back(mapNode); - ExpectOutput("- 1.01\n- {position: [1.01, 2.01, 3.01]}", blockNode); + ExpectOutput("- 1.0625\n- {position: [1.5, 2.25, 3.125]}", blockNode); } TEST_F(NodeEmitterTest, NestBlockMixMapListNode) { Node node, mapNode, blockNode; - node.push_back(1.01); - node.push_back(2.01); - node.push_back(3.01); + node.push_back(1.5); + node.push_back(2.25); + node.push_back(3.125); mapNode.SetStyle(EmitterStyle::Flow); mapNode["position"] = node; - blockNode["scalar"] = 1.01; + blockNode["scalar"] = 1.0625; blockNode["object"] = mapNode; ExpectAnyOutput(blockNode, - "scalar: 1.01\nobject: {position: [1.01, 2.01, 3.01]}", - "object: {position: [1.01, 2.01, 3.01]}\nscalar: 1.01"); + "scalar: 1.0625\nobject: {position: [1.5, 2.25, 3.125]}", + "object: {position: [1.5, 2.25, 3.125]}\nscalar: 1.5"); } TEST_F(NodeEmitterTest, NestBlockMapListNode) { Node node, mapNode; - node.push_back(1.01); - node.push_back(2.01); - node.push_back(3.01); + node.push_back(1.5); + node.push_back(2.25); + node.push_back(3.125); mapNode.SetStyle(EmitterStyle::Block); mapNode["position"] = node; - ExpectOutput("position:\n - 1.01\n - 2.01\n - 3.01", mapNode); + ExpectOutput("position:\n - 1.5\n - 2.25\n - 3.125", mapNode); } TEST_F(NodeEmitterTest, NestFlowMapListNode) { Node node, mapNode; - node.push_back(1.01); - node.push_back(2.01); - node.push_back(3.01); + node.push_back(1.5); + node.push_back(2.25); + node.push_back(3.125); mapNode.SetStyle(EmitterStyle::Flow); mapNode["position"] = node; - ExpectOutput("{position: [1.01, 2.01, 3.01]}", mapNode); + ExpectOutput("{position: [1.5, 2.25, 3.125]}", mapNode); } } }