From 50bc3b144dae4d6bc3571160b670cd2756bc15cf Mon Sep 17 00:00:00 2001 From: Liam Keegan Date: Fri, 24 Nov 2023 12:16:54 +0100 Subject: [PATCH] Make double parsing locale independent (and faster) - use `std::from_chars` instead of `strtod` ro parse doubles if available (c++17 required) - otherwise falls back to previous `strtod` implementation - `std::from_chars` is locale independent and faster than `strtod` - add more double parsing tests - add double parsing google benchmarks - resolves #1566 --- benchmarks/parsing_google.cpp | 44 +++++++++++++++++++++++++++ symengine/parser/parser.cpp | 25 +++++++++++++-- symengine/tests/basic/test_parser.cpp | 21 +++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/benchmarks/parsing_google.cpp b/benchmarks/parsing_google.cpp index 25ffe8b969..3baac591bb 100644 --- a/benchmarks/parsing_google.cpp +++ b/benchmarks/parsing_google.cpp @@ -12,6 +12,45 @@ static void parse_0(benchmark::State &state) for (auto _ : state) { a = parse("0"); } + benchmark::DoNotOptimize(a); +} + +static void parse_double(benchmark::State &state) +{ + std::string text = "-.123e-11"; + RCP a; + for (auto _ : state) { + a = parse(text); + } + benchmark::DoNotOptimize(a); +} + +static void parse_many_doubles(benchmark::State &state) +{ + std::string text; + for (int i = 0; i < state.range(0); i++) { + text.append("+24.45645 - 0.9834e-2 - 23.243 + 0.058263e-11"); + } + RCP a; + for (auto _ : state) { + a = parse(text); + } + benchmark::DoNotOptimize(a); + state.SetComplexityN(4 * state.range(0)); +} + +static void parse_many_doubles_implicit_mul(benchmark::State &state) +{ + std::string text; + for (int i = 0; i < state.range(0); i++) { + text.append("+24.45645x - 0.9834e-2y - 23.243z + 0.058263e-11w"); + } + RCP a; + for (auto _ : state) { + a = parse(text); + } + benchmark::DoNotOptimize(a); + state.SetComplexityN(4 * state.range(0)); } static void parse_long_expr1(benchmark::State &state) @@ -25,6 +64,7 @@ static void parse_long_expr1(benchmark::State &state) for (auto _ : state) { a = parse(text); } + benchmark::DoNotOptimize(a); state.SetComplexityN(state.range(0)); } @@ -39,10 +79,14 @@ static void parse_long_expr2(benchmark::State &state) for (auto _ : state) { a = parse(text); } + benchmark::DoNotOptimize(a); state.SetComplexityN(state.range(0)); } BENCHMARK(parse_0); +BENCHMARK(parse_double); +BENCHMARK(parse_many_doubles)->Range(1, 4096)->Complexity(); +BENCHMARK(parse_many_doubles_implicit_mul)->Range(1, 4096)->Complexity(); BENCHMARK(parse_long_expr1)->Range(2, 4096)->Complexity(); BENCHMARK(parse_long_expr2)->Range(2, 4096)->Complexity(); diff --git a/symengine/parser/parser.cpp b/symengine/parser/parser.cpp index 902124a899..aca512b27d 100644 --- a/symengine/parser/parser.cpp +++ b/symengine/parser/parser.cpp @@ -4,10 +4,29 @@ #include #include #include +#if ((defined(_MSVC_LANG) && _MSVC_LANG >= 201703L) || __cplusplus >= 201703L) +#include +#endif namespace SymEngine { +static double string_to_double(const char *str, char **endptr) +{ +#if ((defined(_MSVC_LANG) && _MSVC_LANG >= 201703L) || __cplusplus >= 201703L) + // use std::from_chars: locale-independent, non-allocating, non-throwing + double result{0.0}; + auto [ptr, ec] = std::from_chars(str, nullptr, result); + if (endptr != nullptr) { + *endptr = const_cast(ptr); + } + return result; +#else + // otherwise fall-back to using strtod + return std::strtod(str, endptr); +#endif +} + RCP parse(const std::string &s, bool convert_xor, const std::map> &constants) @@ -311,7 +330,7 @@ RCP Parser::parse_numeric(const std::string &expr) } if (digits <= 15) { char *endptr = 0; - double d = std::strtod(startptr, &endptr); + double d = string_to_double(startptr, &endptr); return real_double(d); } else { // mpmath.libmp.libmpf.dps_to_prec @@ -321,7 +340,7 @@ RCP Parser::parse_numeric(const std::string &expr) } #else char *endptr = 0; - double d = std::strtod(startptr, &endptr); + double d = string_to_double(startptr, &endptr); return real_double(d); #endif } @@ -332,7 +351,7 @@ Parser::parse_implicit_mul(const std::string &expr) { const char *startptr = expr.c_str(); char *endptr = 0; - std::strtod(startptr, &endptr); + string_to_double(startptr, &endptr); RCP num = one, sym; diff --git a/symengine/tests/basic/test_parser.cpp b/symengine/tests/basic/test_parser.cpp index 5622db53d6..5567a1632d 100644 --- a/symengine/tests/basic/test_parser.cpp +++ b/symengine/tests/basic/test_parser.cpp @@ -807,6 +807,11 @@ TEST_CASE("Parsing: doubles", "[parser]") REQUIRE(eq(*res, *real_double(1.324))); REQUIRE(eq(*res, *parse(res->__str__()))); + s = "+1.324"; + res = parse(s); + REQUIRE(eq(*res, *real_double(1.324))); + REQUIRE(eq(*res, *parse(res->__str__()))); + s = "0.0324*x + 2*3"; res = parse(s); REQUIRE(eq(*res, *add(mul(real_double(0.0324), x), integer(6)))); @@ -817,6 +822,22 @@ TEST_CASE("Parsing: doubles", "[parser]") REQUIRE(eq(*res, *add(mul(real_double(0.0324), x), integer(6)))); REQUIRE(eq(*res, *parse(res->__str__()))); + s = " +0.324e-19x+4.238"; + res = parse(s); + CAPTURE(res->__str__()); + REQUIRE(eq(*res, *add(mul(real_double(0.324e-19), x), real_double(4.238)))); + REQUIRE(eq(*res, *parse(res->__str__()))); + + s = ".32485123e-21"; + res = parse(s); + REQUIRE(eq(*res, *real_double(0.32485123e-21))); + REQUIRE(eq(*res, *parse(res->__str__()))); + + s = "1345.35e13"; + res = parse(s); + REQUIRE(eq(*res, *real_double(1345.35e13))); + REQUIRE(eq(*res, *parse(res->__str__()))); + s = "1.324/(2+3)"; res = parse(s); REQUIRE(is_a(*res));