Hi Peter, On Sun, Nov 28, 2021 at 10:17:50AM +0100, Peter Rosin wrote: > Hi! > > On 2021-11-27 21:27, Liam Beguin wrote: > > Hi Peter, > > > > On Mon, Nov 22, 2021 at 01:53:44AM +0100, Peter Rosin wrote: > >> Hi Liam! > >> > >> On 2021-11-15 04:43, Liam Beguin wrote: > >>> Hi Jonathan, Peter, > > snip > > >>> - keep IIO_VAL_FRACTIONAL scale when possible, if not default to fixed > >>> point > >> > >> This is not what is going on. Patch 9/14 will convert all fractional > >> scales to fixed point. But I would really like if you in the "reduce > >> risk of integer overflow" patch (8/14) would hold true to the above > >> and keep the fractional scale when possible and only fall back to > >> the less precise fractional-log case if any of the multiplications > >> needed for an exact fractional scale causes overflow. > > > > Thanks for looking at these patches again. > > > >> The v8 discussion concluded that this was a valid approach, right? > > > > Yes, I remember you saying that you'd be more comfortable keeping the > > IIO_VAL_FRACTIONAL. > > > >> I know you also said that the core exposes the scale with nano > >> precision in sysfs anyway, but that is not true for in-kernel > >> consumers. They have an easier time reading the "real" scale value > >> compared to going via the string representation of fixed point > >> returned from iio_format_value. At least the rescaler itself does so, > >> which means that chaining rescalers might suffer needless accuracy > >> degradation. > > > > Agreed, that makes total sense. > > > > If I'm not mistaken, the first condition in the case, if (!rem), will > > return IIO_VAL_FRACTIONAL if the division is exact, keeping all the > > precision. No? > > Only if the resulting scale fits in nine decimals. That's never the > case if you have primes other than 2 and 5 in the denominator (after > eliminating gcd of course). Which mean that if you chain one rescaler > doing 1/3 and one doing 3/1, you would get a combined scale of > 0.999999999 instead of 3/3 if we take the approach of these patches. > > So, what I'm after is that - for IIO_VAL_FRACTIONAL - not take the > multiply-by-1e9 code path /unless/ the existing fractional approach > overflows in either numerator or denominator (or both). Understood, I'll update based on this. > Side note: The same could be done for IIO_VAL_INT when the numerator > overflows (since the denominator cannot overflow), but I guess that > can be done later. Agreed, I don't mind working on this later but I'd like to focus on getting the current changes in first. Thanks, Liam > Cheers, > Peter
