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

Two problems about the get_components function #170

Open
HenryLin2000 opened this issue Nov 24, 2023 · 3 comments
Open

Two problems about the get_components function #170

HenryLin2000 opened this issue Nov 24, 2023 · 3 comments

Comments

@HenryLin2000
Copy link

HenryLin2000 commented Nov 24, 2023

I found two problems with this code in lines 599-602 of the get_components function in printf.c. And I hope to discuss with you.

printf/src/printf/printf.c

Lines 599 to 602 in f8ed5a9

else if ((remainder == 0.5) && ((number_.fractional == 0U) || (number_.fractional & 1U))) {
// if halfway, round up if odd OR if last digit is 0
++number_.fractional;
}

(1)Is there a lack of carry processing when the fractional rounding up?
For example, 0.995 takes 2 decimal places, and the decimal place results in 100, which exceeds the limit of 2 significant digits, but there is no carry to the integer, which will cause an exception.
I noticed that in the previous if block, for the case of remainder>0.5, carry processing is done when the decimal digit exceeds the significant digit. Should there be a similar processing here?

  if (remainder > 0.5) {
    ++number_.fractional;
    // handle rollover, e.g. case 0.99 with precision 1 is 1.0
    if ((double) number_.fractional >= powers_of_10[precision]) {
      number_.fractional = 0;
      ++number_.integral;
    }
  }

(2)What is the function of the second condition (number_.fractional == 0U)?
I know that (number_.fractional & 1U) is used to implement banker's rounding, but what is the role of fractional==0 here? Can't we round down according to the banker's rounding rules? Will this cause error accumulation?
Similarly, I noticed that in the latter if block, for the case of precision==0, there is no similar judgment of integral==0.

  if (precision == 0U) {
    remainder = abs_number - (double) number_.integral;
    if ((!(remainder < 0.5) || (remainder > 0.5)) && (number_.integral & 1)) {
      // exactly 0.5 and ODD, then round up
      // 1.5 -> 2, but 2.5 -> 2
      ++number_.integral;
    }
  }

Also, I found a similar issue in mpaland's repository
mpaland#51

@HenryLin2000
Copy link
Author

HenryLin2000 commented Nov 24, 2023

I tried calling printf on my computer (x86+windows+mingw) to output floating point numbers. If it is retained to an integer, the integer part is rounded towards an even number. If it is retained to n decimal places and the first n decimal places are all 0, the decimal part is rounded up. This behaves the same as printf.c in our repository. So problem 2 I mentioned may not be a problem.

But based on the above rules, for the lines 599-602 I mentioned, simply adding a carry will result in different behaviors when retaining to an integer, so I think the complete modification can be as follows:

  else if (remainder == 0.5) {
    if ((number_.fractional == 0U) || (number_.fractional & 1U)) {
      // if halfway, round up if odd OR if last digit is 0
      ++number_.fractional;
      // handle rollover after round up, e.g. case 0.995 with precision 2 is 1.00
      if ((double) number_.fractional >= powers_of_10[precision]) {
        number_.fractional = 0;
        ++number_.integral;
      }
    }
  }

  if (precision == 0U) {
    remainder = abs_number - (double) number_.integral;
    if ((!(remainder < 0.5) || (remainder > 0.5)) && (number_.integral & 1)) {
      // exactly 0.5 and ODD, then round up
      // 1.5 -> 2, but 2.5 -> 2
      ++number_.integral;
    }
    if (remainder == -0.5 && (number_.integral & 1)){
      --number_.integral;
    }
  }

Also, I found the original code of the 3rd if block strange, it seemed to never be executed based on the current conditions.
A related issue: #153

@eyalroz
Copy link
Owner

eyalroz commented Nov 24, 2023

It looks like you're looking at the master branch rather than the development branch. Please re-examine...

@HenryLin2000
Copy link
Author

Sorry, I didn't notice the develop branch before. But I found there is still a problem in develop branch.

printf/src/printf/printf.c

Lines 626 to 630 in df06fd4

else if ((remainder == one_half) && (number_.fractional & 1U)) {
// Banker's rounding, i.e. round half to even:
// 1.5 -> 2, but 2.5 -> 2
++number_.fractional;
}

For the second if block, it still doesn't handle integer carries when the decimal exceeds the digit limit.
For example, when you call get_components(0.995, 2), you will get a number_ with fractional=100, which will make a mistake.

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

2 participants