Skip to content

Commit

Permalink
Fix float precision (#649)
Browse files Browse the repository at this point in the history
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<T>::digits10 + 1' with
'std::numeric_limits<T>::max_digits10'.

Background:
Using 'std::numeric_limits<T>::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<T>::max_digits10' has to be used.
  • Loading branch information
SGSSGene authored and jbeder committed Dec 21, 2018
1 parent b659858 commit abf941b
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 49 deletions.
2 changes: 1 addition & 1 deletion include/yaml-cpp/node/convert.h
Expand Up @@ -93,7 +93,7 @@ struct convert<_Null> {
struct convert<type> { \
static Node encode(const type& rhs) { \
std::stringstream stream; \
stream.precision(std::numeric_limits<type>::digits10 + 1); \
stream.precision(std::numeric_limits<type>::max_digits10); \
stream << rhs; \
return Node(stream.str()); \
} \
Expand Down
8 changes: 4 additions & 4 deletions src/emitterstate.cpp
Expand Up @@ -24,8 +24,8 @@ EmitterState::EmitterState()
m_seqFmt.set(Block);
m_mapFmt.set(Block);
m_mapKeyFmt.set(Auto);
m_floatPrecision.set(std::numeric_limits<float>::digits10 + 1);
m_doublePrecision.set(std::numeric_limits<double>::digits10 + 1);
m_floatPrecision.set(std::numeric_limits<float>::max_digits10);
m_doublePrecision.set(std::numeric_limits<double>::max_digits10);
}

EmitterState::~EmitterState() {}
Expand Down Expand Up @@ -349,15 +349,15 @@ bool EmitterState::SetMapKeyFormat(EMITTER_MANIP value, FmtScope::value scope) {
}

bool EmitterState::SetFloatPrecision(std::size_t value, FmtScope::value scope) {
if (value > std::numeric_limits<float>::digits10 + 1)
if (value > std::numeric_limits<float>::max_digits10)
return false;
_Set(m_floatPrecision, value, scope);
return true;
}

bool EmitterState::SetDoublePrecision(std::size_t value,
FmtScope::value scope) {
if (value > std::numeric_limits<double>::digits10 + 1)
if (value > std::numeric_limits<double>::max_digits10)
return false;
_Set(m_doublePrecision, value, scope);
return true;
Expand Down
12 changes: 6 additions & 6 deletions test/integration/emitter_test.cpp
Expand Up @@ -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) {
Expand Down
82 changes: 44 additions & 38 deletions test/node/node_test.cpp
Expand Up @@ -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<float>());
}

TEST(NodeTest, FloatingPrecisionDouble) {
const double x = 0.123456789;
Node node = Node(x);
EXPECT_EQ(x, node.as<double>());
Expand Down Expand Up @@ -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);
}
}
}

0 comments on commit abf941b

Please sign in to comment.