Skip to content

Conversation

@mborland
Copy link
Member

Closes #794

@libbooze
Copy link

I was just opening a new bug for printing... if you have time can you check if this now works as expected:

template<typename T>
void print_mins()
{
    std::print("{}\n", boost::typeindex::type_id<T>().pretty_name());
    using nl = std::numeric_limits<T>;
    const auto denorm_min = nl::denorm_min();
    const auto denorm_min_mult = denorm_min*T(1'000'000'000'000'000'000);
    const auto min = nl::min();
    std::print("denorm min                        : {:.30e}\n", denorm_min);
    std::print("denorm min multiplied             : {:.30e}\n", denorm_min_mult);
    std::print("min                               : {:.30e}\n\n", min);
}

@mborland
Copy link
Member Author

I was just opening a new bug for printing... if you have time can you check if this now works as expected:

template<typename T>
void print_mins()
{
    std::print("{}\n", boost::typeindex::type_id<T>().pretty_name());
    using nl = std::numeric_limits<T>;
    const auto denorm_min = nl::denorm_min();
    const auto denorm_min_mult = denorm_min*T(1'000'000'000'000'000'000);
    const auto min = nl::min();
    std::print("denorm min                        : {:.30e}\n", denorm_min);
    std::print("denorm min multiplied             : {:.30e}\n", denorm_min_mult);
    std::print("min                               : {:.30e}\n\n", min);
}

For:

std::cout << std::scientific
              << "denorm min: " << std::numeric_limits<decimal32>::denorm_min()
              << "\ndenorm min multiplied: " << (std::numeric_limits<decimal32>::denorm_min() * decimal32(UINT64_C(1'000'000'000'000'000'000)))
              << "\nmin: " << std::numeric_limits<decimal32>::min() << std::endl;

we get:

denorm min: 1.000000e-101
denorm min multiplied: 1.000000e-83
min: 1.000000e-95

Denorms were being treated as non-finite values.

@libbooze
Copy link

we get:

denorm min: 1.000000e-101
denorm min multiplied: 1.000000e-83
min: 1.000000e-95

Denorms were being treated as non-finite values.

Nice, thank you for the fix.

@mborland
Copy link
Member Author

@libbooze I believe I hit all of your comments

static_assert(test_value(std::numeric_limits<decimal128>::denorm_min(), "1.0000000000000000000000000000000000000000e-6176"));
static_assert(test_value(std::numeric_limits<decimal128_fast>::denorm_min(), "1.0000000000000000000000000000000000000000e-6143"));

static_assert(test_value(std::numeric_limits<decimal32>::max() + std::numeric_limits<decimal32>::lowest(), "0.0000000000000000000000000000000000000000e+00"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add same comment(// Lowest + max) as in other preprocessor branch to keep it consistent

@libbooze
Copy link

@libbooze I believe I hit all of your comments

yes, thank you. Left just one comment regarding comment.

@mborland
Copy link
Member Author

@libbooze I believe I hit all of your comments

yes, thank you. Left just one comment regarding comment.

I'll add it. This needs a little more investigation because I think something has gone wrong with fpclassify now. The fuzzer seems like it's consistently hitting an assertion.

@codecov
Copy link

codecov bot commented Jan 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.9%. Comparing base (394daf2) to head (4ae8c77).
Report is 19 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop    #796     +/-   ##
=========================================
+ Coverage     98.9%   98.9%   +0.1%     
=========================================
  Files          226     227      +1     
  Lines        16949   16967     +18     
  Branches      1807    1809      +2     
=========================================
+ Hits         16749   16776     +27     
+ Misses         200     191      -9     
Files with missing lines Coverage Δ
include/boost/decimal/charconv.hpp 89.1% <100.0%> (+2.4%) ⬆️
include/boost/decimal/decimal128.hpp 97.8% <ø> (ø)
include/boost/decimal/decimal128_fast.hpp 98.3% <ø> (ø)
include/boost/decimal/decimal32.hpp 98.6% <ø> (ø)
include/boost/decimal/decimal32_fast.hpp 96.5% <ø> (ø)
include/boost/decimal/decimal64.hpp 98.8% <ø> (ø)
include/boost/decimal/decimal64_fast.hpp 98.2% <ø> (ø)
include/boost/decimal/detail/comparison.hpp 99.4% <100.0%> (+0.1%) ⬆️
test/test_decimal32.cpp 100.0% <ø> (ø)
test/test_fixed_width_trunc.cpp 100.0% <100.0%> (ø)
... and 3 more

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 394daf2...4ae8c77. Read the comment docs.

This was linked to issues Jan 20, 2025
@libbooze
Copy link

Not sure if this is related to this issue, so feel free to not address this in this pull, but I noticed another issue with printing. Just mentioning it in case you might find it useful.
(this is with latest commit 53b0c3d from this branch):

    std::print("{:.5e}\n", std::numeric_limits<decimal32>::max());
    std::print("{:.6e}\n\n", std::numeric_limits<decimal32>::max());
    std::print("{:.14e}\n", std::numeric_limits<decimal64>::max());
    std::print("{:.15e}\n\n", std::numeric_limits<decimal64>::max());
    std::print("{:.32e}\n", std::numeric_limits<decimal128>::max());
    std::print("{:.33e}\n\n", std::numeric_limits<decimal128>::max());

on my machine prints:

9.9999990000000000000000000000000000000000e+96
9.9999999999999990000000000000000000000000e+384
9.9999999999999999999999999999999990000000e+6144

1.000000e+96
9.999999e+96

1.000000000000000e+384
9.999999999999999e+384

1.000000000000000000000000000000000e+6144
9.999999999999999999999999999999999e+6144

@mborland
Copy link
Member Author

mborland commented Jan 20, 2025

Can you open as a separate issue? I think this one has already exceeded its original scope since it wipes out a few issues. I'll have to compare print to regular io streams to see if they're identically wrong. The format support is the newest part of the library and got merged in like a week before review.

@libbooze
Copy link

Can you open as a separate issue? I think this one has already exceeded its original scope since it wipes out a few issues. I'll have to compare print to regular io streams to see if they're identically wrong. The format support is the newest part of the library and got merged in like a week before review.

Sure, but will wait till this is merged so I can test on develop. Easier to track/reproduce then.

@mborland mborland merged commit 069c26a into develop Jan 20, 2025
75 checks passed
@mborland mborland deleted the limits branch January 20, 2025 19:37
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

Successfully merging this pull request may close these issues.

gcc-12 warnings for charconv Incorrect == for decimal64 Incorrect numeric_limits::max

3 participants