Skip to content

[#71] Fix rounding issues in format_time_minutes #72

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.

Sign up for GitHub

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

Open
Giles314 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[#71] Fix rounding issues in format_time_minutes #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9af7bbb
[#71] Fix rounding issues that make the tests fail
Giles314 Aug 4, 2024
0af8721
[#71] Add TC to validate 2nd part of fix and confirm that the 2nd bug…
Giles314 Aug 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 39 additions & 13 deletions pv/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,42 @@ static QString pad_number(unsigned int number, int length)

QString format_time_minutes(const Timestamp& t, signed precision, bool sign)
{
const Timestamp whole_seconds = floor(abs(t));
const Timestamp days = floor(whole_seconds / (60 * 60 * 24));
const unsigned int hours = fmod(whole_seconds / (60 * 60), 24).convert_to<uint>();
const unsigned int minutes = fmod(whole_seconds / 60, 60).convert_to<uint>();
const unsigned int seconds = fmod(whole_seconds, 60).convert_to<uint>();
// Do integer computation to avoid various rounding risks in Boost::multiprecision
// In addition it will be faster especially when using multiplication and substraction instead of modulo.
// Unsigned long are garantied to hold at least 32 bits, more than 100 years of seconds. Should be sufficient for a capture...
unsigned long whole_seconds = floor(abs(t)).convert_to<unsigned long>();

int carry = 0; //-- Carry when rounding: it is counted a carry when 0.9999... is represented 1.00...

ostringstream ss;
string fs;

if (precision > 0) {
// Ensure we are substracting the result of rounding done above
// in case Boost abs and/or floor would produce unstable results when working with integers
Timestamp fraction = abs(t) - whole_seconds;
if (fraction < 0.0) {
fraction = 0.0;
}

ss << fixed << setprecision(precision) << setfill('0') << fraction;
fs = ss.str();

// Check whether stringification rounding has produced a carry
if (fs.at(0) == '1') {
carry = 1;
}
}

// Take into the carry for representing the integer value
whole_seconds += carry;
const unsigned long days = std::floor(whole_seconds / (60 * 60 * 24));
unsigned long remain_seconds = whole_seconds - days * (60 * 60 * 24);
// unsigned int (with at least 16 bits) is sufficient for remaining parts
const unsigned int hours = (unsigned int)std::floor(remain_seconds / (60 * 60));
remain_seconds -= hours * (60 * 60);
const unsigned int minutes = (unsigned int)std::floor(remain_seconds / 60);
const unsigned int seconds = (unsigned int)(remain_seconds - minutes * 60);

QString s;
QTextStream ts(&s);
Expand All @@ -217,7 +248,7 @@ QString format_time_minutes(const Timestamp& t, signed precision, bool sign)

// DD
if (days) {
ts << days.str().c_str() << ":";
ts << days << ":";
use_padding = true;
}

Expand All @@ -235,15 +266,10 @@ QString format_time_minutes(const Timestamp& t, signed precision, bool sign)
// SS
ts << pad_number(seconds, 2);

if (precision) {
if (precision > 0) {
// Format the fraction part
ts << ".";

const Timestamp fraction = fabs(t) - whole_seconds;

ostringstream ss;
ss << fixed << setprecision(precision) << setfill('0') << fraction;
string fs = ss.str();

// Copy all digits, inserting spaces as unit separators
for (int i = 1; i <= precision; i++) {
// Start at index 2 to skip the "0." at the beginning
Expand Down
1 change: 1 addition & 0 deletions test/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ BOOST_AUTO_TEST_CASE(format_time_minutes_test)
BOOST_CHECK_EQUAL(fmt(ts("10641906.007008009"), 1), "+123:04:05:06.0");
BOOST_CHECK_EQUAL(fmt(ts("10641906.007008009"), 2), "+123:04:05:06.01");
BOOST_CHECK_EQUAL(fmt(ts("10641906.007008009"), 3), "+123:04:05:06.007");
BOOST_CHECK_EQUAL(fmt(ts("10641906.999900000"), 3), "+123:04:05:07.000");
BOOST_CHECK_EQUAL(fmt(ts("10641906.007008009"), 4), "+123:04:05:06.007 0");
BOOST_CHECK_EQUAL(fmt(ts("10641906.007008009"), 5), "+123:04:05:06.007 01");
BOOST_CHECK_EQUAL(fmt(ts("10641906.007008009"), 6), "+123:04:05:06.007 008");
Expand Down