-
Notifications
You must be signed in to change notification settings - Fork 183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
powd
overflow
#431
Comments
Here's how to reproduce: $ evcxr
Welcome to evcxr. For help, type :help
>> :dep rust_decimal = { version = "1.16", default-features = false, features = ["maths"] }
>> use rust_decimal::prelude::*;
>> let base = Decimal::new(32_313_447, 0);
>> let exponent = Decimal::new(68, 2);
>> let one = Decimal::new(1, 0);
>> base
32313447
>> exponent
0.68
>> base.powd(exponent)
127750.73491154239696209320793
>> (base + one).powd(exponent)
thread '<unnamed>' panicked at 'Pow overflowed', .../github.com-1ecc6299db9ec823/rust_decimal-1.16.0/src/maths.rs:283:21
stack backtrace:
0: std::panicking::begin_panic
1: <rust_decimal::decimal::Decimal as rust_decimal::maths::MathematicalOps>::powd
2: run_user_code_11
3: evcxr::runtime::Runtime::run_loop
4: evcxr::runtime::runtime_hook
5: evcxr::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Segmentation fault.
0: evcxr::runtime::Runtime::install_crash_handlers::segfault_handler
1: <unknown>
2: mi_free
3: alloc::alloc::dealloc
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/alloc/src/alloc.rs:104:14
<alloc::alloc::Global as core::alloc::Allocator>::deallocate
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/alloc/src/alloc.rs:239:22
alloc::alloc::box_free
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/alloc/src/alloc.rs:334:9
panic_unwind::real_imp::cleanup
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/panic_unwind/src/gcc.rs:83:5
__rust_panic_cleanup
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/panic_unwind/src/lib.rs:99:19
4: std::panicking::try::cleanup
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:360:42
5: std::panicking::try::do_catch
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:404:23
std::panicking::try
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:343:19
std::panic::catch_unwind
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panic.rs:431:14
std::rt::lang_start_internal
at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/rt.rs:34:21
6: main
7: __libc_start_main
at /build/glibc-vjB4T1/glibc-2.28/csu/../csu/libc-start.c:308:16
8: _start
Child process terminated with status: signal: 6
>> |
|
We cannot. We are forbidden to use floating point at all, because of non-determinism, i.e. different results being obtained Isn't there a way to use fixed point arithmetic but truncating the less significant digits, instead of overflowing? I'm now using saturating ops (through |
Also, I've noticed that |
I understand that the IEEE 754 standard defines deterministic calculations for addition, subtraction, multiplication, division plus the square root, meaning that they should generate deterministic output when given the exact same inputs. So your restriction is strange; different implementations / architectures should NOT give different results - they would only be wrong results, meaning that the implementation is wrong For exponential (and also power), you start running into the Table Maker's Dilemma and sooner or later you get into rounding situations no matter what you use. It is probably possible to make an I'm not really an expert on this, but if you want |
Yes. In fact it would be nice if this library used For now, it would be good for us if |
https://github.com/paupino/rust-decimal/blob/master/src/maths.rs#L316-L323 This is where it calculates the Since you know the exponent will be <1, I suggest you break down the steps and display intermediate values to find out which step it is overflowing. Then you may simply add |
The problem is a I guess there's not much to do, except for increasing the number of bits, or improving / optimizing the impl somehow (if possible). |
Mul overflows because you have too many significant digits. You can avoid it by trimming down the number of digits. |
As @schungx correctly points out, it's due to the current implementation attempting to maintain precision. This "overflowing" precision is not uncommon in simple multiplication and division scenarios - i.e. typically we'd call this an "underflow" and automatically round the number to make it fit into the allocated bits. Of course, when we have more complex scenarios whereby a multiplication is just a small part of the formula you'll start losing precision later down the track each time you round - especially as smaller numbers trend to zero. The current Anyway, one possible solution would be to keep the 196 bit precision product in tact until the calculation is complete - i.e. only rounding to 96 bits at the last moment. Unfortunately, this requires a fair bit of refactoring to get going since other operations would also need to be able to support The better solution is to of course keep as much precision, for as long as possible. This is definitely a large chunk of work which makes it tempting to wait until v2 for (since variable precision could make this much easier). The easier solution (for now) would be to provide saturating logic in the power function - perhaps as a separate function. It'd still require a bit of fiddling around to get going but could perhaps solve your requirement. |
Thanks for the detailed explanation. I tried the "normalizing" approach mentioned above (basically, calling |
OK, best solution for me is to dump the last term (27) of the power series when computing the exponential. That introduces an error of roughly one decimal digit, but extends the range of That said, I don't think this is the right thing to do. Will just introduce an upper cut-off / saturating value in my function, and that's it. Thanks for your time and your work. |
Of course, I can always do: for n in 2..=27 {
term = term.checked_mul(self.div(Decimal::new(n, 0)))?;
let next = result + term;
let diff = (next - result).abs();
result = next;
if diff <= tolerance {
break;
}
} basically distributing the powers over the factorial. But I guess this opens up its own can of worms, in terms of precision. Do you think this method has merit? Problems as I see it are:
Nice thing is that the number of terms can be easily adjusted. And, results are pretty good for small exponents (which is my case in particular). |
We can also use different algorithms for different ranges of inputs, picking the best one depending on the actual input values. |
You'd need to normalize to a precision that |
We got a similar issue with the following code: >> let v = Decimal::from_f64(13.815517970299580976037625513).unwrap();
>> v.exp() We are also using Decimal in the blockchain. |
Am I correct in thinking a division also hits a similar error because of maintaining precision? In my case I'm just simply dividing two numbers and somehow must have hit the exact right inputs to trigger this:
|
@arrtchiu For division to overflow it would typically mean the quotient was being divided by a very small number which is a slightly different problem. Behind the scenes, division actually does what That said, can you provide the inputs that you were using? That'll help define if there is an issue or if it is expected behavior. |
Thanks @paupino, makes sense. I don't have the inputs unfortunately but if I see it again I'll instrument and try to get a repro case. |
Hello, and first off, thanks for the library.
We're using
rust_decimal
to compute a function using fixed point math, involvingexp
andpowd
.Now, what we are noticing is that
powd
's range is not very large. By example, when raising aDecimal
larger than 32_313_447 to a Decimal power smaller than one (0.68 in particular) usingpowd
, we are hittingthread ... panicked at 'Pow overflowed', .../github.com-1ecc6299db9ec823/rust_decimal-1.16.0/src/maths.rs:283:21
This doesn't make much sense, as a number raised to an exponent smaller than 1 would yield a smaller number (127_755, in this particular case).
Are you aware of this? I can always take a look at the code, but, asking first in case this is a kwown issue / there are known workarounds. What do you suggest?
Thanks,
The text was updated successfully, but these errors were encountered: