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

Version 2 Roadmap planning #354

Open
8 tasks
paupino opened this issue Apr 23, 2021 · 9 comments
Open
8 tasks

Version 2 Roadmap planning #354

paupino opened this issue Apr 23, 2021 · 9 comments

Comments

@paupino
Copy link
Owner

paupino commented Apr 23, 2021

Some thoughts for version 2 of the Decimal library. Issue numbers will be added as they are created.

  • Define updated rust minimum version.
  • Remove deprecated functions/features.
  • Disable all features by default - make opt in. std is only required by default IF it is required.
  • Review API design. It is a little inconsistent in places - it'd be nice to make this more consistent and extensible. Some guidelines for API design may be useful. Related to Review error handling within Decimal #49.
  • Investigate u128 optimizations when performing 96 bit operations. The hypothesis is that I can squeeze further performance from more complex numbers. Related to i128/u128 based implementation #135.
  • Investigate configurable precision decimals, perhaps leveraging constant generics. Reduced precision is easy, however greater precision is an interesting challenge since current optimizations won't hold true. This requires some experimentation to figure out if anything makes sense within this library or whether it's a variation. Related to i128/u128 based implementation #135 and Plans for const generics? #310.
  • Remove From support for Decimal. Decimal::from((u128|i128)::MAX) panics at runtime, TryFrom looks like the only reasonable approach. (excerpt: "Note: This trait must not fail. The From trait is intended for perfect conversions. If the conversion can fail or is not perfect, use TryFrom").
  • "Strict" mode. This will allow for explicit underflow handling (e.g. return an error instead of rounding).
@paupino
Copy link
Owner Author

paupino commented Mar 4, 2022

Brainstorming thread:

Lazily evaluated rational numbers? i.e. maintain precision until absolutely necessary. Could be a different type using decimal as a subset.
Const generics - should this be the underlying type (e.g. u32 vs u64 vs u128) or the size of the words (e.g. 4, 8, 16 etc).

@zzhengzhuo
Copy link

zzhengzhuo commented Jul 17, 2024

Sorry to bother you.
I think the current version is stable enough. Would you consider starting the upgrade to v2?
And I would be pleased to offer my assistance.

@paupino
Copy link
Owner Author

paupino commented Jul 24, 2024

Hi @zzhengzhuo - sorry for the delay in reply. Truth be told I haven't had much time to focus on this project (or others for that matter) as of late. But I do agree that we could probably, at the very least, start a branch revisiting the API/Errors and cleaning that up. From there we could really just focus on the internals (if need be).
If you're willing to start a new PR/development I'd be more than happy to branch off v1 so that development of v2 breaking changes can be performed on the main branch. I really appreciate the offer for assistance!

@zzhengzhuo
Copy link

Thanks for your response. I'll be starting development on the master branch soon. I'll likely begin with feature #310, as it seems to involve significant breaking changes.

@Tony-Samuels
Copy link
Collaborator

Do you know what features you want included in 2.0, @paupino? I wasn't sure if #310 would make the cut. It's on the list in the original message, but only as an investigation.

@paupino
Copy link
Owner Author

paupino commented Jul 25, 2024

I don't have a firm list however you're right that #310 is biting off quite a large chunk (hence why it was to investigate). If I were to think of a simple path to v2 I'd probably begin with:

  1. Create simple error type
  2. Revisit current API - remove deprecated, remove invalid From implementations (that panic), replace option with error where appropriate, rename badly named functions (e.g. constructors) etc.
  3. Remove std from default features (and serde for that matter)
  4. Add further const functions (some can't be because of the error type etc)
  5. Revisit "add-on" features - is there a way to make these more componentized/compartmentlized?

I think for underlying storage it could be interesting to play around with 128 bit types. At the time, I didn't notice much difference, or it was slower, on the machines I tested on. That said, recent improvements with 128 bit handling could be interesting. Though, this is perhaps another investigation rather than a "go do".

While I'd be interested in playing around with const generics it really is likely a complete rewrite - or at least touching almost every part of the code. So I think an investigation/prototype and documentating what needs to change / could change is worthwhile as a first step.

@zzhengzhuo
Copy link

zzhengzhuo commented Jul 28, 2024

Based on my preliminary investigation, if Decimal is defined as

pub struct Decimal<const LIMBS: usize> {
    flags: u32,
    limbs: [u32; LIMBS],
}

then const generics would be a variation of the current version, making modifications relatively simple.
In this case, I think the parts that need to be modified are:

  • Remove From<u32>, etc., and replace it with TryFrom<u32>. FromPrimitive would be implemented by calling TryFrom, not the other way around.
  • Similarly, TryFrom<Decimal> and ToPrimitive.
  • Use from_i64_with_scale instead of the Decimal::new method.

Next comes the more tedious part of removing deprecated methods and so on.

In addition, there are a few other aspects that need to be concluded:

  • negative should not be part of const generics, which should be obvious
  • Should scale be part of const generics? This seems debatable. I tend to think it should not be part of const generics because having two const generics might not be very ergonomic. I can alias Decimal<3> as Decimal96, but including scale would be more complicated.

What do you think? @paupino

@paupino
Copy link
Owner Author

paupino commented Aug 2, 2024

I think the complexity will creep in once you get to the multiplication / division logic. There are some optimizations in there that assume a fixed size array right now. In fact, there is some "short circuit" optimizations that rely on the structure of hi/lo/mid with relevant overflow.

For example, multiplication allocates space for a 192 bit number and then tries to resize it back into a 96 bit number. If you have a variable size number you'd need to be able to handle all of this more dynamically. div gets a little bit more complex again, since it tries to handle underflow scenarios (e.g. 10/3), scaling up or down as needed. I'd suggest checking out the code in https://github.com/paupino/rust-decimal/blob/master/src/ops/ etc (you can ignore legacy.rs - that is deprecated)

Anyway - not meaning to discourage you - more so to take a deeper look a these particular areas of code. These are the more complex components that would need rewriting which to be honest, is what has stopped me each time.

After doing so, it should be fairly easy to benchmark that the behavior of Decimal<3> is the same (or better) than the current implementation whilst giving the same results throughout the tests.

@Xuanwo
Copy link

Xuanwo commented Oct 14, 2024

Hi, @paupino, thank you a lot for building this! I'm using this crate in iceberg-rust, the Rust implementation of Apache Iceberg™.

I love this crate, but I've noticed a gap: Iceberg specifies decimal support up to 38 precision, while this crate only supports 28. Is it possible to extend our decimal support range? For example, could we use 128 bits to store the value? I'm willing to help implement this part.

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

4 participants