eyalroz / printf

Tiny, fast(ish), self-contained, fully loaded printf, sprinf etc. implementation; particularly useful in embedded systems.
MIT License
416 stars 51 forks source link

Rounding-Error: Use proper banker's rounding #153

Open orgua opened 1 year ago

orgua commented 1 year ago

Isn't that comparison (first line) wrong considering the following comment?

    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;
    }

Link

I think it should be

    if (!((remainder < 0.5) || (remainder > 0.5)) && (number_.integral & 1)) {
eyalroz commented 1 year ago

So, this is code from Marco Paland's original repository, which I haven't touched. It does seem a bit weird and I need to take another look.

... but I don't quite understand your suggestion, e.g. why do you suggest we avoid the equality operator.

orgua commented 1 year ago

yeah, this is inherited from the original repo, but it's still wrong compared to the embedded comment. it came up during linting of a code-base.

Doing it like that with an extra step might fix some weird float-behavior with very small remainders in the fraction-part?!

eyalroz commented 1 year ago

So, I just made it Banker's rounding.

HenryLin2000 commented 10 months ago

In fact, I found that when executing this block, there is always remainder<=0.5, because the remainder here is updated (in line 605) https://github.com/eyalroz/printf/blob/f8ed5a9bd9fa8384430973465e94aa14c925872d/src/printf/printf.c#L604-L611 The abs_number here is the absolute value of the original number, and the value of integral depends on whether the number is carried. There are 3 situations at this time

  1. If the previous remainder>0.5 (carry), then the current remainder is a negative number
  2. If the previous remainder<0.5 (no carry), then the current remainder is between the interval [0,0.5)
  3. If the previous remainder=0.5 (special case), then the current remainder is also 0.5, and rounding processing is executed.

So, there is never remainder>0.5, and the condition here is really a bit strange.