Re: [PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-02-02 Thread Dean Rasheed
On Tue, 31 Jan 2023 at 21:59, Joel Jacobson wrote: > > Nice, you managed to simplify it even further. > I think the comment and the code now are crystal clear together. > > I've tested it successfully, test report attached. > Cool. Thanks for testing. Committed. Regards, Dean

Re: [PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-01-31 Thread Joel Jacobson
On Tue, Jan 31, 2023, at 20:25, Dean Rasheed wrote: > That seems a bit wordy, given the context of this comment. I think > it's sufficient to just give the formula, and note that it simplifies > when DEC_DIGITS is even (not just 4): > > /* > * Assume the input was normalized, so

Re: [PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-01-31 Thread Dean Rasheed
On Tue, 31 Jan 2023 at 15:05, Joel Jacobson wrote: > > I also think the performance impact no matter how small isn't worth it, > but a comment based on your comments would be very valuable IMO. > > Below is an attempt at summarising your text, and to avoid the performance > impact, > maybe an

Re: [PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-01-31 Thread Joel Jacobson
Hi, On Tue, Jan 31, 2023, at 14:40, Dean Rasheed wrote: > That's still not right. If you want a proper mathematically justified > formula, it's fairly easy to derive. ... > or equivalently, in code with truncated integer division: > > if (arg.weight >= 0) > sweight = arg.weight *

Re: [PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-01-31 Thread Dean Rasheed
On Tue, 31 Jan 2023 at 08:00, Joel Jacobson wrote: > > I think this is what we want: > > if (arg.weight < 0) > sweight = (arg.weight + 1) * DEC_DIGITS / 2 - 1; > else > sweight = arg.weight * DEC_DIGITS / 2 + 1; > That's still not right. If you

Re: [PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-01-31 Thread Joel Jacobson
Hi, On Sun, Jan 29, 2023, at 14:33, Dean Rasheed wrote: > On Sat, 28 Jan 2023 at 22:14, Joel Jacobson wrote: >> HEAD, patched: >> sweight = (arg.weight * DEC_DIGITS) / 2 + 1 > > You haven't actually said why this formula is more correct than the > current one. I believe that it is when

Re: [PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-01-29 Thread Dean Rasheed
On Sat, 28 Jan 2023 at 22:14, Joel Jacobson wrote: > > I found what appears to be a small harmless error in numeric.c, > that seems worthwhile to fix only because it's currently causes confusion. > Shrug. Looking at git blame, it's been like that for about 20 years, and I wasn't aware of it

[PATCH] Fix old thinko in formula to compute sweight in numeric_sqrt().

2023-01-28 Thread Joel Jacobson
Hi, I found what appears to be a small harmless error in numeric.c, that seems worthwhile to fix only because it's currently causes confusion. It hasn't caused any problems, since the incorrect formula happens to always produce the same result for DEC_DIGITS==4. However, for other DEC_DIGITS