Re: [lmi] Converting cents to dollars

[Vadim Zeitlin]

On Sat, 10 Apr 2021 20:46:34 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 4/8/21 1:10 PM, Vadim Zeitlin wrote:
GC> [...]
GC> > ---------------------------------- >8 
--------------------------------------
GC> > diff --git a/currency.hpp b/currency.hpp
GC> > index ce04d75a2..a73a2b627 100644
GC> > --- a/currency.hpp
GC> > +++ b/currency.hpp
GC> > @@ -40,6 +40,7 @@ class currency
GC> > 
GC> >      static constexpr int    cents_digits     = 2;
GC> >      static constexpr double cents_per_dollar = 100.0;
GC> > +    static constexpr double cents_per_dollar_inv = 0.01;
GC> > 
GC> >    public:
GC> >      using data_type = double;
GC> > @@ -58,8 +59,7 @@ class currency
GC> > 
GC> >      data_type cents() const {return m_;}
GC> >      // CURRENCY !! add a unit test for possible underflow
GC> > -    // CURRENCY !! is multiplication by reciprocal faster or more 
accurate?
GC> > -    double d() const {return m_ / cents_per_dollar;}
GC> > +    double d() const {return m_ * cents_per_dollar_inv;}
GC> > 
GC> >    private:
GC> >      explicit currency(data_type z, raw_cents) : m_ {z} {}
GC> > ---------------------------------- >8 
--------------------------------------
GC> 
GC> As I awoke this morning, I thought we might perhaps combine this
GC> idea with something nearly similar in 'round_to.hpp':
GC> 
GC> /// Division by an exact integer value would afford a stronger
GC> /// guarantee of accuracy, particularly when the value to be rounded
GC> /// is already rounded--e.g., if 3.00 is to be rounded to hundredths,
GC> /// then
GC> ///   (100.0 * 3.00) / 100
GC> /// is preferable to
GC> ///   (100.0 * 3.00) * 0.01
GC> /// especially if the rounding style is anything but to-nearest.
GC> ///
GC> /// However, reciprocal multiplication is faster than division--see:
GC> ///   https://lists.nongnu.org/archive/html/lmi/2021-04/msg00010.html
GC> /// et seqq., which demonstrates an average three-percent speedup,
GC> /// with no observed difference in a comprehensive system test as
GC> /// long as the multiplications are performed in extended precision.
GC> 
GC> IOW, instead of
GC> 
GC> > +    static constexpr double cents_per_dollar_inv = 0.01;
GC> ...
GC> > -    double d() const {return m_ / cents_per_dollar;}
GC> > +    double d() const {return m_ * cents_per_dollar_inv;}
GC> 
GC> above, use a 'long double' reciprocal.
GC> 
GC> It does cause regressions, so of course I'm not disposed to commit it;
GC> but it does raise several questions, enumerated below, for discussion.
GC> 
GC> (1) I'm testing this in our i686+x87 production environment. What are
GC> your thoughts about migrating (later) to x86_64 and using the x87
GC> hardware only for 'long double'?

 Sorry, I'm not sure what are the alternatives? If we use "long double",
then we don't really have any choice about what to use for it, as it's only
support for x87 instruction set.

 Of course, personally I'd rather get rid of "long double" entirely and use
just normal, much faster and much, much less surprising "double". But from
what I understand this doesn't really seem possible, as this would result
in so many changes in the system test that they never could be validated,
realistically. Please correct me if I'm wrong.

GC> (2) In the first part of that patch, I couldn't find a way to write
GC> a 'long double' constexpr inside the class. I imagine this is a
GC> gcc-8.3 or C++17 limitation that will probably be lifted in the near
GC> future;

 I have indeed no trouble compiling

        struct S {
            static constexpr long double x = 1.0;
        };

with g++ 10 in C++20 mode, but then it also compiles for me with g++ 8.4 in
C++17 mode without any problems. If you're still interested[*] in debugging
this problem, could you please show me the error messages you got?

GC> if not, writing that constexpr at namespace scope is a good
GC> enough expedient.

[*] in spite of this


GC> (3) Over the course of this experiment, that patch evolved into
GC> something different from the original intention. In effect, it's
GC> equivalent to just this:
GC> 
GC> -    double d() const {return              m_ / cents_per_dollar;}
GC> +    double d() const {double volatile z = m_ / cents_per_dollar; return z;}
GC> 
GC> IOW, compared to HEAD, it just forces rounding to 'double'
GC> instead of using an 80-bit x87 register for the return value.

 This is probably wrong because I can't logically explain it, but it just
seems wrong to use long double for the divisor when both the dividend and
the final result are double. I realize that the result may be different,
but ideally nothing should depend on such difference.

GC> So should we just do whatever testing is needed to accept
GC> something like this patch? No. This is only a symptom. We
GC> should instead spend time on the underlying problem: i.e.,
GC> that lmi was written in terms of fractional dollars, and then
GC> only partly migrated to integral cents. We don't want d() to
GC> be faster--we want to render it unnecessary.

 This would certainly be the best solution, especially if you think it's
achievable. I'm afraid I can't really help with anything here but, again,
please let me know if I'm wrong about this too and if there is anything I
can do to advance towards this goal and/or getting rid of the dependency on
long double.

 Thanks,
VZ

pgpQaWaE4xen_.pgp
Description: PGP signature