Overflow with small number of multiplication

[dnsl48]

Overflow with small number of multiplication

[dnsl48]

I think that comes to the internal representation of our Decimal implementation. Our Decimal is just a wrapper over Fraction type, which in turn is a wrapper around num::Ratio. As such, Decimals use rationals internally for the computations.
Given that,
sum is internally represented as Ratio { numer: 357527999989334071111237863, denom: 1000000000000000000000000 }
circ_ratio is internally represented as Ratio { numer: 3141592653589793, denom: 180000000000000000 }

I didn't manually debug that, but from what I can see, the denominators are different, so to figure out the result of the multiplication we will have to get an intermediate value of these two denoms and numerators multiplied, and those values don't fit on stack.

the structure needs to be copy-able so I can't use BigDecimal yet

Well, that feels like a limitation of this library, but it needs the intermediate values of the computations to be bigger than the computation results. One potential way to work it around might be to use BigDecimal for the computation and then downcast it to the simple Decimal if the final result fits. But obviously that requires some manual juggling in the code. Sorry it's not that simple for your use-case.

Also, there might be other rust libraries out there which are more optimized for decimal calculus (not using Rationals internally).

[ChristopherRabotin]

I ended up implementing it this way, and it works like a charm since this function returns an f64:

fn inner_g_rad(&self) -> f64 {
        // Let's do this computation on a big decimal and downcast later.
        use crate::fraction::BigDecimal;
        use std::f64::consts::TAU;
        let g_rad = (BigDecimal::from(TAU) / BigDecimal::from(360.0))
            * (BigDecimal::from(357.528)
                + BigDecimal::from(35_999.050 * self.as_tt_centuries_j2k()));
        let g_rad_f64 = g_rad.to_f64();

        let inner = g_rad + BigDecimal::from(0.0167 * g_rad_f64.as_ref().unwrap().sin());
        inner.to_f64().unwrap()
    }

[ChristopherRabotin]

Thanks for all your help and for being super responsive.

[dnsl48]

Hi @ChristopherRabotin, thanks for reporting that!

To me this looks correct as we are relying on the num::Ratio implementation underneath, and it does not perform checked math by default (for performance reasons).

In the case you reported the precision of -0.0000000000002962836210611707; appears to be high enough to assume that there won't be enough space on stack for most of math operations with those numbers (as Decimals are using Fractions internally, and those rely on Rational calculations).

To work with those numbers without overflows you may switch to BigDecimal or DynaDecimal.
Those types use memory allocation for working with bigger numbers, not fitting into stack.

[ChristopherRabotin]

Thanks for your quick response.

How come that number can't be represented without memory allocation? It fits as an f64, and if PI and TAU can be represented (although they are irrational), I would assume that this other number would be representable. What am I misunderstanding?

I'm using fraction for high precision time computations, but the structure needs to be copy-able so I can't use BigDecimal yet.

[ChristopherRabotin]

I like the idea of using BigDecimal only for this calculation and then downcasting it.

I'm using fraction for hifitime and the bug is extremely limited: if you try to convert a time to this odd time system (called Dynamic Barycentric Time, or TDB) which is within +/- 1.5 seconds from 2000-01-01T11:59:27.815064907 then it blows up in debug mode. Every other conversions works fine.

[dnsl48]

using BigDecimal only for this calculation and then downcasting it

This is what DynaInt type already does. However, the type itself is not implementing Copy trait. However, you could perhaps use that type manually fetching its value with its unpack method. I guess the easiest way to do that would be to use DynaDecimal and then extract its numer/denom values initializing Decimal with unpack'ed` numer and denom values.

[dnsl48]

Oh, true that's much simpler if you need f64 as the result :)