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

test_abieos_template fails on arm8 builds #614

Closed
spoonincode opened this issue Jul 6, 2022 · 7 comments · Fixed by AntelopeIO/leap#437
Closed

test_abieos_template fails on arm8 builds #614

spoonincode opened this issue Jul 6, 2022 · 7 comments · Fixed by AntelopeIO/leap#437
Assignees

Comments

@spoonincode
Copy link
Member

terminate called after throwing an instance of 'std::runtime_error'
  what():  Expected number or boolean
@spoonincode
Copy link
Member Author

The problem appears to be that when a float is serialized to JSON via fpconv_dtoa(), the string ends up having more decimal places than strtof() likes when read back from JSON causing an ERANGE error.

@spoonincode
Copy link
Member Author

More specifically, the test wants to ensure it can make a round trip between binary, json, and the native types. And it tests at the extremities for some of these types; float being the problematic one here.

An example is std::numeric_limits<float>::min(). Somehow this value is getting written out as 1.1754943508222875e-38, but then later when strtof() is later used to convert the string back to a native type, it gets flagged as an overflow and errors out with ERANGE. The minimum float after all is only 1.17549e-38.

It's not clear where this "extension" is occurring. I thought it may be due to the double promotion before calling fpconv_dtoa(). But, it must be more subtle. Indeed something like

#include <iostream>
#include <limits>

int main() {
	std::cout << (double)std::numeric_limits<float>::min() << std::endl;
	return 0;
}

just prints 1.17549e-38. It could be something more subtle in fpconv_dtoa().

@swatanabe
Copy link
Contributor

Promotion from float to double should be exact. The problem is almost certainly a result of differences in rounding. The conversions from binary to decimal and decimal to binary are both inexact. fpconv_dtoa is platform independent, so the difference is probably in the implementation of strtof.

@swatanabe
Copy link
Contributor

The extra digits definitely appear because of using fpconv_dtoa. iostreams does not print all the digits by default, so that doesn't mean anything. Note that "all the digits" is a bit vague anyway. The exact value of FLOAT_MIN is 1.1754943508222875079687365372222456778186655567720875215087517062784172594547271728515625e-38. Usually, we just keep enough digits to round-trip accurately. 6 decimal places is not enough to round-trip float (24-bit mantissa).

@swatanabe
Copy link
Contributor

I think the only solution here is to replace strtof.

@spoonincode
Copy link
Member Author

Yes, fwiw

--- a/include/eosio/from_json.hpp
+++ b/include/eosio/from_json.hpp
@@ -485,7 +485,7 @@ void from_json(float& result, S& stream) {
    std::string s(sv); // strtof expects a null-terminated string
    errno = 0;
    char* end;
-   result = std::strtof(s.c_str(), &end);
+   result = std::strtod(s.c_str(), &end);
    check( !errno && end == s.c_str() + s.size(),
          convert_json_error(from_json_error::expected_number) );
 }

Is passing on both ARM8 & x86. Any downsides to this?

@swatanabe
Copy link
Contributor

Double rounding can increase the overall rounding error if the value being parsed was not originally a single-precision float. Also, I suspect that strtod may still have issues with denormalized values (though only for subnormal doubles, not subnormal floats).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
2 participants