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

Serialization inaccuracy #72

Open
AbitTheGray opened this issue Dec 25, 2024 · 1 comment
Open

Serialization inaccuracy #72

AbitTheGray opened this issue Dec 25, 2024 · 1 comment

Comments

@AbitTheGray
Copy link
Contributor

There is a problem with serialization to/from stream.
I've started with 0.9 in fpm::fixed_16_16 which is serialized as 0.899994 which is OK but if you de-serialize it, you get 0.899887 making it different by 0.000106812 which are 7 "raw values".
EnableRounding has no effect.

Simple test: https://godbolt.org/z/68qW64v93

Converting to float or double before/after stream serialization "fixes" this problem in this case but it is not a true fix.
Using std::setprecision with std::fixed to make sure the serialization provides all the digits did not help.


Some other values:

Original Serialized Retrieved Raw Difference
1.5 1.5 1.5 0
1.4 1.39999 1.39989 7
1.3 1.3 1.29999 1
1.2 1.2 1.2 0
1.1 1.1 1.09999 1
1.0 1.0 1.0 0
0.9 0.899994 0.899887 7
0.8 0.800003 0.799988 1
0.7 0.699997 0.69989 7
0.6 0.600006 0.59999 1
0.5 0.5 0.5 0
0.4 0.399994 0.399887 7
0.3 0.300003 0.299988 1
0.2 0.199997 0.19989 7
0.1 0.100006 0.0999908 1

I understand that there is inaccuracy by the conversion to/from string but I think difference of 7 is little too big.

If you use std::setprecision and std::fixed, you may get value like 1.1000061035 instead of 1.1 but for the table I went with unmodified std::cout.
If you want to use approach like this, you need string long enough to represent 0.0000152587890625 (1/(2^16)) for 16 fractional bits as it is the epsilon. 0.00390625 for 8 bits and 0.00000005960464477539063 for 24 bits.


Probably linked to #37 as stream serialization is not valid replacement for raw values.

@AbitTheGray
Copy link
Contributor Author

I've traced it to if (divisor > MaxFraction / base) { (line 695 of ios.hpp).

Using fixed_8_8 = fpm::fixed<int16_t, int32_t, 8> for less possible values and 0.9 (0.898438), it reads only 2 digits (fraction=89, divisor=100, MaxFraction=255, base=10) and then decides on the value.
So I've tested highest value below 0.9 (which is the same as fixed_8_8{0.9}, 0.898438) and above 0.89 (0.890625). They are different by 0.0078125 but if you deserialize from those strings, you get 0.886719 (all those decimal values are in fixed_8_8).


Here is a simple test script:

using F = fpm::fixed<int16_t, int32_t, 8>; // fixed_8_8
F v0{0.9};
F v1{0.89};
while(static_cast<double>(v1) < 0.89)
{
	v1 = F::from_raw_value(v1.raw_value() + 1);
}
std::cout << "v0: " << v0 << std::endl;
std::cout << "v1: " << v1 << std::endl;
std::cout << "diff: " << (v0 - v1) << std::endl;
{
	std::stringstream ss("0.898438");
	F result{};
	ss >> result;
	std::cout  << ss.str()<< " -> " << result << std::endl;
}
{
	std::stringstream ss("0.890625");
	F result{};
	ss >> result;
	std::cout << ss.str() << " -> " << result << std::endl;
}

and its result:

v0: 0.898438
v1: 0.890625
diff: 0.0078125
0.898438 -> 0.886719
0.890625 -> 0.886719

To have consistent output, you can use std::setprecision(FractionBits) << std::fixed (for 0.9 it gives 0.89843750 instead of 0.898438).

But with current parsing code ( (fraction << F) / divisor = (89843750 << 8) / 100000000) you won't get the correct result due to it not fitting into IntermediateType after being shifted. It would require re-writing how is significand converted to raw_value or using higher IntermediateTypes (like int64_t for fixed_8_8) - you need a type which fits 10 ^ FractionBits digits (decimal places) and can be shifted left by FractionBits without discarding any digits.

I would like to write a function that does not require the large type but I am not sure how (with good enough performance).
And those types are still limited:

Type digits10
int8_t 2
int16_t 4
int32_t 9
int64_t 18
__int128 38

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