Skip to content
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

[Documentation] Tolerance handling in checked_exp_with_tolerance method #587

Open
saona-raimundo opened this issue Apr 25, 2023 · 2 comments

Comments

@saona-raimundo
Copy link

Hi! Thanks for the crate!

This request concerns the desired documentation about the handling of tolerance.
The method checked_exp_with_tolerance is the only one that accepts a tolerance in the rust_decimal::MathematicalOps trait.

The current documentation for checked_exp_with_tolerance says the following.

The estimated exponential function, ex using the tolerance provided as a hint as to when to stop calculating. 
A larger tolerance will cause the number to stop calculating sooner at the potential cost of a slightly less accurate result. 
Returns None on overflow.

The tolerance being a "hint" does not imply any guarantee for the computation.
In the implementation of Decimal, a fixed-length factorial expansion is used, which is very reasonable.

Would you consider adding some words about the guarantees one can make about the tolerance?
Even guarantees within a range of Decimal would be appreciated.

(I know precision guarantees are hard, so thanks for considering this request.)

@paupino
Copy link
Owner

paupino commented Apr 27, 2023

Yes, great point! Documentation is one area that can always be improved.

In lieu of updating the docs, the short answer is that the tolerance provides the acceptable range in which we can consider the result correct. The lower the tolerance, the more accurate the result, however it also means it may be a slower calculation. We're effectively allowing us to configure how deep into the "series" that we need to go before achieving an acceptable result.

Of course the reverse of this, the higher the tolerance, the more inexact the result is going to potentially be. That said, for some simple applications, this may be fine.

So in terms of guarantees, tolerance is kind of like specifying that the result is going to be within 0 <= x <= tolerance of the final answer OR has reached the end of the series due to not being able to represent a more granular result within Decimal (yet!!!). Because of the latter (i.e. end of the series), it's difficult to make a strong guarantee right now, but hopefully it helps.

@saona-raimundo
Copy link
Author

Thanks for the answer.

So in terms of guarantees, tolerance is kind of like specifying that the result is going to be within 0 <= x <= tolerance of the final answer OR has reached the end of the series due to not being able to represent a more granular result within Decimal (yet!!!). Because of the latter (i.e. end of the series), it's difficult to make a strong guarantee right now, but hopefully it helps.

I understand the role of tolerance and think it is a very intuitive API.
But, as a (maybe not so well-informed) user, I expect that a given tolerance specified in Decimal will translate to an actual error guarantee (which is not the case and it is very hard to do).

Proposal

If the idea is that tolerance is really just a hint, and no formal guarantee will be given, I suggest renaming the input tolerance to tolerance_hint and adding to the documentation that no guarantee is given in the result of the computation.

Alternative 1

Change the return type to signal that the result is not within the tolerance level specified.
From

fn checked_exp_with_tolerance(&self, tolerance: Decimal) -> Option<Decimal>

to

fn checked_exp_with_tolerance(&self, tolerance: Decimal) -> Result<Decimal, ToleranceError>

where ToleranceError has variants: Overflow and OutOfRange (or similar).

Alternative 2

A different approach would be to accept a tolerance_level: usize parameter (renamed cutoff or something else) that specifies the number of terms in the expansion used in the computation.
But honestly, I do not know how useful this would be, and accepting any usize is also a problem because the terms of the series are finite, so one can not use the same approach to deal with any usize as a cutoff limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants