computation #steps and computation order

[mw66]

https://github.com/dlang-community/d-money/blob/master/source/money.d#L249

    T opBinary(string op)(const real rhs) const
    {
        static if (op == "*")
        {
            const converted = T(rhs);
            bool overflow = false;
            const result = muls(amount, converted.amount, overflow);
            if (overflow)
                throw new OverflowException();
            return fromLong(result / __factor);
        }
        else static if (op == "/")
        {
            const converted = T(rhs);
            bool overflow = false;
            auto mult = muls(amount, __factor, overflow);
            if (overflow)
                throw new OverflowException();
            return fromLong(mult / converted.amount);
        }

every operation may lost precision, so the general principle of computation order is:

1. we want use as less op as possible, by remove the unnecessary ops.
2. always use the original value first.

take op "*" as an example,

1. it's first converted to T(rhs), which means rhs * __factor
   https://github.com/dlang-community/d-money/blob/master/source/money.d#L91
2. then converted.amount * this.amount,
3. finally divided by __factor

total 3 ops; instead it should do:

this.amount * rhs => then passing to fromLong

1 ops.

Please check my comments here, which have similar problems:

m3m0ry/fixedpoint#5 (comment)
m3m0ry/fixedpoint#5 (comment)

There are maybe other places in the code which have this order problem, please give it a check when you have time.

Numerical stability is very subtle to get it right.

@schveiguy FYI.

[qznc]

Thanks for raising the topic. What is the actual issue: Numerical stability (precision) or speed?

The op mult example currently: float multiply by static, integer conversion, integer multiply, integer divide by static. An optimizing compiler might convert the integer division by static into something more efficient (like shifts). Maybe the first one as well. We would have to look at the assembly.

Numerical stability is not that much of an issue (i believe) because we are doing mostly integer computation here.

I'm not completely sure what the proposal is. I assume it is meant to be a floating point multiplication because with an integer multiplication we would end up with the current solution. A floating point result cannot be passed to to fromLong directly. We still need a conversion and thus a multiplication by __factor. Also, this.amount has the wrong unit, we have to float-divide by __factor.

Concluding, I don't see a big improvement. I don't even know what the actual problem is. Maybe you have a concrete example which can be faster or more precise?

[mw66]

It's for both precision & speed. Basically first promote (i.e * __factor) to currency, then divide by __factor will lose on both.

I will provide an example later.